Support nullable annotations on unconstrained type parameters

[cston]

Support ? annotations on unconstrained type parameter references:

• ? annotation is allowed for type parameters that are not constrained to reference types or value types
• ? annotation is an error when used on unconstrained type parameter with /langversion:8

Support where T : default constraint:

• default constraint is allowed for method type parameters in overridden or explicitly implemented methods to disambiguate between overloads defined in the base or interface
• default constraint is an error with /langversion:8
• default constraint is an error when used outside of method overrides or explicit implementations
• default constraint is an error when the type parameter is constrained

Fixes #29146

[jaredpar]

Think we should move this comment to a <remarks> section. #Resolved

[jaredpar]

Consider asValueType. #Resolved

[jaredpar]

Suggested change

	return TypeWithAnnotations.Create(Type, NullableAnnotation.Annotated);
	return TypeWithAnnotations.Create(null, NullableAnnotation.Annotated);

Why do we unconditionally use NullableAnnotation.Annotated here for null types? #Resolved

[jaredpar]

Why is the final one not T! F0<T>()? #ByDesign

[cston]

The behavior of SymbolDisplayMiscellaneousOptions.IncludeNotNullableReferenceTypeModifier is unchanged. The type could be reported as T! here, and in T F8<T>() where T : notnull, independent of this PR.

---

In reply to: 455921736 [](ancestors = 455921736)

[jaredpar]

Why do we prefer NotAnnotated here over Oblivious? #Resolved

[cston]

I've removed the explicit Type is null case so the behavior should match the current behavior which returns this.NullableAnnotation. That said, I suspect NullableAnnotation is typically NotAnnotated when Type is null since that is the default value for the struct.

---

In reply to: 456081013 [](ancestors = 456081013)

[jaredpar]

Suggested change

	[InlineData(false)]
	[InlineData(true)]
	[CombinatorialData]
	``` #Resolved

[AlekseyTs]

Please use language version '{0}' [](start = 102, length = 34)

I think we should simply use regular language version message with specific feature name, etc. This way IDE will be able to suggest the change language version fix. #Closed

[cston]

This message has helpful suggestions for using T? in C#8 which the general language version message would not. We can update the IDE to recognize this ErrorCode if necessary.

---

In reply to: 456600909 [](ancestors = 456600909)

[AlekseyTs]

We can update the IDE to recognize this ErrorCode if necessary.

Let's do that then. I think we already have a mechanism to communicate to IDE that it should offer an upgrade to specific language version. CC @jcouv

---

In reply to: 456613552 [](ancestors = 456613552,456600909)

[AlekseyTs]

default type parameter constraints [](start = 11, length = 34)

It feels like the name of the feature isn't very clear, especially that default is used in not very common cases. For example, a user typed T? and compiler reports that "default type parameter constraints" aren't supported by the language version. What constraints the message is talking about? This isn't a constraint. #Closed

[AlekseyTs]

Perhaps we need another feature name for T?

---

In reply to: 456602711 [](ancestors = 456602711)

[cston]

This message is only reported for uses of where T : default. For uses of T?, we continue to report ErrorCode.ERR_NullableUnconstrainedTypeParameter.

---

In reply to: 456603253 [](ancestors = 456603253,456602711)

[AlekseyTs]

_syntaxFactory.DefaultConstraint(defaultToken) [](start = 27, length = 46)

Should we check language version here? #Closed

[cston]

The version check is in BindTypeParameterConstraints().

---

In reply to: 456659327 [](ancestors = 456659327)

[AlekseyTs]

The version check is in BindTypeParameterConstraints().

I realized that. The question is why? Usually, for simple cases like that we perform language version checks in the parser.

---

In reply to: 456693574 [](ancestors = 456693574,456659327)

[AlekseyTs]

case MessageID.IDS_FeatureDefaultTypeParameterConstraint: [](start = 16, length = 57)

Consider adding a comment if this is not a parsing time check #Closed

[AlekseyTs]

IDS_FeatureDefaultTypeParameterConstraint [](start = 48, length = 41)

This is somewhat confusing because default constraint isn't involved. #Closed

[cston]

Added a comment.

---

In reply to: 456677882 [](ancestors = 456677882)

[AlekseyTs]

return type is object && [](start = 12, length = 24)

It looks like we dropped IsNotAnnotated check. Is this intentional? Could you please elaborate? #Closed

[cston]

It looks like we dropped IsNotAnnotated check. Is this intentional?

Yes. Callers above are explicitly checking NullableAnnotation, and requiring NullableAnnotation == NullableAnnotation.NotAnnotated here would mean T and T? would be considered identical in HasTopLevelNullabilityIdentityConversion(). For instance, that would allow assigning IEnumerable<T?> to IEnumerable<T> in UnconstrainedTypeParameter_21().

---

In reply to: 456696256 [](ancestors = 456696256)

[jaredpar]

The spanish failure is in UpdaterService which is flaky. Will filean issue shortly. Merging.

[mysteryx93]

Why was T? on unconstrained types implemented as meaning ValueOrDefault instead of ValueOrNull?

For ValueOrNull, we're still back to the old method of duplicating classes with struct/class constraints.

[RikkiGibson]

Could you please share some specific scenarios that are negatively affected by this?

[mysteryx93]

The following code compiles but doesn't behave as one would expect. Previously it would fail to compile, now it just doesn't do what I'd expect it to.

public enum LogLevel { None, Debug, Info }

public interface IFormatter<T>
{
    T Parse(string? value);
    string? Format(T value);
}

public class PropertyBase<T> : IFormatter<T?>
{
    public virtual T? Parse(string? value) => default!;
    public virtual string? Format(T? value) => null;
}

public class PropertyRead<T> : PropertyBase<T>
{ }

public class PropertyReadWrite<T> : PropertyRead<T>
{ }

public class PropertyEnum<T> : PropertyRead<T>
    where T : struct, Enum
{ }

public class MyClass
{
    public void DoIt()
    {
        var f = new PropertyEnum<LogLevel>();
        var value = f.Parse(null); // non-nullable!
    }
}

The solution is to write it like this

public enum LogLevel { None, Debug, Info }

public interface IFormatter<T>
{
    T Parse(string? value);
    string? Format(T value);
}

public class PropertyBase<TNull> : IFormatter<TNull>
{
    public virtual TNull Parse(string? value) => default!;
    public virtual string? Format(TNull value) => null;
}

public class PropertyRead<T> : PropertyBase<T?>
    where T : struct
{ }

public class PropertyReadRef<T> : PropertyBase<T?>
    where T : class
{ }

public class PropertyReadWrite<T> : PropertyRead<T>
    where T : struct
{ }

public class PropertyReadWriteRef<T> : PropertyReadRef<T>
    where T : class
{ }

public class PropertyEnum<T> : PropertyRead<T>
    where T : struct, Enum
{ }

public class MyClass
{
    public void DoIt()
    {
        var f = new PropertyEnum<LogLevel>();
        var value = f.Parse(null); // nullable!
    }
}

This solution however requires a lot of code copy/paste, and inheritance references can get complicated, especially when the classes contain considerable code.

And the syntax is weird for string: PropertyReadRef<string>, which leads me to creating a shortcut class PropertyReadString