Browse Source

Add AstNode.CheckInvariant to validate the AST after each transform

A DEBUG-only structural check, the analog of ILInstruction.CheckInvariant, run
after every AST transform in RunTransforms. It recursively asserts that each
required (non-optional) single slot holds a child, every child's Parent points
back, the stored flattened index matches the slot position, and the runtime type
fits the slot, so a transform that corrupts the tree fails at that transform
instead of as a downstream output diff. CSharpSlotInfo gains an IsOptional flag
(emitted by the generator) so the check can tell required from optional slots.

Assisted-by: Claude:claude-opus-4-8:Claude Code
pull/3807/head
Siegfried Pammer 2 weeks ago committed by Siegfried Pammer
parent
commit
0a670d96bd
  1. 2
      ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs
  2. 3
      ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs
  3. 32
      ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs
  4. 10
      ICSharpCode.Decompiler/CSharp/Syntax/CSharpSlotInfo.cs
  5. 30
      ICSharpCode.Decompiler/CSharp/Syntax/ComposedType.cs

2
ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs

@ -610,7 +610,7 @@ internal class DecompilerSyntaxTreeGenerator : IIncrementalGenerator @@ -610,7 +610,7 @@ internal class DecompilerSyntaxTreeGenerator : IIncrementalGenerator
// One CSharpSlotInfo static per slot; node.Slot compares against these by object identity.
foreach (var s in slots)
builder.AppendLine($"\tpublic static readonly CSharpSlotInfo {s.PropertyName}Slot = new CSharpSlotInfo(\"{s.PropertyName}\", typeof({s.ElementType}), {(s.IsCollection ? "true" : "false")}, SlotKind.{s.KindName});");
builder.AppendLine($"\tpublic static readonly CSharpSlotInfo {s.PropertyName}Slot = new CSharpSlotInfo(\"{s.PropertyName}\", typeof({s.ElementType}), {(s.IsCollection ? "true" : "false")}, SlotKind.{s.KindName}, {(s.IsCollection || s.IsNullable ? "true" : "false")});");
builder.AppendLine();
builder.AppendLine("\tinternal override CSharpSlotInfo GetChildSlotInfo(int index)");

3
ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs

