# 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
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.

#### 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.