- Warnings on types named
record basecalls on parameterlessrecords- Omitting unnecessary synthesized
recordmembers recordToStringbehavior review- Behavior of trailing commas
- Handling stack overflows
- Should we omit the implementation of
ToStringonabstractrecords - Should we call
ToStringprior toStringBuilder.Appendon value types - Should we try and avoid the double-space in an empty record
- Should we try and make the typename header print more economic
- Reference equality short circuiting
"He was hung up on his principles!"
"I painted my tower all ivory."
We previously discussed warning in C# 9 when a type is named record, as it will need to be escaped in field
definitions in order to parse correctly. This is a breaking change being made in C# 9, but we didn't explicitly
record the outcome of the warning discussion. We also discussed going further here: we could make lower-cased
type names a warning with the next warning wave, under the assumption that lowercase names make good keywords.
For example, we could start warning on types named async or var. Those who want to forbid usage of those
features in their codebase have a solution in analyzers.
We will warn on types named record in C# 9. Further discussion will need to happen for lowercase type names
in general, as there are globalization considerations, and we feel that disallowing record might be a good
smoke test for reception to the idea in general.
We have a small hole in records today where this is not legal, and there is no workaround to making it legal:
record Base(int a);
record Derived : Base(1);The spec as written today explicitly forbids this: "It is an error for a record to provide a record_base
argument_list if the record_declaration does not contain a parameter_list." However, there is good
reason to allow this, and while this design may have fallen out of the principles that were set out given
that a default constructor for a nominal record is not recognized as a primary constructor, this seems to
violate principles around generally making record types smaller. Further, with the spec as written today
it's not possible to work around this issue by giving Derived an empty parameter list, as parameter lists
are not permitted to be empty.
A solution to this problem is relatively straightforward: we make the default constructor of a nominal record
the primary constructor, if no other constructor was provided. This seems to mesh well with the existing rules,
and seems to work well with future plans for generalized primary constructors. We could consider 2 variants of
this: requiring empty parens on the type declaration in all cases, or only requiring them in cases where the
default constructor would otherwise be omitted (if another constructor was added in the record definition).
We'll allow empty parens as a constructor in C# 9. We're also interested in allowing the parens to be omitted when a default constructor would otherwise be emitted, but if this doesn't make it in C# 9 due to time constraints it should be fine to push back to v next.
Some record members might be able to be omitted in certain scenarios if they're unneeded. For example, a
sealed record that derives from System.Object would not need an EqualityContract property, as it
can only have the one contract. We could also simplify the implementation of ToString if we know there
will be no child-types that need to call PrintMembers.
However, despite leading to simplified emitted code for these types, we're concerned that this will cause other code to become more complicated. We'll need to have more complicated logic in the compiler to handle these cases. Further, source generators and other tools that interact with records programmatically will have to duplicate this logic if they need to interact with record equality or any other portion of code that "simplified" by this mechanism. We further don't see a real concern for metadata bloat in this scenario, as we aren't omitted fields from the record declaration.
There are likely more downsides than upsides to doing something different here. We'll continue to have this be regular.
Now that we have a proposal and implementation for ToString on record types, we'll go over some standing
questions from initial implementation review. Prior art for this area in C# that we found relevant was
ToString on both tuples and anonymous types.
There is a proposal to simplify the implementation of ToString by always printing trailing commas after
properties. This would remove the need for some bool tracking flags for whether a comma was printed as
part of a parent's ToString call. However, no prior art (in either C# or other languages) that we could
find does this. Further, it provokes a visceral reaction among team members as feeling unfinished, and that
it would be the first bug report we get after shipping it.
No trailing commas.
There is some concern that ToString could overflow if a record type is recursive. While this is also true
of equality and hashcode methods on records, there is some concern that ToString might be a bit special
here, as it's the tool that a developer would use to get a print-out of the record to find the cyclic element
in the first place. There's examples in the javascript world of this being a pain point. However, any
solution that we come up with for records will have limitations: it wouldn't be able to protect against
indirect cycles through properties that are records but not records of the same type, or against cycles
in properties that are not records at all. In general, if a user makes a record type with a cyclic data
structure, this is a problem they're going to have to confront already in equality and hashcode.
We will not attempt any special handling here. We could consider adding a DebuggerDisplay with more special,
reflective handling at a later point in time if this proves to be a pain point, but we don't think it's
worth the cost in regular ToString calls.
Today, records do not attempt to escape strings when printing, which could result in potentially confusing
printing. However, we do have concerns here. First, none of our prior art in C# does this escaping. Second,
the framework also does not escape ToString() like this. Third, there's a question of what type of
escaping we should do here. Would we want to print a newlines as the C# version, \n, or the VB version?
Further, we already have some handling the expression evaluator and debugger for regular strings and
anonymous types, to ensure that the display looks good there. Given that this is mainly about ensuring
that records appear well-formatted in the debugger, it seems like the correct and language-agnostic approach
is to ensure the expression evaluator knows about record types as well.
We won't do any quoting or escaping of string record members. We should make the debugger better here.
We could theoretically omit providing an implementation of this method on abstract record types as it
will never be called by the derived ToString implementation. On the face of it, we cannot think of a
scenario that would be particularly helped by including the ToString, however we also cannot think of
a scenario that would be harmed by including it, and doing so would make the compiler implementation simpler.
Keep it.
The concern with directly calling Append without ToString on the value being passed is that for
non-primitive struct types, this will cause the struct to be boxed, and the behavior of Append is
to immediately call ToString and pass to the string overload.
Do it.
The implementation currently prints Person { } for an empty record. This provokes immediate "unfinished"
reactions in the LDT, similar to the trailing commas above.
Should we remove it? YES! YES! YES! YES!
Today, we call StringBuilder.Append(nameof(type)) and StringBuilder.Append("{") immediately after
as 2 separate calls, which is another method call we could drop if we did it all in one. We feel that this
is sufficiently outside the language spec as to be a thing a compiler could do if it feels it appropriate.
From the compiler perspective, however, it could actually be bad change, as it would increase the size of
the string table, which is known to be an issue in some programs, while getting rid of one extra method call
in a method not particularly designed to be as fast as possible in the first place.
Up to implementors. However, we don't believe it to be a good idea or worth any effort.
Today, we have a slight difference between the equality operators ==/!= and the implementation of the
Equals(record) instance method, in that the former compare reference equality first, before delegating
to the Equals method. This ensures that null is equal to null as a very quick check, and ensures we
only have to compare one operand to null explicitly in the implementation of the operators. The Equals
instance member then does not check this condition. However, this means that the performance characteristics
of these two members can be very different, with the operators being an order of magnitude faster on even
small record types since it has to do potentially many more comparisons. The proposal, therefore, is to
check reference equality in the Equals instance member as well, as the first element.
We will do this. We'll keep the reference equality check in the operators as well to ensure that null == null,
and add one at the start of the instance method. It does mean that the == operators will check reference
equality twice, but this is an exceptionally quick check so we don't believe it's a big concern.