Browse Source

Tighten AST child-access API and trim output-path overhead

GetChildByRole<T> 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
pull/3807/head
Siegfried Pammer 3 weeks ago committed by Siegfried Pammer
parent
commit
33b6813834
  1. 8
      ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs
  2. 11
      ICSharpCode.Decompiler/CSharp/OutputVisitor/CSharpOutputVisitor.cs
  3. 2
      ICSharpCode.Decompiler/CSharp/OutputVisitor/InsertParenthesesVisitor.cs
  4. 21
      ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs
  5. 4
      ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/EntityDeclaration.cs
  6. 2
      ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/MethodDeclaration.cs
  7. 3
      ICSharpCode.Decompiler/CSharp/Transforms/FixNameCollisions.cs
  8. 2
      ICSharpCode.Decompiler/CSharp/Transforms/NormalizeBlockStatements.cs

8
ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs

@ -1352,11 +1352,11 @@ namespace ICSharpCode.Decompiler.CSharp @@ -1352,11 +1352,11 @@ namespace ICSharpCode.Decompiler.CSharp
RemoveAttribute(prop, KnownAttribute.ExtensionMarker);
if (propDef.Getter != null)
{
RemoveAttribute(prop.GetChildByRole<Accessor>(SlotKind.Getter), KnownAttribute.ExtensionMarker);
RemoveAttribute(prop.GetChildByRole<Accessor>(SlotKind.Getter)!, KnownAttribute.ExtensionMarker);
}
if (propDef.Setter != null)
{
RemoveAttribute(prop.GetChildByRole<Accessor>(SlotKind.Setter), KnownAttribute.ExtensionMarker);
RemoveAttribute(prop.GetChildByRole<Accessor>(SlotKind.Setter)!, KnownAttribute.ExtensionMarker);
}
RunTransforms(syntaxTree, decompileRun, new SimpleTypeResolveContext(propDef.DeclaringTypeDefinition));
break;
@ -1851,11 +1851,11 @@ namespace ICSharpCode.Decompiler.CSharp @@ -1851,11 +1851,11 @@ namespace ICSharpCode.Decompiler.CSharp
RemoveAttribute(prop, KnownAttribute.ExtensionMarker);
if (p.Getter != null)
{
RemoveAttribute(prop.GetChildByRole<Accessor>(SlotKind.Getter), KnownAttribute.ExtensionMarker);
RemoveAttribute(prop.GetChildByRole<Accessor>(SlotKind.Getter)!, KnownAttribute.ExtensionMarker);
}
if (p.Setter != null)
{
RemoveAttribute(prop.GetChildByRole<Accessor>(SlotKind.Setter), KnownAttribute.ExtensionMarker);
RemoveAttribute(prop.GetChildByRole<Accessor>(SlotKind.Setter)!, KnownAttribute.ExtensionMarker);
}
extMemberDecl = prop;
break;

11
ICSharpCode.Decompiler/CSharp/OutputVisitor/CSharpOutputVisitor.cs

@ -1903,7 +1903,7 @@ namespace ICSharpCode.Decompiler.CSharp.OutputVisitor @@ -1903,7 +1903,7 @@ namespace ICSharpCode.Decompiler.CSharp.OutputVisitor
public virtual void VisitLabelStatement(LabelStatement labelStatement)
{
StartNode(labelStatement);
WriteIdentifier(labelStatement.GetChildByRole<Identifier>(SlotKind.Identifier));
WriteIdentifier(labelStatement.GetChildByRole<Identifier>(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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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);

2
ICSharpCode.Decompiler/CSharp/OutputVisitor/InsertParenthesesVisitor.cs

@ -420,7 +420,7 @@ namespace ICSharpCode.Decompiler.CSharp.OutputVisitor @@ -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;

21
ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs

@ -297,21 +297,20 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax @@ -297,21 +297,20 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
}
/// <summary>
/// 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).
/// </summary>
public T GetChildByRole<T>(SlotKind kind) where T : AstNode?
public T? GetChildByRole<T>(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<T>() where T : AstNode
@ -731,11 +730,6 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax @@ -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;
}
/// <summary>
/// Gets the next sibling which fullfills a given predicate
@ -825,11 +819,6 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax @@ -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();

4
ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/EntityDeclaration.cs

@ -48,12 +48,12 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax @@ -48,12 +48,12 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
}
public virtual Identifier NameToken {
get { return GetChildByRole<Identifier>(SlotKind.Identifier); }
get { return GetChildByRole<Identifier>(SlotKind.Identifier)!; }
set { SetChildByRole(SlotKind.Identifier, value); }
}
public virtual AstType ReturnType {
get { return GetChildByRole<AstType>(SlotKind.Type); }
get { return GetChildByRole<AstType>(SlotKind.Type)!; }
set { SetChildByRole(SlotKind.Type, value); }
}

2
ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/MethodDeclaration.cs

@ -72,7 +72,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax @@ -72,7 +72,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
public bool IsExtensionMethod {
get {
ParameterDeclaration pd = (ParameterDeclaration)GetChildByRole<ParameterDeclaration>(SlotKind.Parameter);
ParameterDeclaration? pd = GetChildByRole<ParameterDeclaration>(SlotKind.Parameter);
return pd != null && pd.HasThisModifier;
}
}

3
ICSharpCode.Decompiler/CSharp/Transforms/FixNameCollisions.cs

@ -69,7 +69,8 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms @@ -69,7 +69,8 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
ISymbol? symbol = node.GetSymbol();
if (symbol != null && renamedSymbols.TryGetValue(symbol, out string? newName))
{
node.GetChildByRole<Identifier>(SlotKind.Identifier).Name = newName;
// An IdentifierExpression / MemberReferenceExpression always carries its name identifier.
node.GetChildByRole<Identifier>(SlotKind.Identifier)!.Name = newName;
}
}
}

2
ICSharpCode.Decompiler/CSharp/Transforms/NormalizeBlockStatements.cs

@ -126,7 +126,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms @@ -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);
}

Loading…
Cancel
Save