From 92f54a94a8ce99e7f26255b1ffcc09b6cc2f70f8 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sun, 14 Jun 2026 22:17:04 +0200 Subject: [PATCH] Support optional children in roles and generated DoMatch Preparation for making optional single-value slots nullable: a role may omit its null object and a slot may be cleared via SetChildByRole(role, null), and the generated DoMatch emits MatchOptional for a nullable single child so an absent child matches correctly. Both are inert until a slot is actually made nullable. Assisted-by: Claude:claude-opus-4-8:Claude Code --- .../DecompilerSyntaxTreeGenerator.cs | 20 ++++++++++++------- .../CSharp/Syntax/AstNode.cs | 12 ++++++++++- ICSharpCode.Decompiler/CSharp/Syntax/Role.cs | 6 ++++-- 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs b/ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs index e2b0ca609..2a3bc1b56 100644 --- a/ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs +++ b/ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs @@ -30,7 +30,7 @@ namespace ICSharpCode.Decompiler.Generators; [Generator] internal class DecompilerSyntaxTreeGenerator : IIncrementalGenerator { - record AstNodeAdditions(string NodeName, bool NeedsAcceptImpls, bool NeedsVisitor, bool NeedsNullNode, bool NeedsPatternPlaceholder, int NullNodeBaseCtorParamCount, bool IsTypeNode, string VisitMethodName, string VisitMethodParamType, EquatableArray<(string Member, string TypeName, bool RecursiveMatch, bool MatchAny)>? MembersToMatch, EquatableArray<(string RoleExpr, bool IsCollection, string PropertyName, string PropertyType, string ElementType, bool IsOverride, bool IsNullable, string KindName)>? Slots); + record AstNodeAdditions(string NodeName, bool NeedsAcceptImpls, bool NeedsVisitor, bool NeedsNullNode, bool NeedsPatternPlaceholder, int NullNodeBaseCtorParamCount, bool IsTypeNode, string VisitMethodName, string VisitMethodParamType, EquatableArray<(string Member, string TypeName, bool RecursiveMatch, bool MatchAny, bool Nullable)>? MembersToMatch, EquatableArray<(string RoleExpr, bool IsCollection, string PropertyName, string PropertyType, string ElementType, bool IsOverride, bool IsNullable, string KindName)>? Slots); // Derives the shared SlotKind name from a [Slot] role expression: the last dotted segment with a // trailing "Role" removed (e.g. "Roles.EmbeddedStatement" -> "EmbeddedStatement", "LeftRole" -> @@ -56,7 +56,7 @@ internal class DecompilerSyntaxTreeGenerator : IIncrementalGenerator _ => (targetSymbol.Name, targetSymbol.Name), }; - List<(string Member, string TypeName, bool RecursiveMatch, bool MatchAny)>? membersToMatch = null; + List<(string Member, string TypeName, bool RecursiveMatch, bool MatchAny, bool Nullable)>? membersToMatch = null; if (!targetSymbol.MemberNames.Contains("DoMatch")) { @@ -65,7 +65,7 @@ internal class DecompilerSyntaxTreeGenerator : IIncrementalGenerator var astNodeType = (INamedTypeSymbol)context.SemanticModel.GetSpeculativeSymbolInfo(context.TargetNode.Span.Start, SyntaxFactory.ParseTypeName("AstNode"), SpeculativeBindingOption.BindAsTypeOrNamespace).Symbol!; if (targetSymbol.BaseType!.MemberNames.Contains("MatchAttributesAndModifiers")) - membersToMatch.Add(("MatchAttributesAndModifiers", null!, false, false)); + membersToMatch.Add(("MatchAttributesAndModifiers", null!, false, false, false)); foreach (var m in targetSymbol.GetMembers()) { @@ -75,16 +75,17 @@ internal class DecompilerSyntaxTreeGenerator : IIncrementalGenerator continue; if (property.Type.MetadataName is "CSharpTokenNode" or "TextLocation") continue; + bool nullable = property.NullableAnnotation == NullableAnnotation.Annotated; switch (property.Type) { case INamedTypeSymbol named when named.IsDerivedFrom(astNodeType) || named.MetadataName == "AstNodeCollection`1": - membersToMatch.Add((property.Name, named.Name, true, false)); + membersToMatch.Add((property.Name, named.Name, true, false, nullable)); break; case INamedTypeSymbol { TypeKind: TypeKind.Enum } named when named.GetMembers().Any(_ => _.Name == "Any"): - membersToMatch.Add((property.Name, named.Name, false, true)); + membersToMatch.Add((property.Name, named.Name, false, true, false)); break; default: - membersToMatch.Add((property.Name, property.Type.Name, false, false)); + membersToMatch.Add((property.Name, property.Type.Name, false, false, false)); break; } } @@ -278,12 +279,17 @@ internal class DecompilerSyntaxTreeGenerator : IIncrementalGenerator {{ return other is {source.NodeName} o && !o.IsNull"); - foreach (var (member, typeName, recursive, hasAny) in source.MembersToMatch) + foreach (var (member, typeName, recursive, hasAny, nullable) in source.MembersToMatch) { if (member == "MatchAttributesAndModifiers") { builder.Append($"\r\n\t\t\t&& this.MatchAttributesAndModifiers(o, match)"); } + else if (recursive && nullable && typeName != "AstNodeCollection") + { + // An optional single-value child is null when absent; match null-safely. + builder.Append($"\r\n\t\t\t&& MatchOptional(this.{member}, o.{member}, match)"); + } else if (recursive) { builder.Append($"\r\n\t\t\t&& this.{member}.DoMatch(o.{member}, match)"); diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs b/ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs index fe4e2a203..c78a3365f 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs +++ b/ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs @@ -383,7 +383,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax return collection != null ? (AstNodeCollection)collection : new AstNodeCollection(this, role); } - protected void SetChildByRole(Role role, T newChild) where T : AstNode + protected void SetChildByRole(Role role, T? newChild) where T : AstNode { SetChildByRoleUntyped(role, newChild); } @@ -724,6 +724,16 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax return PatternMatching.Pattern.MatchString(pattern, text); } + /// + /// Matches two optional children: both absent (null), or both present and matching. + /// When the pattern side is present, its own DoMatch decides (e.g. an OptionalNode matches an + /// absent candidate). When it is absent, the candidate must be absent too. + /// + protected static bool MatchOptional(AstNode? thisChild, AstNode? otherChild, PatternMatching.Match match) + { + return thisChild != null ? thisChild.DoMatch(otherChild, match) : otherChild == null; + } + protected internal abstract bool DoMatch(AstNode? other, PatternMatching.Match match); bool PatternMatching.INode.DoMatch(PatternMatching.INode? other, PatternMatching.Match match) diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/Role.cs b/ICSharpCode.Decompiler/CSharp/Syntax/Role.cs index 113519cd0..a27b2773c 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/Role.cs +++ b/ICSharpCode.Decompiler/CSharp/Syntax/Role.cs @@ -96,10 +96,12 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax return node is T; } - public Role(string name, T nullObject) + public Role(string name, T? nullObject) { this.name = name ?? throw new ArgumentNullException(nameof(name)); - this.nullObject = nullObject; + // Roles used only for collections may omit the null object (pass null); NullObject then + // returns null, matching the documented contract. Single-value roles always pass one. + this.nullObject = nullObject!; } public override string ToString()