Browse Source

Enforce one child type per slot kind; de-alias shared kinds

A slot kind names one child position, so the generator emits a single
typed Slots.X constant for it. A kind used with two different child types
had to widen that constant to AstNode, and the typed GetChildren<T>
accessor then cast the live AstNodeCollection<Concrete> to
AstNodeCollection<AstNode> -- an InvalidCastException waiting for the
first caller (latent today, but the model permitted it).

Make the loose state unrepresentable instead of guarding it at runtime:
DSTG001 errors when a [Slot] kind is declared with more than one child
type. The six kinds that only coincidentally shared a name (Body, True,
False, Initializer, SwitchSection, Variable) are split into precisely
typed kinds; a genuinely either/or position (a lambda body) stays one
declared type, AstNode. Every Slots.X now carries its real element type,
so the GetChildren cast can no longer fail.

Output is unchanged: the Pretty suite stays byte-identical with
CheckInvariant green in DEBUG.

Assisted-by: Claude:claude-opus-4-8:Claude Code
pull/3807/head
Siegfried Pammer 2 weeks ago committed by Siegfried Pammer
parent
commit
e264e926bb
  1. 8
      ICSharpCode.Decompiler.Generators/AnalyzerReleases.Unshipped.md
  2. 35
      ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs
  3. 5
      ICSharpCode.Decompiler.Generators/ICSharpCode.Decompiler.Generators.csproj
  4. 2
      ICSharpCode.Decompiler/CSharp/OutputVisitor/CSharpOutputVisitor.cs
  5. 2
      ICSharpCode.Decompiler/CSharp/Syntax/Expressions/LambdaExpression.cs
  6. 4
      ICSharpCode.Decompiler/CSharp/Syntax/Expressions/SwitchExpression.cs
  7. 20
      ICSharpCode.Decompiler/CSharp/Syntax/README.md
  8. 2
      ICSharpCode.Decompiler/CSharp/Syntax/Statements/ForStatement.cs
  9. 4
      ICSharpCode.Decompiler/CSharp/Syntax/Statements/IfElseStatement.cs
  10. 2
      ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/ConstructorDeclaration.cs
  11. 2
      ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/EnumMemberDeclaration.cs
  12. 2
      ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/FixedFieldDeclaration.cs
  13. 2
      ICSharpCode.Decompiler/CSharp/Transforms/DeclareVariables.cs
  14. 4
      ICSharpCode.Decompiler/CSharp/Transforms/NormalizeBlockStatements.cs
  15. 2
      ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs

8
ICSharpCode.Decompiler.Generators/AnalyzerReleases.Unshipped.md

@ -0,0 +1,8 @@ @@ -0,0 +1,8 @@
; Unshipped analyzer release
; https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md
### New Rules
Rule ID | Category | Severity | Notes
--------|----------|----------|-------
DSTG001 | DecompilerSyntaxTreeGenerator | Error | Slot kind must map to a single child type

35
ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs

@ -30,6 +30,20 @@ namespace ICSharpCode.Decompiler.Generators; @@ -30,6 +30,20 @@ namespace ICSharpCode.Decompiler.Generators;
[Generator]
internal class DecompilerSyntaxTreeGenerator : IIncrementalGenerator
{
// Enforces the slot-model invariant: a slot kind names one child position, so it must map to a
// single declared child type across all nodes that use it. A kind used with two different types
// would have to widen its typed Slots constant to AstNode, which makes the typed child accessors
// (GetChildren in particular) unable to recover the real element type and throw at runtime. A
// position that is genuinely either an expression or a statement (a lambda body) is still one
// declared type -- AstNode -- and is fine; the error fires only on coincidental name reuse.
static readonly DiagnosticDescriptor MultipleChildTypesForKind = new(
id: "DSTG001",
title: "Slot kind used with multiple child types",
messageFormat: "Slot kind '{0}' is declared with multiple child types ({1}); each [Slot] kind must map to a single child type. Give the differing position its own [Slot] name.",
category: "DecompilerSyntaxTreeGenerator",
defaultSeverity: DiagnosticSeverity.Error,
isEnabledByDefault: true);
record AstNodeAdditions(string NodeName, bool NeedsVisitor, bool IsAbstract, bool BaseHasDefaultConstructor, bool NeedsPatternPlaceholder, string VisitMethodName, string VisitMethodParamType, EquatableArray<MemberMatch>? MembersToMatch, EquatableArray<SlotInfo>? Slots, EquatableArray<NameAccessor>? NameAccessors, EquatableArray<CtorParam>? CtorParams);
// One DoMatch comparison: Member is the property (or the "MatchAttributesAndModifiers" sentinel);
@ -801,20 +815,25 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax @@ -801,20 +815,25 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
}
}
// A slot kind names one child position, so it maps to a single child type (DSTG001 enforces this);
// the typed Slots constant therefore always carries that precise type. A kind that is a collection
// on one node and a single (or optional) child on another keeps a single type but no single arity,
// so its hard-coded IsCollection/isOptional flags are not authoritative -- the precise per-position
// flags live on the per-node slots, which is where consumers read them; the shared constant carries
// identity (and the now-precise child type), not those flags.
foreach (var kv in kindTypes)
{
if (kv.Value.Count > 1)
context.ReportDiagnostic(Diagnostic.Create(MultipleChildTypesForKind, Location.None, kv.Key, string.Join(", ", kv.Value)));
}
var builder = new StringBuilder();
builder.AppendLine("// <auto-generated/>");
builder.AppendLine();
builder.AppendLine("namespace ICSharpCode.Decompiler.CSharp.Syntax;");
builder.AppendLine();
// Each constant is its own canonical kind (null Kind): node.GetChild(Slots.X) infers the child
// type, and node.Slot.Kind == Slots.X identifies a position polymorphically. A kind reused with
// several child types across node types widens to AstNode (only its concrete per-node slots carry
// the precise type); such kinds are only reached through those per-node slots. For the same reason
// the shared kind's IsCollection flag and its hard-coded isOptional: false are not authoritative
// when a kind is a collection on one node and a single/optional child on another: such dual-use
// kinds carry IsCollection false (no arity claim), and the precise per-position flags live on the
// per-node slots, which is where consumers read them; the shared constant carries identity, not
// those flags.
// type, and node.Slot.Kind == Slots.X identifies a position polymorphically.
builder.AppendLine("/// <summary>The shared slot kinds, one per distinct child position across the AST node types.</summary>");
builder.AppendLine("public static class Slots");
builder.AppendLine("{");

