From 008cf7a5d1882bb69d65bf0fdc7c717cf67a7594 Mon Sep 17 00:00:00 2001 From: Sebastien Lebreton Date: Fri, 24 Jul 2026 18:10:33 +0200 Subject: [PATCH 1/5] 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(); From 0e600382b0c2c5191ee535393a69fa2363755fbd Mon Sep 17 00:00:00 2001 From: Sebastien Lebreton Date: Sat, 25 Jul 2026 16:37:35 +0200 Subject: [PATCH 2/5] 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); } From 61d29675b5c616e2604950ac88dfc1f830e5fe29 Mon Sep 17 00:00:00 2001 From: Sebastien Lebreton Date: Sat, 25 Jul 2026 17:46:02 +0200 Subject: [PATCH 3/5] Name the three type parameter states behind the disambiguator IsReferenceType is a bool?, so choosing between class, default and no constraint at all is a three-state decision. Spelling those states out keeps that visible where the choice is made, rather than leaving it implied by a comparison against true. Assisted-by: Copilot:claude-opus-5:GitHub Copilot CLI --- .../CSharp/Syntax/TypeSystemAstBuilder.cs | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/TypeSystemAstBuilder.cs b/ICSharpCode.Decompiler/CSharp/Syntax/TypeSystemAstBuilder.cs index 5b31831d4..dc2d2f955 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/TypeSystemAstBuilder.cs +++ b/ICSharpCode.Decompiler/CSharp/Syntax/TypeSystemAstBuilder.cs @@ -2615,15 +2615,10 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax // 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. + // 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. void AddNullabilityDisambiguatingConstraints(MethodDeclaration decl, IMethod method) { if (method.TypeParameters.Count == 0) @@ -2636,20 +2631,28 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax return; foreach (ITypeParameter tp in method.TypeParameters) { - if (!collector.NullableTypeParameters.Contains(tp)) - continue; - bool? isReferenceType = tp.IsReferenceType; - if (isReferenceType == false) + if (!collector.NullableTypeParameters.Contains(tp) || GetNullabilityDisambiguator(tp) is not string keyword) 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(isReferenceType == true ? "class" : "default")); + 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 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 or T?[]. Type // parameters of any other owner are ignored: a specialized signature can substitute a From ee2d7478627fd476e12523c81b408fc9b1e55a90 Mon Sep 17 00:00:00 2001 From: Sebastien Lebreton Date: Sun, 26 Jul 2026 19:28:59 +0200 Subject: [PATCH 4/5] Pin the allows ref struct interaction on nullable overrides allows ref struct is inherited implicitly, so restating it on an override is CS0460 even alongside a legal disambiguator. Roslyn still re-emits the byreflike flag on the override's own type parameter, and the general constraint printer turns that flag back into source, so the disambiguator stays legal only as long as it is built separately. Cover a C# 13 base whose annotated and plain methods both allow ref structs. Assisted-by: Copilot:claude-opus-5:GitHub Copilot CLI --- .../TestCases/Pretty/Issue3909.cs | 23 +++++++++++++++++++ .../CSharp/Syntax/TypeSystemAstBuilder.cs | 3 +++ 2 files changed, 26 insertions(+) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Issue3909.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Issue3909.cs index 5bc5a4b75..8d8284974 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Issue3909.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Issue3909.cs @@ -222,5 +222,28 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty return null; } } +#if CS130 + public class BaseWithAllowsRefStruct + { + public virtual void Annotated(T? value) where T : allows ref struct + { + } + + public virtual void Plain(T value) where T : allows ref struct + { + } + } + + public class DerivedWithAllowsRefStruct : BaseWithAllowsRefStruct + { + public override void Annotated(T? value) where T : default + { + } + + public override void Plain(T value) + { + } + } +#endif } } diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/TypeSystemAstBuilder.cs b/ICSharpCode.Decompiler/CSharp/Syntax/TypeSystemAstBuilder.cs index dc2d2f955..a1e99ea6c 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/TypeSystemAstBuilder.cs +++ b/ICSharpCode.Decompiler/CSharp/Syntax/TypeSystemAstBuilder.cs @@ -2619,6 +2619,9 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax // 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) From ffbd2055b08844cd3c887a47facbd12e6797529e Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Mon, 27 Jul 2026 16:45:36 +0200 Subject: [PATCH 5/5] Pin disambiguators for constraints on sibling method type parameters The chain cases in the fixture route T : TOuter through a class-level type parameter; the variant where the dependency target is a sibling method type parameter (M with T : U) was uncovered. It pins the same alignment from a different angle: csc rejects 'class' with CS8665 when U is merely class-constrained and requires 'default', but accepts 'class' when U carries a class-type constraint, matching what the tri-state IsReferenceType derives. Both directions were verified against csc before adding the expected output. Assisted-by: Claude:claude-fable-5:Claude Code --- .../TestCases/Pretty/Issue3909.cs | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Issue3909.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Issue3909.cs index 8d8284974..f46d5df67 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Issue3909.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Issue3909.cs @@ -207,6 +207,32 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty } } + public abstract class MethodChainBase + { + public virtual T? DependentOnClassConstrained(T? value, U other) where T : U where U : class + { + return value; + } + + public virtual T? DependentOnClassType(T? value, U other) where T : U where U : Node + { + return value; + } + } + + public sealed class MethodChainDerived : MethodChainBase + { + public override T? DependentOnClassConstrained(T? value, U other) where T : default + { + return value; + } + + public override T? DependentOnClassType(T? value, U other) where T : class + { + return value; + } + } + public class ContainerBase where TOuter : class { public virtual TOuter? Pick(TItem item)