Merge pull request #3910 from sailro/fix-3909-default-constraint

Fix #3909: emit nullable override disambiguators

ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs

@@ -663,6 +663,12 @@ namespace ICSharpCode.Decompiler.Tests
 			await RunForLibrary(cscOptions: cscOptions);
 		}
 
+		[Test]
+		public async Task Issue3909([ValueSource(nameof(roslyn3OrNewerOptions))] CompilerOptions cscOptions)
+		{
+			await RunForLibrary(cscOptions: cscOptions | CompilerOptions.NullableEnable);
+		}
+
 		[Test]
 		public async Task Issue3452([ValueSource(nameof(roslyn4OrNewerOptions))] CompilerOptions cscOptions)
 		{

ICSharpCode.Decompiler.Tests/TestCases/Pretty/Issue3909.cs

@@ -0,0 +1,275 @@
+using System;
+using System.Collections.Generic;
+
+#nullable enable
+namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
+{
+	internal class Issue3909
+	{
+		public abstract class Base
+		{
+			public virtual T? Unconstrained<T>(T? value)
+			{
+				return value;
+			}
+
+			public virtual T? ReferenceType<T>(T? value) where T : class
+			{
+				return value;
+			}
+
+			public virtual T? ReferenceTypeNullable<T>(T? value) where T : class?
+			{
+				return value;
+			}
+
+			public virtual T? NotNull<T>(T? value) where T : notnull
+			{
+				return value;
+			}
+
+			public virtual T? ValueType<T>(T? value) where T : struct
+			{
+				return value;
+			}
+
+			public virtual U? ReturnOnly<U>()
+			{
+				return default(U);
+			}
+
+			public virtual T?[] Nested<T>(T?[] values)
+			{
+				return values;
+			}
+
+			public virtual T Identity<T>(T value)
+			{
+				return value;
+			}
+		}
+
+		public sealed class Derived : Base
+		{
+			public override T? Unconstrained<T>(T? value) where T : default
+			{
+				return value;
+			}
+
+			public override T? ReferenceType<T>(T? value) where T : class
+			{
+				return value;
+			}
+
+			public override T? ReferenceTypeNullable<T>(T? value) where T : class
+			{
+				return value;
+			}
+
+			public override T? NotNull<T>(T? value) where T : default
+			{
+				return value;
+			}
+
+			public override T? ValueType<T>(T? value)
+			{
+				return value;
+			}
+
+			public override U? ReturnOnly<U>() where U : default
+			{
+				return default(U);
+			}
+
+			public override T?[] Nested<T>(T?[] values) where T : default
+			{
+				return values;
+			}
+
+			public override T Identity<T>(T value)
+			{
+				return value;
+			}
+		}
+
+		public interface IRoundTrip
+		{
+			T? RoundTrip<T>(T? value);
+		}
+
+		public class ExplicitImpl : IRoundTrip
+		{
+			T? IRoundTrip.RoundTrip<T>(T? value) where T : default
+			{
+				return value;
+			}
+		}
+
+		public class Node
+		{
+		}
+
+		public abstract class ConstrainedBase
+		{
+			public virtual T? ClassType<T>(T? value) where T : Node
+			{
+				return value;
+			}
+
+			public virtual T? DelegateType<T>(T? value) where T : Delegate
+			{
+				return value;
+			}
+
+			public virtual T? EnumType<T>(T? value) where T : Enum
+			{
+				return value;
+			}
+
+			public virtual T? InterfaceType<T>(T? value) where T : IDisposable
+			{
+				return value;
+			}
+
+			public virtual List<T?> NestedGeneric<T>(List<T?> values)
+			{
+				return values;
+			}
+
+			public virtual TItem? PartiallyAnnotated<TItem, TOther>(TItem? value, TOther other)
+			{
+				return value;
+			}
+		}
+
+		public sealed class ConstrainedDerived : ConstrainedBase
+		{
+			public override T? ClassType<T>(T? value) where T : class
+			{
+				return value;
+			}
+
+			public override T? DelegateType<T>(T? value) where T : class
+			{
+				return value;
+			}
+
+			public override T? EnumType<T>(T? value) where T : default
+			{
+				return value;
+			}
+
+			public override T? InterfaceType<T>(T? value) where T : default
+			{
+				return value;
+			}
+
+			public override List<T?> NestedGeneric<T>(List<T?> values) where T : default
+			{
+				return values;
+			}
+
+			public override TItem? PartiallyAnnotated<TItem, TOther>(TItem? value, TOther other) where TItem : default
+			{
+				return value;
+			}
+		}
+
+		public class ClassTypeChainBase<TOuter> where TOuter : Node
+		{
+			public virtual T? Chained<T>(T? value) where T : TOuter
+			{
+				return value;
+			}
+		}
+
+		public sealed class ClassTypeChainDerived<TOuter> : ClassTypeChainBase<TOuter> where TOuter : Node
+		{
+			public override T? Chained<T>(T? value) where T : class
+			{
+				return value;
+			}
+		}
+
+		public class ReferenceTypeChainBase<TOuter> where TOuter : class
+		{
+			public virtual T? Chained<T>(T? value) where T : TOuter
+			{
+				return value;
+			}
+		}
+
+		public sealed class ReferenceTypeChainDerived<TOuter> : ReferenceTypeChainBase<TOuter> where TOuter : class
+		{
+			public override T? Chained<T>(T? value) where T : default
+			{
+				return value;
+			}
+		}
+
+		public abstract class MethodChainBase
+		{
+			public virtual T? DependentOnClassConstrained<T, U>(T? value, U other) where T : U where U : class
+			{
+				return value;
+			}
+
+			public virtual T? DependentOnClassType<T, U>(T? value, U other) where T : U where U : Node
+			{
+				return value;
+			}
+		}
+
+		public sealed class MethodChainDerived : MethodChainBase
+		{
+			public override T? DependentOnClassConstrained<T, U>(T? value, U other) where T : default
+			{
+				return value;
+			}
+
+			public override T? DependentOnClassType<T, U>(T? value, U other) where T : class
+			{
+				return value;
+			}
+		}
+
+		public class ContainerBase<TOuter> where TOuter : class
+		{
+			public virtual TOuter? Pick<TItem>(TItem item)
+			{
+				return null;
+			}
+		}
+
+		public sealed class ContainerDerived<TOuter> : ContainerBase<TOuter> where TOuter : class
+		{
+			public override TOuter? Pick<TItem>(TItem item)
+			{
+				return null;
+			}
+		}
+#if CS130
+		public class BaseWithAllowsRefStruct
+		{
+			public virtual void Annotated<T>(T? value) where T : allows ref struct
+			{
+			}
+
+			public virtual void Plain<T>(T value) where T : allows ref struct
+			{
+			}
+		}
+
+		public class DerivedWithAllowsRefStruct : BaseWithAllowsRefStruct
+		{
+			public override void Annotated<T>(T? value) where T : default
+			{
+			}
+
+			public override void Plain<T>(T value)
+			{
+			}
+		}
+#endif
+	}
+}

ICSharpCode.Decompiler/CSharp/Syntax/TypeSystemAstBuilder.cs

@@ -2342,13 +2342,24 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
 			if (method.IsExtensionMethod && method.ReducedFrom == null && decl.Parameters.Any())
 				decl.Parameters.First().HasThisModifier = true;
 
-			if (this.ShowTypeParameters && this.ShowTypeParameterConstraints && !method.IsOverride && !method.IsExplicitInterfaceImplementation)
+			if (this.ShowTypeParameters && this.ShowTypeParameterConstraints)
 			{
-				foreach (ITypeParameter tp in method.TypeParameters)
+				if (method.IsOverride || method.IsExplicitInterfaceImplementation)
 				{
-					var constraint = ConvertTypeParameterConstraint(tp);
-					if (constraint != null)
-						decl.Constraints.Add(constraint);
+					// C# inherits the constraints of an override or explicit interface
+					// implementation from the base member and forbids restating them, with a
+					// single exception: a 'class', 'struct', or 'default' constraint may be given
+					// to disambiguate whether 'T?' denotes a nullable annotation or Nullable<T>.
+					AddNullabilityDisambiguatingConstraints(decl, method);
+				}
+				else
+				{
+					foreach (ITypeParameter tp in method.TypeParameters)
+					{
+						var constraint = ConvertTypeParameterConstraint(tp);
+						if (constraint != null)
+							decl.Constraints.Add(constraint);
+					}
 				}
 			}
 			decl.Body = GenerateBodyBlock();
@@ -2608,6 +2619,68 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
 			return c;
 		}
 
