Skip to content

Latest commit

 

History

History
186 lines (148 loc) · 13.2 KB

LDM-2020-07-27.md

File metadata and controls

186 lines (148 loc) · 13.2 KB
Raw

C# Language Design Meeting for July 27th, 2020

Agenda

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

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

#3707 (comment)

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

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