meetings/2020/LDM-2020-07-27.md
.ToString() or GetDebuggerDisplay() on recordsT t = default;"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."
LDM is going on August hiatus as various members will be out. We'll meet next on August 24th, 2020.
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.
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.
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.
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 typesrecord 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 into Person { FirstName: First, LastName: Last, ID: 42 }.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:
DebuggerDisplay attribute, or users can just expand the object to view the
properties of it as you can today.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:
ToString simple: it's the equality members, full stop.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.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:
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.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.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.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.
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.
We'll add a new warning here, under a warning wave.