@ -715,6 +715,9 @@ namespace ICSharpCode.Decompiler.CSharp @@ -715,6 +715,9 @@ namespace ICSharpCode.Decompiler.CSharp
{
var typeSystemAstBuilder = CreateAstBuilder(decompileRun.Settings);
var context = new TransformContext(typeSystem, decompileRun, decompilationContext, typeSystemAstBuilder);
// The tree handed to the pipeline must already be well-formed; check it once up front so a
// malformed builder output is caught here rather than blamed on the first transform (DEBUG only).
rootNode.CheckInvariant();
foreach (var transform in astTransforms)
{
CancellationToken.ThrowIfCancellationRequested();

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

@ -399,25 +399,41 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax @@ -399,25 +399,41 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
}
/// <summary>
/// Recursively verifies the slot structure of this subtree (DEBUG only): every child's Parent
/// points back here, its stored flattened index matches its slot position, and its runtime type
/// is valid in the slot's role. The transform pipeline runs this after each transform, the
/// analog of <c>ILInstruction.CheckInvariant</c>, so a transform that corrupts the tree fails at
/// the exact transform rather than as a downstream output diff.
/// Recursively verifies the slot structure of this subtree (DEBUG only):
/// <list type="bullet">
/// <item>every required (non-optional) single slot holds a child, so a transform that drops a
/// mandatory node is caught here rather than as malformed output;</item>
/// <item>each present child points its <see cref="Parent"/> back at this node;</item>
/// <item>its stored flattened index matches its slot position; and</item>
/// <item>its runtime type is valid for the slot.</item>
/// </list>
/// The transform pipeline runs this after each transform, the analog of
/// <c>ILInstruction.CheckInvariant</c>, so a transform that corrupts the tree fails at the exact
/// transform rather than as a downstream output diff. The other tree invariants (no node reused
/// across trees, no node parented to itself) are enforced eagerly by the child setters and follow
/// from the parent back-pointer check above. A node type overrides this (calling <c>base</c>) to
/// assert its own constraints -- the scalar invariants its setters enforce eagerly, e.g.
/// <c>ComposedType.PointerRank &gt;= 0</c>.
/// </summary>
[System.Diagnostics.Conditional("DEBUG")]
internal void CheckInvariant()
internal virtual void CheckInvariant()
{
EnsureChildIndices();
int count = GetChildCount();
for (int i = 0; i < count; i++)
{
CSharpSlotInfo slot = GetChildSlotInfo(i);
AstNode? child = GetChild(i);
if (child == null)
continue; // empty optional single slot
{
// A single slot reads null only when empty; that is valid only if the slot is optional.
// (Collection slots never yield a null index, so this branch is always a single slot.)
Debug.Assert(slot.IsOptional, $"required slot '{slot.Name}' on {GetType().Name} must not be empty");
continue;
}
Debug.Assert(child.parent == this, "child's Parent must point back to this node");
Debug.Assert(child.childIndex == i, "child's flattened index must match its slot position");
Debug.Assert(GetChildSlotInfo(i).ChildType.IsInstanceOfType(child), "child's type must be valid in its slot");
Debug.Assert(slot.ChildType.IsInstanceOfType(child), "child's type must be valid in its slot");
child.CheckInvariant();
}
}

10
ICSharpCode.Decompiler/CSharp/Syntax/CSharpSlotInfo.cs

@ -46,12 +46,20 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax @@ -46,12 +46,20 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
/// </summary>
public SlotKind Kind { get; }
internal CSharpSlotInfo(string name, Type childType, bool isCollection, SlotKind kind)
/// <summary>
/// Whether the slot may be empty: a single slot whose child is nullable, or any collection slot
/// (a collection may hold zero children). A non-optional single slot must always hold a child in a
/// well-formed tree; <see cref="AstNode.CheckInvariant"/> asserts this.
/// </summary>
public bool IsOptional { get; }
internal CSharpSlotInfo(string name, Type childType, bool isCollection, SlotKind kind, bool isOptional)
{
Name = name;
ChildType = childType;
IsCollection = isCollection;
Kind = kind;
IsOptional = isOptional;
}
public override string ToString() => Name;

30
ICSharpCode.Decompiler/CSharp/Syntax/ComposedType.cs

@ -27,6 +27,7 @@ @@ -27,6 +27,7 @@
#nullable enable
using System;
using System.Diagnostics;
using System.Linq;
using System.Text;
@ -69,18 +70,8 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax @@ -69,18 +70,8 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
public bool HasNullableSpecifier { get; set; }
int pointerRank;
public int PointerRank {
get {
return pointerRank;
}
set {
if (value < 0)
throw new ArgumentOutOfRangeException(nameof(value));
pointerRank = value;
}
}
// Non-negativity is verified by CheckInvariant rather than an eager setter guard.
public int PointerRank { get; set; }
[Slot("ArraySpecifier")]
public partial AstNodeCollection<ArraySpecifier> ArraySpecifiers { get; }
@ -129,6 +120,14 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax @@ -129,6 +120,14 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
this.HasRefSpecifier = true;
return this;
}
internal override void CheckInvariant()
{
base.CheckInvariant();
// PointerRank is the number of '*' specifiers; a transform that corrupts it
// (e.g. an unbalanced decrement) is caught here at the offending transform.
Debug.Assert(PointerRank >= 0, "ComposedType.PointerRank must not be negative");
}
}
/// <summary>
@ -152,5 +151,12 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax @@ -152,5 +151,12 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
{
return "[" + new string(',', this.Dimensions - 1) + "]";
}
internal override void CheckInvariant()
{
base.CheckInvariant();
// A rank specifier always has at least one dimension ('[]' is rank 1); the output relies on it.
Debug.Assert(Dimensions >= 1, "ArraySpecifier.Dimensions must be at least 1");
}
}
}

Loading…
Cancel
Save