From 0e600382b0c2c5191ee535393a69fa2363755fbd Mon Sep 17 00:00:00 2001 From: Sebastien Lebreton Date: Sat, 25 Jul 2026 16:37:35 +0200 Subject: [PATCH] Classify nullable override disambiguators by reference type A class-type constraint such as Stream or Delegate sets no ReferenceTypeConstraint flag, so keying the disambiguator off that flag gave those overrides the default constraint, which is CS8822, and the output still did not recompile. The restated disambiguator leaves no metadata trace of its own, so the choice has to follow from whether the inherited constraints make the type parameter a reference type, a value type, or neither. Matching the annotated type parameters by identity rather than by owner kind and index also keeps a specialized signature from contributing a foreign type parameter that happens to share an index. Assisted-by: Copilot:claude-opus-5:GitHub Copilot CLI --- .../TestCases/Pretty/Issue3909.cs | 121 ++++++++++++++++++ .../CSharp/Syntax/TypeSystemAstBuilder.cs | 44 ++++--- 2 files changed, 147 insertions(+), 18 deletions(-) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Issue3909.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Issue3909.cs index f8a4c41a2..5bc5a4b75 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Issue3909.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Issue3909.cs @@ -1,3 +1,6 @@ +using System; +using System.Collections.Generic; + #nullable enable namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty { @@ -101,5 +104,123 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty return value; } } + + public class Node + { + } + + public abstract class ConstrainedBase + { + public virtual T? ClassType(T? value) where T : Node + { + return value; + } + + public virtual T? DelegateType(T? value) where T : Delegate + { + return value; + } + + public virtual T? EnumType(T? value) where T : Enum + { + return value; + } + + public virtual T? InterfaceType(T? value) where T : IDisposable + { + return value; + } + + public virtual List NestedGeneric(List values) + { + return values; + } + + public virtual TItem? PartiallyAnnotated(TItem? value, TOther other) + { + return value; + } + } + + public sealed class ConstrainedDerived : ConstrainedBase + { + public override T? ClassType(T? value) where T : class + { + return value; + } + + public override T? DelegateType(T? value) where T : class + { + return value; + } + + public override T? EnumType(T? value) where T : default + { + return value; + } + + public override T? InterfaceType(T? value) where T : default + { + return value; + } + + public override List NestedGeneric(List values) where T : default + { + return values; + } + + public override TItem? PartiallyAnnotated(TItem? value, TOther other) where TItem : default + { + return value; + } + } + + public class ClassTypeChainBase where TOuter : Node + { + public virtual T? Chained(T? value) where T : TOuter + { + return value; + } + } + + public sealed class ClassTypeChainDerived : ClassTypeChainBase where TOuter : Node + { + public override T? Chained(T? value) where T : class + { + return value; + } + } + + public class ReferenceTypeChainBase where TOuter : class + { + public virtual T? Chained(T? value) where T : TOuter + { + return value; + } + } + + public sealed class ReferenceTypeChainDerived : ReferenceTypeChainBase where TOuter : class + { + public override T? Chained(T? value) where T : default + { + return value; + } + } + + public class ContainerBase where TOuter : class + { + public virtual TOuter? Pick(TItem item) + { + return null; + } + } + + public sealed class ContainerDerived : ContainerBase where TOuter : class + { + public override TOuter? Pick(TItem item) + { + return null; + } + } } } diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/TypeSystemAstBuilder.cs b/ICSharpCode.Decompiler/CSharp/Syntax/TypeSystemAstBuilder.cs index f8195087b..5b31831d4 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/TypeSystemAstBuilder.cs +++ b/ICSharpCode.Decompiler/CSharp/Syntax/TypeSystemAstBuilder.cs @@ -2613,49 +2613,57 @@ 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. + // 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. + // Which one is legal follows from whether the inherited constraints make the type + // parameter a reference type, a value type, or neither: + // - a value type uses Nullable rather than a nullable annotation, so it neither + // needs nor permits a disambiguator; + // - a reference type requires 'class'; + // - anything else requires 'default'. + // The inherited constraints are re-emitted on the override's own type parameters in + // metadata, so this classification does not depend on resolving the base member. The + // restated disambiguator itself is not, hence it must be derived rather than read back. void AddNullabilityDisambiguatingConstraints(MethodDeclaration decl, IMethod method) { if (method.TypeParameters.Count == 0) return; - NullableTypeParameterCollector collector = new(); + NullableTypeParameterCollector collector = new(method.TypeParameters); method.ReturnType.AcceptVisitor(collector); foreach (IParameter p in method.Parameters) p.Type.AcceptVisitor(collector); - if (collector.MethodTypeParameterIndices.Count == 0) + if (collector.NullableTypeParameters.Count == 0) return; foreach (ITypeParameter tp in method.TypeParameters) { - if (!collector.MethodTypeParameterIndices.Contains(tp.Index) || tp.HasValueTypeConstraint) + if (!collector.NullableTypeParameters.Contains(tp)) + continue; + bool? isReferenceType = tp.IsReferenceType; + if (isReferenceType == false) 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")); + c.BaseTypes.Add(new PrimitiveType(isReferenceType == true ? "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 + // 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 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 typeParameters) : TypeVisitor { - public readonly HashSet MethodTypeParameterIndices = []; + public readonly HashSet NullableTypeParameters = []; public override IType VisitNullabilityAnnotatedType(NullabilityAnnotatedType type) { if (type is NullabilityAnnotatedTypeParameter { Nullability: Nullability.Nullable } natp - && natp.OriginalTypeParameter.OwnerType == SymbolKind.Method) + && typeParameters.Contains(natp.OriginalTypeParameter)) { - MethodTypeParameterIndices.Add(natp.OriginalTypeParameter.Index); + NullableTypeParameters.Add(natp.OriginalTypeParameter); } return base.VisitNullabilityAnnotatedType(type); }