5
ICSharpCode.Decompiler.Generators/ICSharpCode.Decompiler.Generators.csproj

@ -18,4 +18,9 @@ @@ -18,4 +18,9 @@
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="5.0.0" />
</ItemGroup>
<ItemGroup>
<!-- Analyzer release tracking for the generator's own diagnostics (RS2008). -->
<AdditionalFiles Include="AnalyzerReleases.Unshipped.md" />
</ItemGroup>
</Project>

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

@ -225,7 +225,7 @@ namespace ICSharpCode.Decompiler.CSharp.OutputVisitor @@ -225,7 +225,7 @@ namespace ICSharpCode.Decompiler.CSharp.OutputVisitor
bool SkipToken()
{
return kind == Slots.Initializer
return kind == Slots.ForInitializer
|| kind == Slots.Iterator
|| kind == Slots.ResourceAcquisition;
}

2
ICSharpCode.Decompiler/CSharp/Syntax/Expressions/LambdaExpression.cs

@ -44,7 +44,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax @@ -44,7 +44,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
[Slot("Parameter")]
public partial AstNodeCollection<ParameterDeclaration> Parameters { get; }
[Slot("Body")]
[Slot("LambdaBody")]
public partial AstNode Body { get; set; }
}
}

4
ICSharpCode.Decompiler/CSharp/Syntax/Expressions/SwitchExpression.cs

@ -32,7 +32,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax @@ -32,7 +32,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
[Slot("Expression")]
public partial Expression Expression { get; set; }
[Slot("SwitchSection")]
[Slot("SwitchExpressionSection")]
public partial AstNodeCollection<SwitchExpressionSection> SwitchSections { get; }
}
@ -45,7 +45,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax @@ -45,7 +45,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
[Slot("Pattern")]
public partial Expression Pattern { get; set; }
[Slot("Body")]
[Slot("SwitchExpressionBody")]
public partial Expression Body { get; set; }
}
}

20
ICSharpCode.Decompiler/CSharp/Syntax/README.md

