diff --git a/ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs b/ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs index 70326536e..c389bdfba 100644 --- a/ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs +++ b/ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs @@ -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)"); diff --git a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs index a3aedcb40..2a592dd9d 100644 --- a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs +++ b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs @@ -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(); diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs b/ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs index 560f36183..a3af4b46e 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs +++ b/ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs @@ -399,25 +399,41 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax } /// - /// 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 ILInstruction.CheckInvariant, 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): + /// + /// 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; + /// each present child points its back at this node; + /// its stored flattened index matches its slot position; and + /// its runtime type is valid for the slot. + /// + /// The transform pipeline runs this after each transform, the analog of + /// ILInstruction.CheckInvariant, 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 base) to + /// assert its own constraints -- the scalar invariants its setters enforce eagerly, e.g. + /// ComposedType.PointerRank >= 0. /// [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(); } } diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/CSharpSlotInfo.cs b/ICSharpCode.Decompiler/CSharp/Syntax/CSharpSlotInfo.cs index b9242effd..ae6bc0a03 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/CSharpSlotInfo.cs +++ b/ICSharpCode.Decompiler/CSharp/Syntax/CSharpSlotInfo.cs @@ -46,12 +46,20 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax /// public SlotKind Kind { get; } - internal CSharpSlotInfo(string name, Type childType, bool isCollection, SlotKind kind) + /// + /// 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; asserts this. + /// + 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; diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/ComposedType.cs b/ICSharpCode.Decompiler/CSharp/Syntax/ComposedType.cs index ad27e2988..c805dfc81 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/ComposedType.cs +++ b/ICSharpCode.Decompiler/CSharp/Syntax/ComposedType.cs @@ -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 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 ArraySpecifiers { get; } @@ -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"); + } } /// @@ -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"); + } } }