From 33b68138349bcfba4bc4171206e7732a5d3ea973 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Fri, 19 Jun 2026 18:14:05 +0200 Subject: [PATCH] Tighten AST child-access API and trim output-path overhead GetChildByRole returned default! while it can legitimately yield null (empty slot or a kind the node does not declare); it now returns T?. Callers that already null-checked are unchanged; the few that rely on a structural guarantee (name tokens, label identifier, property accessors) now state it with an explicit !, and MethodDeclaration drops a redundant cast that the lie required. Also removes two members left dead by the Role removal: the AddAnnotation override that only forwarded to the base, and GetCSharpNodeBefore, which ignored 'this' and had no callers. On the output path, GetChildNodes() allocates a snapshot List per call; the read-only .Children traversals that never mutate the tree now walk the live FirstChild/NextSibling chain (O(n), allocation-free, since child indices are cached), an EmptyStatement child check uses HasChildren, and the label-statement sibling scan breaks on the first match instead of walking to the end of the block. The generated GetChildNodes snapshot is kept where traversal may mutate. Assisted-by: Claude:claude-opus-4-8:Claude Code --- .../CSharp/CSharpDecompiler.cs | 8 +++---- .../OutputVisitor/CSharpOutputVisitor.cs | 11 +++++----- .../OutputVisitor/InsertParenthesesVisitor.cs | 2 +- .../CSharp/Syntax/AstNode.cs | 21 +++++-------------- .../Syntax/TypeMembers/EntityDeclaration.cs | 4 ++-- .../Syntax/TypeMembers/MethodDeclaration.cs | 2 +- .../CSharp/Transforms/FixNameCollisions.cs | 3 ++- .../Transforms/NormalizeBlockStatements.cs | 2 +- 8 files changed, 22 insertions(+), 31 deletions(-) diff --git a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs index f09270828..3cda20113 100644 --- a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs +++ b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs @@ -1352,11 +1352,11 @@ namespace ICSharpCode.Decompiler.CSharp RemoveAttribute(prop, KnownAttribute.ExtensionMarker); if (propDef.Getter != null) { - RemoveAttribute(prop.GetChildByRole(SlotKind.Getter), KnownAttribute.ExtensionMarker); + RemoveAttribute(prop.GetChildByRole(SlotKind.Getter)!, KnownAttribute.ExtensionMarker); } if (propDef.Setter != null) { - RemoveAttribute(prop.GetChildByRole(SlotKind.Setter), KnownAttribute.ExtensionMarker); + RemoveAttribute(prop.GetChildByRole(SlotKind.Setter)!, KnownAttribute.ExtensionMarker); } RunTransforms(syntaxTree, decompileRun, new SimpleTypeResolveContext(propDef.DeclaringTypeDefinition)); break; @@ -1851,11 +1851,11 @@ namespace ICSharpCode.Decompiler.CSharp RemoveAttribute(prop, KnownAttribute.ExtensionMarker); if (p.Getter != null) { - RemoveAttribute(prop.GetChildByRole(SlotKind.Getter), KnownAttribute.ExtensionMarker); + RemoveAttribute(prop.GetChildByRole(SlotKind.Getter)!, KnownAttribute.ExtensionMarker); } if (p.Setter != null) { - RemoveAttribute(prop.GetChildByRole(SlotKind.Setter), KnownAttribute.ExtensionMarker); + RemoveAttribute(prop.GetChildByRole(SlotKind.Setter)!, KnownAttribute.ExtensionMarker); } extMemberDecl = prop; break; diff --git a/ICSharpCode.Decompiler/CSharp/OutputVisitor/CSharpOutputVisitor.cs b/ICSharpCode.Decompiler/CSharp/OutputVisitor/CSharpOutputVisitor.cs index 9949072f2..3be6dc8ce 100644 --- a/ICSharpCode.Decompiler/CSharp/OutputVisitor/CSharpOutputVisitor.cs +++ b/ICSharpCode.Decompiler/CSharp/OutputVisitor/CSharpOutputVisitor.cs @@ -1903,7 +1903,7 @@ namespace ICSharpCode.Decompiler.CSharp.OutputVisitor public virtual void VisitLabelStatement(LabelStatement labelStatement) { StartNode(labelStatement); - WriteIdentifier(labelStatement.GetChildByRole(SlotKind.Identifier)); + WriteIdentifier(labelStatement.GetChildByRole(SlotKind.Identifier)!); WriteToken(Roles.Colon); bool foundLabelledStatement = false; for (AstNode? tmp = labelStatement.NextSibling; tmp != null; tmp = tmp.NextSibling) @@ -1911,6 +1911,7 @@ namespace ICSharpCode.Decompiler.CSharp.OutputVisitor if (tmp.Slot?.Kind == labelStatement.Slot?.Kind) { foundLabelledStatement = true; + break; } } if (!foundLabelledStatement) @@ -2428,7 +2429,7 @@ namespace ICSharpCode.Decompiler.CSharp.OutputVisitor WriteIdentifier(customEventDeclaration.NameToken); OpenBrace(policy.EventBraceStyle); // output add/remove in their original order - foreach (AstNode node in customEventDeclaration.Children) + for (AstNode? node = customEventDeclaration.FirstChild; node != null; node = node.NextSibling) { if (node.Slot?.Kind == SlotKind.AddAccessor || node.Slot?.Kind == SlotKind.RemoveAccessor) { @@ -2505,7 +2506,7 @@ namespace ICSharpCode.Decompiler.CSharp.OutputVisitor if (isSingleLine) Space(); // output get/set in their original order - foreach (AstNode node in indexerDeclaration.Children) + for (AstNode? node = indexerDeclaration.FirstChild; node != null; node = node.NextSibling) { if (node.Slot?.Kind == SlotKind.Getter || node.Slot?.Kind == SlotKind.Setter) { @@ -2667,7 +2668,7 @@ namespace ICSharpCode.Decompiler.CSharp.OutputVisitor if (isSingleLine) Space(); // output get/set in their original order - foreach (AstNode node in propertyDeclaration.Children) + for (AstNode? node = propertyDeclaration.FirstChild; node != null; node = node.NextSibling) { if (node.Slot?.Kind == SlotKind.Getter || node.Slot?.Kind == SlotKind.Setter) { @@ -2735,7 +2736,7 @@ namespace ICSharpCode.Decompiler.CSharp.OutputVisitor foreach (var trivia in syntaxTree.LeadingTrivia) trivia.AcceptVisitor(this); // don't do node tracking as we visit all children directly - foreach (AstNode node in syntaxTree.Children) + for (AstNode? node = syntaxTree.FirstChild; node != null; node = node.NextSibling) { node.AcceptVisitor(this); MaybeNewLinesAfterUsings(node); diff --git a/ICSharpCode.Decompiler/CSharp/OutputVisitor/InsertParenthesesVisitor.cs b/ICSharpCode.Decompiler/CSharp/OutputVisitor/InsertParenthesesVisitor.cs index 762a1ef5a..439ad6641 100644 --- a/ICSharpCode.Decompiler/CSharp/OutputVisitor/InsertParenthesesVisitor.cs +++ b/ICSharpCode.Decompiler/CSharp/OutputVisitor/InsertParenthesesVisitor.cs @@ -420,7 +420,7 @@ namespace ICSharpCode.Decompiler.CSharp.OutputVisitor if (node is CastExpression cast) return InterpolationNeedsParenthesis(cast.Expression); - foreach (var child in node.Children) + for (AstNode? child = node.FirstChild; child != null; child = child.NextSibling) { if (InterpolationNeedsParenthesis(child)) return true; diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs b/ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs index f027b9356..a867c64a0 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs +++ b/ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs @@ -297,21 +297,20 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax } /// - /// Gets the first child with the specified role. - /// Returns null if the child is not found. + /// Gets the first child with the specified role, or null if this node has no such child (the + /// slot is empty or the node declares no slot of that kind). /// - public T GetChildByRole(SlotKind kind) where T : AstNode? + public T? GetChildByRole(SlotKind kind) where T : AstNode { int count = GetChildCount(); for (int i = 0; i < count; i++) { if (GetChildSlotInfo(i).Kind == kind) { - AstNode? c = GetChild(i); - return c != null ? (T)c : default!; + return GetChild(i) as T; } } - return default!; + return null; } public T? GetParent() where T : AstNode @@ -731,11 +730,6 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax prev = prev.GetPrevNode(); return prev; } - // Comments and directives are trivia, not sibling nodes, so the previous sibling is always a C# node. - public AstNode? GetCSharpNodeBefore(AstNode node) - { - return node.PrevSibling; - } /// /// Gets the next sibling which fullfills a given predicate @@ -825,11 +819,6 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax return this.StartLocation <= location && location <= this.EndLocation; } - public override void AddAnnotation(object annotation) - { - base.AddAnnotation(annotation); - } - internal string DebugToString() { string text = ToString(); diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/EntityDeclaration.cs b/ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/EntityDeclaration.cs index c38ba9aa2..a48cc8b10 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/EntityDeclaration.cs +++ b/ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/EntityDeclaration.cs @@ -48,12 +48,12 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax } public virtual Identifier NameToken { - get { return GetChildByRole(SlotKind.Identifier); } + get { return GetChildByRole(SlotKind.Identifier)!; } set { SetChildByRole(SlotKind.Identifier, value); } } public virtual AstType ReturnType { - get { return GetChildByRole(SlotKind.Type); } + get { return GetChildByRole(SlotKind.Type)!; } set { SetChildByRole(SlotKind.Type, value); } } diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/MethodDeclaration.cs b/ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/MethodDeclaration.cs index a7237d9c2..57fe55ac3 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/MethodDeclaration.cs +++ b/ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/MethodDeclaration.cs @@ -72,7 +72,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax public bool IsExtensionMethod { get { - ParameterDeclaration pd = (ParameterDeclaration)GetChildByRole(SlotKind.Parameter); + ParameterDeclaration? pd = GetChildByRole(SlotKind.Parameter); return pd != null && pd.HasThisModifier; } } diff --git a/ICSharpCode.Decompiler/CSharp/Transforms/FixNameCollisions.cs b/ICSharpCode.Decompiler/CSharp/Transforms/FixNameCollisions.cs index de1e44003..9363b453b 100644 --- a/ICSharpCode.Decompiler/CSharp/Transforms/FixNameCollisions.cs +++ b/ICSharpCode.Decompiler/CSharp/Transforms/FixNameCollisions.cs @@ -69,7 +69,8 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms ISymbol? symbol = node.GetSymbol(); if (symbol != null && renamedSymbols.TryGetValue(symbol, out string? newName)) { - node.GetChildByRole(SlotKind.Identifier).Name = newName; + // An IdentifierExpression / MemberReferenceExpression always carries its name identifier. + node.GetChildByRole(SlotKind.Identifier)!.Name = newName; } } } diff --git a/ICSharpCode.Decompiler/CSharp/Transforms/NormalizeBlockStatements.cs b/ICSharpCode.Decompiler/CSharp/Transforms/NormalizeBlockStatements.cs index ff24d8ec9..0f731678f 100644 --- a/ICSharpCode.Decompiler/CSharp/Transforms/NormalizeBlockStatements.cs +++ b/ICSharpCode.Decompiler/CSharp/Transforms/NormalizeBlockStatements.cs @@ -126,7 +126,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms { var b = new BlockStatement(); statement.ReplaceWith(b); - if (statement is EmptyStatement && !statement.Children.Any()) + if (statement is EmptyStatement && !statement.HasChildren) { b.CopyAnnotationsFrom(statement); }