186 lines
13 KiB
Markdown
186 lines
13 KiB
Markdown
# 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.
|