2020-08-25 00:48:15 +02:00
|
|
|
# C# Language Design Meeting for July 27th, 2020
|
|
|
|
|
|
|
|
## Agenda
|
|
|
|
|
|
|
|
- [Warnings on types named `record`](#warnings-on-types-named-record)
|
|
|
|
- [`base` calls on parameterless `record`s](#base-calls-on-parameterless-records)
|
|
|
|
- [Omitting unnecessary synthesized `record` members](#omitting-unnecessary-synthesized-record-members)
|
|
|
|
- [`record` `ToString` behavior review](#record-tostring-behavior-review)
|
|
|
|
- [Behavior of trailing commas](#behavior-of-trailing-commas)
|
|
|
|
- [Handling stack overflows](#handling-stack-overflows)
|
|
|
|
- [Should we omit the implementation of `ToString` on `abstract` records](#should-we-omit-the-implementation-of-tostring-on-abstract-records)
|
|
|
|
- [Should we call `ToString` prior to `StringBuilder.Append` on value types](#should-we-call-tostring-prior-to-stringbuilder.append-on-value-types)
|
|
|
|
- [Should we try and avoid the double-space in an empty record](#should-we-try-and-avoid-the-double-space-in-an-empty-record)
|
|
|
|
- [Should we try and make the typename header print more economic](#should-we-try-and-make-the-typename-header-print-more-economic)
|
|
|
|
- [Reference equality short circuiting](#reference-equality-short-circuiting)
|
|
|
|
|
|
|
|
## Quote of the Day
|
|
|
|
|
|
|
|
"He was hung up on his principles!"
|
|
|
|
|
|
|
|
"I painted my tower all ivory."
|
|
|
|
|
|
|
|
## Discussion
|
|
|
|
|
|
|
|
### Warnings on types named `record`
|
|
|
|
|
|
|
|
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
|
2020-08-25 17:04:19 +02:00
|
|
|
record the outcome of the warning discussion. We also discussed going further here: we could make lower-cased
|
2020-08-25 00:48:15 +02:00
|
|
|
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.
|
|
|
|
|
|
|
|
#### Conclusion
|
|
|
|
|
|
|
|
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.
|
|
|
|
|
|
|
|
### `base` calls on parameterless `record`s
|
|
|
|
|
|
|
|
We have a small hole in records today where this is not legal, and there is no workaround to making it legal:
|
|
|
|
|
|
|
|
```cs
|
|
|
|
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).
|
|
|
|
|
|
|
|
#### Conclusion
|
|
|
|
|
|
|
|
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.
|
|
|
|
|
|
|
|
### Omitting unnecessary synthesized `record` members
|
|
|
|
|
|
|
|
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.
|
|
|
|
|
|
|
|
#### Conclusion
|
|
|
|
|
|
|
|
There are likely more downsides than upsides to doing something different here. We'll continue to have this
|
|
|
|
be regular.
|
|
|
|
|
|
|
|
### `record` `ToString` behavior review
|
|
|
|
|
|
|
|
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.
|
|
|
|
|
|
|
|
#### Behavior of trailing commas
|
|
|
|
|
|
|
|
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.
|
|
|
|
|
|
|
|
##### Conclusion
|
|
|
|
|
|
|
|
No trailing commas.
|
|
|
|
|
|
|
|
#### Handling stack overflows
|
|
|
|
|
|
|
|
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.
|
|
|
|
|
|
|
|
##### Conclusion
|
|
|
|
|
|
|
|
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.
|
|
|
|
|
|
|
|
#### Quoting/escaping values
|
|
|
|
|
|
|
|
Today, records do not attempt to escape `string`s 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.
|
|
|
|
|
|
|
|
##### Conclusion
|
|
|
|
|
|
|
|
We won't do any quoting or escaping of string record members. We should make the debugger better here.
|
|
|
|
|
|
|
|
#### Should we omit the implementation of `ToString` on `abstract` records
|
|
|
|
|
|
|
|
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.
|
|
|
|
|
|
|
|
##### Conclusion
|
|
|
|
|
|
|
|
Keep it.
|
|
|
|
|
|
|
|
#### Should we call `ToString` prior to `StringBuilder.Append` on value types
|
|
|
|
|
|
|
|
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.
|
|
|
|
|
|
|
|
##### Conclusion
|
|
|
|
|
|
|
|
Do it.
|
|
|
|
|
|
|
|
#### Should we try and avoid the double-space in an empty record
|
|
|
|
|
|
|
|
The implementation currently prints `Person { }` for an empty record. This provokes immediate "unfinished"
|
|
|
|
reactions in the LDT, similar to the trailing commas above.
|
|
|
|
|
|
|
|
##### Conclusion
|
|
|
|
|
|
|
|
Should we remove it? YES! YES! YES! YES!
|
|
|
|
|
|
|
|
#### Should we try and make the typename header print more economic
|
|
|
|
|
|
|
|
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.
|
|
|
|
|
|
|
|
##### Conclusion
|
|
|
|
|
|
|
|
Up to implementors. However, we don't believe it to be a good idea or worth any effort.
|
|
|
|
|
|
|
|
### Reference equality short circuiting
|
|
|
|
|
|
|
|
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.
|
|
|
|
|
|
|
|
#### Conclusion
|
|
|
|
|
|
|
|
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.
|