13 KiB
C# Language Design Meeting for July 27th, 2020
Agenda
- Improved nullable analysis in constructors
- Equality operators in records
.ToString()
orGetDebuggerDisplay()
on records- 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."
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:
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:
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:
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
record
generates a ToString that prints out its type name followed by a positional syntax: Point(X: 1, Y: 2). (issue: disfavors nominal records)record
generates a ToString that does the above and also appends nominal properties by calling ToStringNominal:FirstName: First, LastName: Last
, and assembles the parts intoPerson { 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 thingtm". 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 sincedata
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 displayprivate
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 asprotected
fields, show up in theToString
, and violates encapsulation principles. - All
public
andinternal
members. This propsal has some of the same issues as the previous one: you could making aninternal
type that implements apublic
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
wasinternal
, then internal members would be shown. If the record waspublic
, then onlypublic
members would be shown. This doesn't solve any of our issues with the previous proposal, and adds a behavioral difference between making a typepublic
orinternal
. - 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 ofinternal
orprivate
state that want to display these in aToString
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.