+		// A disambiguator is required only where the type parameter itself carries a nullable
+		// annotation ('T?') in the signature: without it the compiler reads 'T?' as Nullable<T>.
+		// The inherited constraints are re-emitted on the override's own type parameters in
+		// metadata, so which disambiguator is legal follows from them without resolving the base
+		// member. The restated disambiguator leaves no metadata trace of its own, hence it must be
+		// derived rather than read back.
+		// The clause is built here rather than through ConvertTypeParameterConstraint, which also
+		// prints 'allows ref struct' from the byreflike flag. That flag is re-emitted on the
+		// override's own type parameter as well, and restating it is CS0460.
+		void AddNullabilityDisambiguatingConstraints(MethodDeclaration decl, IMethod method)
+		{
+			if (method.TypeParameters.Count == 0)
+				return;
+			NullableTypeParameterCollector collector = new(method.TypeParameters);
+			method.ReturnType.AcceptVisitor(collector);
+			foreach (IParameter p in method.Parameters)
+				p.Type.AcceptVisitor(collector);
+			if (collector.NullableTypeParameters.Count == 0)
+				return;
+			foreach (ITypeParameter tp in method.TypeParameters)
+			{
+				if (!collector.NullableTypeParameters.Contains(tp) || GetNullabilityDisambiguator(tp) is not string keyword)
+					continue;
+				Constraint c = new();
+				c.TypeParameter = MakeSimpleType(tp.Name);
+				c.BaseTypes.Add(new PrimitiveType(keyword));
+				decl.Constraints.Add(c);
+			}
+		}
+
+		// Returns the constraint that keeps 'T?' meaning a nullable annotation on an override or
+		// explicit interface implementation, or null where the type parameter neither needs nor
+		// permits one.
+		static string? GetNullabilityDisambiguator(ITypeParameter tp) => tp.IsReferenceType switch {
+			// C# accepts only plain 'class' here, never 'class?'; the constraint's own nullability
+			// is inherited from the base member regardless.
+			true => "class",
+			// Constrained to neither a reference type nor a value type.
+			null => "default",
+			// A value type uses Nullable<T> rather than a nullable annotation.
+			false => null
+		};
+
+		// Collects the type parameters of one method that appear with a nullable annotation ('T?')
+		// anywhere in a visited type, including nested positions such as List<T?> or T?[]. Type
+		// parameters of any other owner are ignored: a specialized signature can substitute a
+		// foreign type parameter that happens to share an index with one of this method's own.
+		sealed class NullableTypeParameterCollector(IReadOnlyList<ITypeParameter> typeParameters) : TypeVisitor
+		{
+			public readonly HashSet<ITypeParameter> NullableTypeParameters = [];
+
+			public override IType VisitNullabilityAnnotatedType(NullabilityAnnotatedType type)
+			{
+				if (type is NullabilityAnnotatedTypeParameter { Nullability: Nullability.Nullable } natp
+					&& typeParameters.Contains(natp.OriginalTypeParameter))
+				{
+					NullableTypeParameters.Add(natp.OriginalTypeParameter);
+				}
+				return base.VisitNullabilityAnnotatedType(type);
+			}
+		}
+
 		static bool IsObjectOrValueType(IType type)
 		{
 			ITypeDefinition? d = type.GetDefinition();