language
language copied to clipboard
Records: What does `toString` do?
The record proposal says:
The toString() method's behavior is unspecified.
This is all well and good, but we have to implement something. So, what do we do here?
cc @munificent @eernstg @lrhn @jakemac53 @natebosch @stereotype441 @rakudrama @mraleph
Ugh, yeah. Honestly, I think the right thing is to print the fields and all of their names. Give the users what they want to see when they are debugging their app. We keep the shape and names around for printing function types, so it doesn't seem unreasonable to do the same thing for records.
But I know some people would really like to be able to tree-shake those names. I sympathize but... it's really painful to make debugging harder. I'm happy to defer to letting the core lib and compiler folks hash this one out. :)
tl;dr: The specification should not force retaining information that isn't necessary at runtime.
Records are structural data, and we can see from the program code how they are destructured.
If a compiler can see that a record type contains an element which is never used, it should be able to completely eliminate that element and never store a value.
If the toString
is required to return (${this.0}, ${this.1}, foo: {$this.foo})
for (int, int, foo: int)
, even though that toString
is the only code which ever uses the .1
value for anything, then we are preventing that optimization.
I'd at least prefer to keep the toString
unspecified or under-specified, so it's possible to return (${this.0}, <optimized away>, foo: ${this.foo})
and stay within the specification.
We probably will retain enough information to know the named element names of records, in order to facilitate dynamic
member access. It's OK to use that in the output, but it should also be fine if it ends up with minified member names instead of the source identifier.
As long as the toString
can be computed entirely from the information in the record shape (which should only exist as one object per shape in the program) and the stored element values, we should be fine. We should not mandate what that record shape information contains at runtime, the implementations should just use what they have, and not retain anything solely for implementing toString
.
We should then also document, very clearly, on Record.toString
, that THE FORMAT IS UNSPECIFIED AND ANYTHING YOU SEE IS BEST EFFORT ONLY, and ONLY INTENDED FOR DEBUGGING.
Tell all code reviewers that code parsing record toString
is broken and should be rejected.
Put questions on stack overflow about it, and answer them with "never do that!"
(Which also means that a result of (<optimized away>)
should be completely acceptable. And that we can choose to make the toString
act differently between debug and production mode.)
Could we:
- leave it unspecified
- in debug mode print
(${this.0}, ${this.1}, foo: {$this.foo})
for(int, int, foo: int)
- in release mode print
(${this.0}, ${this.1}, _: {$this.foo})
for(int, int, foo: int)
?
@mit-mit in principle, yes, and I don't think that's necessarily a bad idea. But it's always somewhat risky to have code that behaves one way when you run your tests and another way when out in the wild. Any code that parses the result of toString()
on a record will pass its tests but then fail in the shipped app. How about we:
- Specify that
toString()
prints it nicely, including the names of named fields. - Also specify that an optimizing compiler may choose to eliminate the field names.
?
2. Also specify that an optimizing compiler may choose to eliminate the field names.
Can we also specify that it may choose to eliminate field values?
Oof, maybe?