# C# Language Design Meeting for July 27th, 2020

## Agenda

- [Improved nullable analysis in constructors](#Improved-nullable-analysis-in-constructors)
- [Equality operators in records](#Equality-operators-in-records)
- [`.ToString()` or `GetDebuggerDisplay()` on records](#ToString-or-DebuggerDisplay-on-record-types)
- [W-warnings for `T t = default;`](#W-warnings-for-`T-t-=-default;`)

## Quote of the Day

"We should pick our poison: one is arsenic, and will kill us. The other is a sweet champagne, and will give us a headache tomorrow."

![Delectable Tea or Deadly Poison](delectable_tea_2020_07_27.png)

## Procedural Note

LDM is going on August hiatus as various members will be out. We'll meet next on August 24th, 2020.

## Discussion

### Improved nullable analysis in constructors

https://github.com/dotnet/csharplang/blob/master/proposals/nullable-constructor-analysis.md

This proposal is a tweak to the nullable warnings we emit today in constructors that would bring these warnings more in line with
behavior for `MemberNotNull`, as well as addressing a few surprising cases where you do not get a deserved nullable warning today.
This code, for example, has no warnings today:
```cs
public class C
{
    public string Prop { get; set; }
    public C() // we get no "uninitialized member" warning, as expected
    {
        Prop.ToString(); // unexpectedly, there is no "possible null receiver" warning here
        Prop = "";
    }
}
```
We consider this to be possible to retrofit into the nullable feature, as we're still within the nullable adoption phase. After
C# 9 is released, however, this would be a breaking change, so we will need to either do it now, or do it in a warning wave
(which would heavily complicate the implementation). Additionally, even though we're still within the nullable adoption phase,
this is a relatively big change in warnings that could result in lots of new errors in codebases that have heavily adopted nullable.
We also considered whether there could be any interference here with C# 10 plans around required properties or initialization debt.
There does not appear to be, as this would effectively be the initialization debt world where a constructor is required to set all
properties in the body, and any relaxation of this by requiring properties to be initialized at construction site will play well.

#### Conclusion:

We should implement the change and smoke test it on roslyn, the runtime repo, and on VS. If there is large churn, we'll re-evaluate
at that point.

### Equality operators in records

https://github.com/dotnet/csharplang/issues/3707#issuecomment-661800278

We've talked briefly about equality operators in records prior to now, but we never ended up making any firm decisions, so now we
need to make an intentional decision about records before we ship as doing so after would be a breaking change. The concern is that,
because class types inherit a default equality operator from simply being a class, we'll get into a scenario where a record's `Equals`
method could return `true`, and the `==` operator returns false. Most standard .NET "values" behave in this fashion as well: `string`
being the most common example.

There are some interesting niggles, however, particularly with `float` and `double` values. For example,
this code:
```cs
var a = (float.NaN, double.NaN);
System.Console.WriteLine(a == a); // Prints false
System.Console.WriteLine(a.Equals(a)); // Prints true
```
This happens because the `==` operator and the `Equals` method for floats and doubles behave differently. The `==` operator uses IEEE
equality, which does not consider `NaN` equal to itself, while `Equals` uses the CLR definition of equality, which does. When combined
with `ValueTuple`, this makes for an interesting set of behaviors, as `ValueTuple` doesn't have its own definition of the `==` operator.
Rather, the compiler synthesizes the set of `==` operators on the component elements of the tuple. This means that for generic cases,
such as `public void M<T>((T, T) tuple) { _ = tuple == tuple; }`, you get an error when attempting to use `==` on the tuple, since `==`
is not defined on generic type parameters. `ValueTuple` is a particularly special case here though. The compiler special cases knowledge
of the implementation, which is not generalizable across inheritance hierarchies with record types. In order for an implementation to do
memberwise `==` across all levels of a record type, there will have to be hidden virtual methods to take care of this, and it gets more
complicated when you consider that a derived record type could introduce a generic type parameter field that can't be `==`'d. We feel
that attempting to get too clever here would end up being unexplainable, so if we want to implement the operators, it should just
delegate to the virtual `Equals` methods we already set up.

Next, we considered whether we should implement `==` operators on all levels of a record inheritance hierarchy, or just the top level.
With records as they're shipping in C# 9, we don't believe that there's a scenario you can get into that would break this. However, the
eventual goal is to enable records and classes to inherit from each other, and that point if every level didn't provide its own
implementation it could end up being possible to break assumptions. Further, attempting to trim implementations of `==` and `!=` would
end up resulting in complicated rules that don't feel necessary: adding the operators isn't going to bloat metadata size, will potentially
allow eliminating of some levels of equality method calls, and future proofs ourselves.

Finally, we looked at whether the user will be able to provide their own implementation of `==` and `!=` operators, and if we'd error
on this or allow it and not generate the operators ourselves in this case. We feel that allowing this would complicate records: there
are existing extension points into record equality that can be overridden as needed, and we want to have a stronger concept of `Equals`
and `==` being the same. If there are good user scenarios around allowing this, we can consider it at a later point.

#### Conclusion

We will generate the `==` and `!=` operators on every level of a record hierarchy. These implementations will not be user-overridable,
and they will delegate to `.Equals`. The implementation will have this semantic meaning:
```cs
pubic static bool operator==(R? one, R? other)
    => (object)one == other || (one?.Equals(other) ?? false);
public static bool operator!=(R? one, R? other)
    => !(one == other);
```

### `ToString` or `DebuggerDisplay` on record types

#### Initial proposal

1. `record` generates a ToString that prints out its type name followed by a positional syntax: Point(X: 1, Y: 2).
(issue: disfavors nominal records)
2. `record` generates a ToString that does the above and also appends nominal properties by calling ToStringNominal:
`FirstName: First, LastName: Last`, and assembles the parts into `Person { FirstName: First, LastName: Last, ID: 42 }`.

#### Discussion

The initial proposal here is to add a `ToString` method that would format a record type by their positional properties, and have a
separate method for creating a string from nominal properties named something like `ToStringNominal`. The initial reaction was that
this seemed overly complicated: there should just be one method that does "the right thing<sup>tm</sup>". There's further question
about whether having a `ToString` or `DebuggerDisplay` would be useful: depending on the properties of the record type, it could end
up doing more harm than good (if the properties were lazy, for example). Additionally, when a `ToString` is provided it's often domain
specific, and nothing we do in the compiler would be able to predict what shape that should be. We ended up coming up with a list of
common scenarios in which a `ToString` would be useful:

* Viewing in the debugger. This could be done with just a `DebuggerDisplay` attribute, or users can just expand the object to view the
properties of it as you can today.
* Viewing in test output. Using an equality test, for example, will usually print the objects if they were not equal to each other,
and it would be useful if that was actually meaningful, particularly for these value-like class types.
* Logging output. This is a very similar case to the previous.

We also considered whether we should implement this via source generators. However, it feels very similar to equality: we have a
sensible default there, and anyone who wants to customize (this field should be excluded, this one should use sequence equality)
can either write it manually or use a source generator. The same arguments apply here: we can provide a sensible default that will
enable short syntax, and anyone who wants to get custom behavior can override this.

After considering _whether_ to do this feature, we looked at what properties would be included in such a feature. How, would they
be formatted, and which ones are considered? The initial proposal was for positional only, with a separate method for nominal
properties. We felt that this was too complicated and likely not to be the right defaults: it totally disadvantages nominal records
and makes the implementation more complicated. The options we considered are:

* The same members as .Equals. This means all fields of a class, translating auto-generated backing fields to their properties for
display names. This would make explaining what appears in the `ToString` simple: it's the equality members, full stop.
* The positional members and `data` members of a record. We feel that this is too restrictive, especially since `data` members will
not be shipping in final form with C# 9: they'll still be under the preview language version.
* The fields and properties of a record type with some accessibility, accessibility to be determined next. We believe that it's very
unusual for a `ToString` to display `private` fields, which the other proposal would do.

We leaned towards just doing the fields and properties of a type, with some accessibility. Finally, we looked at what accessibility
to use by default for these:

* All members not marked `private`. This means that things that not relevant to a consumer of the type, such as `protected` fields,
show up in the `ToString`, and violates encapsulation principles.
* All `public` and `internal` members. This propsal has some of the same issues as the previous one: you could making an `internal`
type that implements a `public` interface, and it would be odd if the final user sees that internal state.
* At least as accessible as the record itself. This would mean that if the `record` was `internal`, then internal members would be
shown. If the record was `public`, then only `public` members would be shown. This doesn't solve any of our issues with the previous
proposal, and adds a behavioral difference between making a type `public` or `internal`.
* Only `public` members. This is what the positional constructor will generate by default, and it feels like the most natural state
to choose. This ensures that encapsulation isn't violated, and records with lots of `internal` or `private` state that want to
display these in a `ToString` are likely not really records anyway. If the user really wants to change this, they can customize it
as they choose.

Conclusion:

We'll have a generated `ToString`, that will display the public properties and fields of a `record` type. We'll come back with a
specific proposal on how this will be implemented at a later point.

### W-warnings for `T t = default;`

Just after we initially shipped C# 8, we made a change to warn whenever `default` was assigned to a local or property of type `T`,
or when a default was cast to that type. There was no way to silence this warning without using a `!`, so we reverted the change.
However, now that we are shipping `T?`, there is an actual syntax that users can write in order to express "this location might
be assigned default". We know from experience that this is something that users already do a lot, so making a change here would be
pretty breaking, even for the nullable rollout phase of the first year. Further, as we know that if the `!` is the only way to get
rid of this warning then we'll get lots of feedback from users, we believe that we can't unconditionally warn here: you can only
get rid of the warning in C# 9, so at a minimum it should only be reported in C# 9. We could also put this in a warning wave, and
you would only get it when you opt into the warning wave. An important thing to consider with doing this as a warning wave implies
that it should be separate error code from 8600: we've previously told people that if they don't want to annotate all their local
variables with `?`s, then they should disable that warning and all the rest of the warnings will be actual safety warnings. This
would add another warning to that list to disable. We could make it memorable as well, but one of the key aspects in warning waves
is that you should be able to opt out of individual warnings if necessary, so users will need to be able to opt out of this new
warning without turning off all the existing 8600 warnings. All that being said, there's also a serious backcompat concern here,
as introducing the warning when upgrading to C# 9 under the existing error code could cause people to hold off on upgrading at all.

#### Conclusion

We'll add a new warning here, under a warning wave.
