Ambiguous implements/overrides with generic methods and NRTs

[gafter]

See also #2370. This is a proposal for how to address the issue raised there.

There is a language ambiguity introduced by the addition of nullable reference types to C#. The scenario is an explicit interface implementation, or a method override, where the constraint does not appear on the implementation, so the compiler does not know at the point where the implementation was written whether the type parameters are constrained to be value types or constrained to be reference types. Consequently the compiler might match the implementation to more than one interface member, where it previously matched precisely one interface member.

Here is a motivating example:

interface I
{
    void Foo<T>(T value) where T : class;
    void Foo<T>(T? value) where T : struct;
}

class C2 : I
{
    void I.Foo<T>(T? value) { }
}

This implementation, as written, is a valid for either the first method (with a warning about mismatched nullable annotation on the first parameter) or the second method in C# 8. But in C# 7 it was only valid for the second implementation. The compiler only looks for one method, so it picks the first. That is incompatible with what older compilers did, as they only were able to match the second overload. Consequently this is a breaking change.

We want the language to satisfy the following requirements:

1. When compiling a valid C# 7 program, we should behave as the C# 7 language specification requires.
2. When compiling the same source as a C# 8 program, we should produce compatible behavior (with possibly nullable warnings)
3. It would be nice if there were a way to implement both of the methods in the following

interface I
{
    void Foo<T>(T? value) where T : class;
    void Foo<T>(T? value) where T : struct;
}

There are a number of ways to accomplish this.

Option 1

We extend the syntax of implementations by permitting the class constraint (only) on type parameters of the implementing method. When matching the implementation with possible implemented methods, T? is interpreted to mean Nullable<T> if T lacks a constraint in the implementation. However, if T has the class constraint then T? is interpreted to mean a nullable reference type . In addition to the usual OHI matching rules, we would require that any type parameter that has the class constraint in the implementation must correspond to a type parameter in the implemented method that is known to be a non-nullable reference type.

For the user's convenience, this would come with a couple of additional IDE helpers:

1. If the user gets a warning the the nullable annotations on the implementing method do not match those on the implemented method, a code fixer would add both the necessary nullable annotations and the necessary class constraints.
2. When the user implements an interface method, the code generation that the IDE produces for the implementation would have any necessary class constraints for type parameters that appear with a nullable annotation in the signature.

Option 2

This is like option 1, but in addition we would permit a struct constraint in the implementation. Where a struct constraint appears on a type parameter in the implementing method, we would require that the implemented method have a corresponding type parameter that is known to be a non-nullable value type.

We could also require (softly, with a warning) that every type parameter that is annotated with a ? in the signature of an implementation have either a struct or a class constraint. If a T? appears without either constraint, we would treat it as Nullable<T> and produce a warning. The IDE would offer to fix the warning by adding where T: struct.

Option 3

Do not extend the syntax of implementations by permitting constraints, but look for a matching interface method in two passes. In the first pass, T? (where T is a type parameter of the method) is interpreted as meaning Nullable<T>. Only if we find no matching method in the first pass do we try a second pass, where T? may match either Nullable<T> or T? (meaning nullable reference type). This option accomplishes requirements (1) and (2) but not (3).

[YairHalberstadt]

As I see it, options 1 and 2 have the issue that the constraint on the base type may be a more specific object than class (eg where T : Stream), and it would seem odd that to match it you are required to have the constraint class on the explicit implementation, but can't use Stream.

---

Rules 1, 2 and 3 feel like they are special casing NRTs in a rather arbitrary way. For example given

#nullable enable
interface I
{
    void Foo<T>(T? value) where T : class;
    void Foo<T>(T? value) where T : struct;
}

class C2 : I
{
    void I.Foo<T>(T? value) { }
}

Both 1, 2, and 3 would resolve this explicit implementation as matching the one with the struct constraint, but that's not at all obvious from looking at the code.

An alternative to 3 that I suggested is to do two passes. In the first we only match on an interface method if it's nullability annotations match, and in the second we match even if doesn't. If there are multiple matches in either pass, we warn.

This achieves aims 1 and 2. On the other hand it leaves some cases as implementation defined behaviour where 3 wouldn't (These cases are only possible in C# 8, so would not break backwards compatibility). However explicit implementations can already be ambiguous, and it may be better to leave the behaviour ambiguous and provide a warning, than choose a seemingly arbitrary rule to distinguish them.

[gafter]

We discussed this 2019-04-03 at the LDM. What we came away with are the following resolutions:

1. In an override or implementation (that lacks constraints), T? would always mean Nullable<T>. This "fix" will be implemented ASAP to fix the product compatibility bug. The other steps below would be done separately.
2. If you want to use a nullable reference type annotation on a type parameter (e.g. T?) in an override or implementation, you would be required to write the constraint where T: class in the override or implementation. The ability to write that constraint is a new feature.
3. We would also allow where T: struct in an override or implementation for symmetry.

If you write such a constraint (where T: struct or where T: class) on an override or implementation, such constraints are checked that they are implied by the constraints on the method being implemented or overridden.

The IDE would conveniently insert appropriate constraints when emitting code for implementing or overriding a method.

This is essentially Option 2 above (without the second paragraph).

[CyrusNajmabadi]

The IDE would conveniently insert appropriate constraints when emitting code for implementing or overriding a method.

This makes sense to me. When this work is done on the compiler side, can you please loop in IDE to make sure this happens on the IDE side? Thanks!

[YairHalberstadt]

I implemented the fix for this slightly differently, but I'm happy to change that PR to implement the fix the way the LDM decided.

[YairHalberstadt]

@gafter Should this be closed as fixed?