From 008cf7a5d1882bb69d65bf0fdc7c717cf67a7594 Mon Sep 17 00:00:00 2001 From: Sebastien Lebreton Date: Fri, 24 Jul 2026 18:10:33 +0200 Subject: [PATCH] Fix #3909: emit nullable override disambiguators Override constraints are normally inherited and omitted, but nullable type parameters still require class or default to distinguish annotations from Nullable. Derive that legal discriminator from the method metadata. Assisted-by: Copilot:gpt-5.6-sol:GitHub Copilot CLI Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 86d2918e-5a24-48b4-9a86-41d331ec3720 --- .../PrettyTestRunner.cs | 6 + .../TestCases/Pretty/Issue3909.cs | 105 ++++++++++++++++++ .../CSharp/Syntax/TypeSystemAstBuilder.cs | 69 +++++++++++- 3 files changed, 175 insertions(+), 5 deletions(-) create mode 100644 ICSharpCode.Decompiler.Tests/TestCases/Pretty/Issue3909.cs diff --git a/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs b/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs index 84bcd1921..8db635146 100644 --- a/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs +++ b/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) { diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Issue3909.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Issue3909.cs new file mode 100644 index 000000000..f8a4c41a2 --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Issue3909.cs @@ -0,0 +1,105 @@ +#nullable enable +namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty +{ + internal class Issue3909 + { + public abstract class Base + { + public virtual T? Unconstrained(T? value) + { + return value; + } + + public virtual T? ReferenceType(T? value) where T : class + { + return value; + } + + public virtual T? ReferenceTypeNullable(T? value) where T : class? + { + return value; + } + + public virtual T? NotNull(T? value) where T : notnull + { + return value; + } + + public virtual T? ValueType(T? value) where T : struct + { + return value; + } + + public virtual U? ReturnOnly() + { + return default(U); + } + + public virtual T?[] Nested(T?[] values) + { + return values; + } + + public virtual T Identity(T value) + { + return value; + } + } + + public sealed class Derived : Base + { + public override T? Unconstrained(T? value) where T : default + { + return value; + } + + public override T? ReferenceType(T? value) where T : class + { + return value; + } + + public override T? ReferenceTypeNullable(T? value) where T : class + { + return value; + } + + public override T? NotNull(T? value) where T : default + { + return value; + } + + public override T? ValueType(T? value) + { + return value; + } + + public override U? ReturnOnly() where U : default + { + return default(U); + } + + public override T?[] Nested(T?[] values) where T : default + { + return values; + } + + public override T Identity(T value) + { + return value; + } + } + + public interface IRoundTrip + { + T? RoundTrip(T? value); + } + + public class ExplicitImpl : IRoundTrip + { + T? IRoundTrip.RoundTrip(T? value) where T : default + { + return value; + } + } + } +} diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/TypeSystemAstBuilder.cs b/ICSharpCode.Decompiler/CSharp/Syntax/TypeSystemAstBuilder.cs index 897e157a1..f8195087b 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/TypeSystemAstBuilder.cs +++ b/ICSharpCode.Decompiler/CSharp/Syntax/TypeSystemAstBuilder.cs @@ -2336,13 +2336,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. + AddNullabilityDisambiguatingConstraints(decl, method); + } + else + { + foreach (ITypeParameter tp in method.TypeParameters) + { + var constraint = ConvertTypeParameterConstraint(tp); + if (constraint != null) + decl.Constraints.Add(constraint); + } } } decl.Body = GenerateBodyBlock(); @@ -2602,6 +2613,54 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax return c; } + // The compiler records the inherited class/struct constraint flags on an override or + // explicit interface implementation in metadata, even though the source does not restate + // them, so the disambiguator can be derived from the method's own type parameters without + // resolving the base member. 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. A struct-constrained parameter uses Nullable rather than a + // nullable annotation, so it neither needs nor permits one. + void AddNullabilityDisambiguatingConstraints(MethodDeclaration decl, IMethod method) + { + if (method.TypeParameters.Count == 0) + return; + NullableTypeParameterCollector collector = new(); + method.ReturnType.AcceptVisitor(collector); + foreach (IParameter p in method.Parameters) + p.Type.AcceptVisitor(collector); + if (collector.MethodTypeParameterIndices.Count == 0) + return; + foreach (ITypeParameter tp in method.TypeParameters) + { + if (!collector.MethodTypeParameterIndices.Contains(tp.Index) || tp.HasValueTypeConstraint) + continue; + Constraint c = new(); + c.TypeParameter = MakeSimpleType(tp.Name); + // C# accepts only plain 'class' here, never 'class?'; the constraint's own + // nullability is inherited from the base member regardless. + c.BaseTypes.Add(new PrimitiveType(tp.HasReferenceTypeConstraint ? "class" : "default")); + decl.Constraints.Add(c); + } + } + + // Collects the indices of a method's own type parameters that appear with a nullable + // annotation ('T?') anywhere in a visited type, including nested positions such as + // List or T?[]. + sealed class NullableTypeParameterCollector : TypeVisitor + { + public readonly HashSet MethodTypeParameterIndices = []; + + public override IType VisitNullabilityAnnotatedType(NullabilityAnnotatedType type) + { + if (type is NullabilityAnnotatedTypeParameter { Nullability: Nullability.Nullable } natp + && natp.OriginalTypeParameter.OwnerType == SymbolKind.Method) + { + MethodTypeParameterIndices.Add(natp.OriginalTypeParameter.Index); + } + return base.VisitNullabilityAnnotatedType(type); + } + } + static bool IsObjectOrValueType(IType type) { ITypeDefinition? d = type.GetDefinition();