@ -60,10 +60,13 @@ and the **string** is the slot *kind* (see [Slots and kinds](#slots-and-kinds)): @@ -60,10 +60,13 @@ and the **string** is the slot *kind* (see [Slots and kinds](#slots-and-kinds)):
The kind string is shared across node types: every slot tagged `[Slot("Expression")]` has the same
kind, so `node.Slot.Kind == Slots.Expression` works polymorphically. Pick an existing kind name when
the position is the same logical role as on other nodes; pick a new one when it differs. Keep a kind
mapping to a single child type
where you can -- if one kind is used with several child types its typed `Slots` constant widens to
`AstNode` (still usable, but you lose inference for that kind; see the de-aliasing note below).
the position is the same logical role *and* child type as on other nodes; pick a new one when either
differs. A kind must map to a **single child type** across every node that uses it: the generator
emits one typed `Slots.X` constant per kind, and a kind spanning two child types would have to widen
that constant to `AstNode`, leaving the typed accessors (`GetChildren` especially) unable to recover
the real element type. The generator enforces this at build time (`DSTG001`). A position that is
genuinely an expression *or* a statement -- a lambda body -- is still a single declared type,
`AstNode`, and is fine.
### `[ExcludeFromMatch]`
@ -140,9 +143,12 @@ So a transform that corrupts the tree fails at the exact transform, not as a dow @@ -140,9 +143,12 @@ So a transform that corrupts the tree fails at the exact transform, not as a dow
## Conventions and gotchas
- The `[Slot]` string is the **bare kind name** ("Body", "Expression"), not a dotted role expression.
- A kind reused with two different child types widens its `Slots` constant to `AstNode`. If you need a
precise type, give the position its own kind name -- as the declaration-level attribute slot does
(`[Slot("AttributeSection")]`) versus an attribute section's own `[Slot("Attribute")]`.
- A kind maps to exactly one child type; reusing one name for two different child types is a build
error (`DSTG001`). When two positions would share a name but differ in child type, give each its own
kind name -- as the declaration-level attribute slot (`[Slot("AttributeSection")]`) does versus an
attribute section's own `[Slot("Attribute")]`, or the for-statement initializer
(`[Slot("ForInitializer")]`, a `Statement` collection) versus an object/array initializer
(`[Slot("Initializer")]`, an `ArrayInitializerExpression`).
- Optional **names** read back as `null` (not an empty string) and a `null`/empty assignment clears the
backing token.
- Don't hand-write what the generator emits (visitors, `DoMatch`, child dispatch, constructors, slot

2
ICSharpCode.Decompiler/CSharp/Syntax/Statements/ForStatement.cs

@ -41,7 +41,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax @@ -41,7 +41,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
/// Note: this contains multiple statements for "for (a = 2, b = 1; a > b; a--)", but contains
/// only a single statement for "for (int a = 2, b = 1; a > b; a--)" (a single VariableDeclarationStatement with two variables)
/// </summary>
[Slot("Initializer")]
[Slot("ForInitializer")]
public partial AstNodeCollection<Statement> Initializers { get; }
[Slot("Condition")]

4
ICSharpCode.Decompiler/CSharp/Syntax/Statements/IfElseStatement.cs

@ -40,10 +40,10 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax @@ -40,10 +40,10 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
[Slot("Condition")]
public partial Expression Condition { get; set; }
[Slot("True")]
[Slot("TrueStatement")]
public partial Statement TrueStatement { get; set; }
[Slot("False")]
[Slot("FalseStatement")]
public partial Statement? FalseStatement { get; set; }
}
}

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

@ -51,7 +51,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax @@ -51,7 +51,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
[Slot("Parameter")]
public partial AstNodeCollection<ParameterDeclaration> Parameters { get; }
[Slot("Initializer")]
[Slot("ConstructorInitializer")]
public partial ConstructorInitializer? Initializer { get; set; }
[Slot("Body")]

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

@ -46,7 +46,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax @@ -46,7 +46,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
[Slot("Identifier")]
public override partial Identifier NameToken { get; set; }
[Slot("Initializer")]
[Slot("EnumMemberInitializer")]
public partial Expression? Initializer { get; set; }
}
}

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

@ -47,7 +47,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax @@ -47,7 +47,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
[Slot("Type")]
public override partial AstType ReturnType { get; set; }
[Slot("Variable")]
[Slot("FixedVariable")]
public partial AstNodeCollection<FixedVariableInitializer> Variables { get; }
}
}

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

@ -587,7 +587,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms @@ -587,7 +587,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
if (v.Type.IsByRefLike)
return true; // by-ref-like variables always must be initialized at their declaration.
if (v.InsertionPoint.nextNode.Slot?.Kind == Slots.Initializer)
if (v.InsertionPoint.nextNode.Slot?.Kind == Slots.ForInitializer)
return true; // for-statement initializers always should combine declaration and initialization.
return !context.Settings.SeparateLocalVariableDeclarations;

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

@ -117,7 +117,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms @@ -117,7 +117,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
bool IsElseIf(Statement statement, Statement parent)
{
return parent is IfElseStatement && statement.Slot?.Kind == Slots.False;
return parent is IfElseStatement && statement.Slot?.Kind == Slots.FalseStatement;
}
static void InsertBlock(Statement statement)
@ -142,7 +142,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms @@ -142,7 +142,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
switch (statement)
{
case IfElseStatement ies:
return parent is IfElseStatement && ies.Slot?.Kind == Slots.False;
return parent is IfElseStatement && ies.Slot?.Kind == Slots.FalseStatement;
case VariableDeclarationStatement vds:
case WhileStatement ws:
case DoWhileStatement dws:

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

@ -192,7 +192,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms @@ -192,7 +192,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
if (next is ForStatement forStatement && ForStatementUsesVariable(forStatement, variable))
{
node.Remove();
next.InsertChildAfter(null, node, Slots.Initializer);
next.InsertChildAfter(null, node, Slots.ForInitializer);
return (ForStatement)next;
}
Match m3 = forPattern.Match(next);

Loading…
Cancel
Save