Port the pattern matcher to list-indexed slot children

The pattern matcher walked collections through INode.Role/FirstChild/
NextSibling, skipping siblings of a different role. Now that each
AstNodeCollection<T> is already the per-role child list, the engine matches two
collections by list index, and INode sheds Role/FirstChild/NextSibling
entirely. A collection exposes its IReadOnlyList<INode> view through a cached
adapter rather than implementing the interface directly, so a typed collection
does not become ambiguous for LINQ. Characterization tests pin the matcher's
behavior first.
Assisted-by: Claude:claude-opus-4-8:Claude Code

ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs

@@ -222,9 +222,9 @@ internal class DecompilerSyntaxTreeGenerator : IIncrementalGenerator
 			return child.DoMatch(other, match);
 		}}
 
-		bool PatternMatching.INode.DoMatchCollection(Role? role, PatternMatching.INode? pos, PatternMatching.Match match, PatternMatching.BacktrackingInfo backtrackingInfo)
+		bool PatternMatching.INode.DoMatchCollection(global::System.Collections.Generic.IReadOnlyList<PatternMatching.INode> other, int pos, PatternMatching.Match match, PatternMatching.BacktrackingInfo backtrackingInfo)
 		{{
-			return child.DoMatchCollection(role, pos, match, backtrackingInfo);
+			return child.DoMatchCollection(other, pos, match, backtrackingInfo);
 		}}
 	}}
 "

ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj

@@ -331,6 +331,7 @@
     <Compile Include="TestCases\Pretty\VariableNaming.cs" />
     <Compile Include="TestCases\Pretty\VariableNamingWithoutSymbols.cs" />
     <Compile Include="PdbGenerationTestRunner.cs" />
+    <Compile Include="PatternMatching\PatternMatchingTests.cs" />
     <None Include="TestCases\ILPretty\Issue1047.il" />
     <None Include="TestCases\ILPretty\Issue959.cs" />
     <None Include="TestCases\ILPretty\FSharpLoops_Debug.cs" />

ICSharpCode.Decompiler.Tests/PatternMatching/PatternMatchingTests.cs

@@ -0,0 +1,266 @@
+// Copyright (c) 2026 AlphaSierraPapa for the SharpDevelop Team
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy of this
+// software and associated documentation files (the "Software"), to deal in the Software
+// without restriction, including without limitation the rights to use, copy, modify, merge,
+// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons
+// to whom the Software is furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in all copies or
+// substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED,
+// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR
+// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE
+// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+// DEALINGS IN THE SOFTWARE.
+
+using System.Linq;
+
+using ICSharpCode.Decompiler.CSharp.Syntax;
+using ICSharpCode.Decompiler.CSharp.Syntax.PatternMatching;
+
+using NUnit.Framework;
+
+namespace ICSharpCode.Decompiler.Tests.PatternMatching
+{
+	/// <summary>
+	/// Substrate-agnostic tests of the pattern-matching engine: single-node matching with captures,
+	/// the special pattern nodes (AnyNode, Choice, Repeat, OptionalNode, the back-references), and the
+	/// collection backtracking core. They are written against the public matching API so they hold
+	/// across the engine's storage model.
+	/// </summary>
+	[TestFixture]
+	public class PatternMatchingTests
+	{
+		static ExpressionStatement Stmt(string identifier)
+		{
+			return new ExpressionStatement(new IdentifierExpression(identifier));
+		}
+
+		static BlockStatement Block(params Statement[] statements)
+		{
+			var block = new BlockStatement();
+			foreach (var s in statements)
+				block.Add(s);
+			return block;
+		}
+
+		#region single node
+		[Test]
+		public void IdenticalExpressions_Match()
+		{
+			Assert.That(new IdentifierExpression("x").IsMatch(new IdentifierExpression("x")), Is.True);
+		}
+
+		[Test]
+		public void DifferentExpressions_DoNotMatch()
+		{
+			Assert.That(new IdentifierExpression("x").IsMatch(new IdentifierExpression("y")), Is.False);
+		}
+
+		[Test]
+		public void AnyNode_MatchesNonNull_RejectsNull()
+		{
+			Assert.That(new AnyNode().IsMatch(new IdentifierExpression("x")), Is.True);
+			Assert.That(new AnyNode().IsMatch(null), Is.False);
+		}
+
+		[Test]
+		public void AnyNodeOrNull_MatchesNull()
+		{
+			Assert.That(new AnyNodeOrNull().IsMatch(null), Is.True);
+			Assert.That(new AnyNodeOrNull().IsMatch(new IdentifierExpression("x")), Is.True);
+		}
+
+		[Test]
+		public void NamedNode_CapturesTheMatchedNode()
+		{
+			var pattern = new NamedNode("id", new AnyNode());
+			var target = new IdentifierExpression("foo");
+			var m = pattern.Match(target);
+			Assert.That(m.Success, Is.True);
+			Assert.That(m.Get("id").Single(), Is.SameAs(target));
+		}
+		#endregion
+
+		#region Choice
+		[Test]
+		public void Choice_MatchesAnyAlternative()
+		{
+			var choice = new Choice { new IdentifierExpression("a"), new IdentifierExpression("b") };
+			Assert.That(choice.IsMatch(new IdentifierExpression("a")), Is.True);
+			Assert.That(choice.IsMatch(new IdentifierExpression("b")), Is.True);
+			Assert.That(choice.IsMatch(new IdentifierExpression("c")), Is.False);
+		}
+
+		[Test]
+		public void Choice_DoesNotLeakCapturesFromFailedAlternative()
+		{
+			// The first alternative captures "g" then fails on the operator; the checkpoint must be
+			// restored so the surviving match carries only the second alternative's capture.
+			var choice = new Choice {
+				new BinaryOperatorExpression {
+					Left = new NamedNode("g", new IdentifierExpression("a")),
+					Operator = BinaryOperatorType.Add,
+					Right = new AnyNode()
+				},
+				new NamedNode("g", new IdentifierExpression("z"))
+			};
+			var m = choice.Match(new IdentifierExpression("z"));
+			Assert.That(m.Success, Is.True);
+			Assert.That(m.Get("g").Count(), Is.EqualTo(1));
+			Assert.That(((IdentifierExpression)m.Get("g").Single()).Identifier, Is.EqualTo("z"));
+		}
+		#endregion
+
+		#region back-references
+		[Test]
+		public void Backreference_MatchesEqualNode()
+		{
+			var pattern = new BinaryOperatorExpression {
+				Left = new NamedNode("x", new AnyNode()),
+				Operator = BinaryOperatorType.Any,
+				Right = new Backreference("x")
+			};
+			Assert.That(pattern.IsMatch(new BinaryOperatorExpression(
+				new IdentifierExpression("a"), BinaryOperatorType.Add, new IdentifierExpression("a"))), Is.True);
+			Assert.That(pattern.IsMatch(new BinaryOperatorExpression(
+				new IdentifierExpression("a"), BinaryOperatorType.Add, new IdentifierExpression("b"))), Is.False);
+		}
+
+		[Test]
+		public void IdentifierExpressionBackreference_MatchesSameName()
+		{
+			var pattern = new BinaryOperatorExpression {
+				Left = new NamedNode("v", new IdentifierExpression(Pattern.AnyString)),
+				Operator = BinaryOperatorType.Any,
+				Right = new IdentifierExpressionBackreference("v")
+			};
+			Assert.That(pattern.IsMatch(new BinaryOperatorExpression(
+				new IdentifierExpression("i"), BinaryOperatorType.Add, new IdentifierExpression("i"))), Is.True);
+			Assert.That(pattern.IsMatch(new BinaryOperatorExpression(
+				new IdentifierExpression("i"), BinaryOperatorType.Add, new IdentifierExpression("j"))), Is.False);
+		}
+
+		[Test]
+		public void IdentifierExpressionBackreference_RejectsTypeArguments()
+		{
+			var pattern = new BinaryOperatorExpression {
+				Left = new NamedNode("v", new IdentifierExpression(Pattern.AnyString)),
+				Operator = BinaryOperatorType.Any,
+				Right = new IdentifierExpressionBackreference("v")
+			};
+			var rightWithTypeArgs = new IdentifierExpression("i");
+			rightWithTypeArgs.TypeArguments.Add(new SimpleType("T"));
+			Assert.That(pattern.IsMatch(new BinaryOperatorExpression(
+				new IdentifierExpression("i"), BinaryOperatorType.Add, rightWithTypeArgs)), Is.False);
+		}
+		#endregion
+
+		#region collections
+		[Test]
+		public void EmptyCollections_Match()
+		{
+			Assert.That(Block().IsMatch(Block()), Is.True);
+		}
+
+		[Test]
+		public void Repeat_GreedilyMatchesAll()
+		{
+			var pattern = new BlockStatement { Statements = { new Repeat(new AnyNode("s")) } };
+			var m = pattern.Match(Block(Stmt("a"), Stmt("b"), Stmt("c")));
+			Assert.That(m.Success, Is.True);
+			Assert.That(m.Get("s").Count(), Is.EqualTo(3));
+		}
+
+		[Test]
+		public void Repeat_MatchesEmptyWhenMinZero()
+		{
+			var pattern = new BlockStatement { Statements = { new Repeat(new AnyNode()) } };
+			Assert.That(pattern.IsMatch(Block()), Is.True);
+		}
+
+		[Test]
+		public void Repeat_MinCountNotSatisfied_DoesNotMatch()
+		{
+			var pattern = new BlockStatement { Statements = { new Repeat(new AnyNode()) { MinCount = 2 } } };
+			Assert.That(pattern.IsMatch(Block(Stmt("a"))), Is.False);
+			Assert.That(pattern.IsMatch(Block(Stmt("a"), Stmt("b"))), Is.True);
+		}
+
+		[Test]
+		public void Repeat_MaxCountLeavesTrailingUnmatched_DoesNotMatch()
+		{
+			var pattern = new BlockStatement { Statements = { new Repeat(new AnyNode()) { MaxCount = 1 } } };
+			Assert.That(pattern.IsMatch(Block(Stmt("a"))), Is.True);
+			Assert.That(pattern.IsMatch(Block(Stmt("a"), Stmt("b"))), Is.False);
+		}
+
+		[Test]
+		public void Repeat_BacktracksToSatisfyTrailingFixedNode()
+		{
+			var pattern = new BlockStatement {
+				Statements = {
+					new Repeat(new AnyNode("body")),
+					new NamedNode("last", Stmt("end"))
+				}
+			};
+			var m = pattern.Match(Block(Stmt("a"), Stmt("b"), Stmt("end")));
+			Assert.That(m.Success, Is.True);
+			Assert.That(m.Get("body").Count(), Is.EqualTo(2));
+			Assert.That(((IdentifierExpression)((ExpressionStatement)m.Get("last").Single()).Expression).Identifier,
+				Is.EqualTo("end"));
+		}
+
+		[Test]
+		public void Optional_MatchesWhetherPresentOrAbsent()
+		{
+			var pattern = new BlockStatement {
+				Statements = {
+					new OptionalNode(Stmt("opt")),
+					new NamedNode("tail", Stmt("tail"))
+				}
+			};
+			Assert.That(pattern.IsMatch(Block(Stmt("opt"), Stmt("tail"))), Is.True);
+			Assert.That(pattern.IsMatch(Block(Stmt("tail"))), Is.True);
+			Assert.That(pattern.IsMatch(Block(Stmt("other"), Stmt("tail"))), Is.False);
+		}
+
+		[Test]
+		public void FixedLengthCollection_LengthMismatch_DoesNotMatch()
+		{
+			var pattern = Block(Stmt("a"), Stmt("b"));
+			Assert.That(pattern.IsMatch(Block(Stmt("a"), Stmt("b"))), Is.True);
+			Assert.That(pattern.IsMatch(Block(Stmt("a"), Stmt("b"), Stmt("c"))), Is.False);
+			Assert.That(pattern.IsMatch(Block(Stmt("a"))), Is.False);
+		}
+		#endregion
+
+		#region nesting
+		[Test]
+		public void NestedPattern_MatchesAndCaptures()
+		{
+			var pattern = new BinaryOperatorExpression {
+				Left = new BinaryOperatorExpression {
+					Left = new NamedNode("a", new AnyNode()),
+					Operator = BinaryOperatorType.Multiply,
+					Right = new NamedNode("b", new AnyNode())
+				},
+				Operator = BinaryOperatorType.Add,
+				Right = new NamedNode("c", new AnyNode())
+			};
+			var target = new BinaryOperatorExpression(
+				new BinaryOperatorExpression(new IdentifierExpression("x"), BinaryOperatorType.Multiply, new IdentifierExpression("y")),
+				BinaryOperatorType.Add,
+				new IdentifierExpression("z"));
+			var m = pattern.Match(target);
+			Assert.That(m.Success, Is.True);
+			Assert.That(((IdentifierExpression)m.Get("a").Single()).Identifier, Is.EqualTo("x"));
+			Assert.That(((IdentifierExpression)m.Get("b").Single()).Identifier, Is.EqualTo("y"));
+			Assert.That(((IdentifierExpression)m.Get("c").Single()).Identifier, Is.EqualTo("z"));
+		}
+		#endregion
+	}
+}

ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs

@@ -717,18 +717,12 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
 			return (other == null || o != null) && DoMatch(o, match);
 		}
 
-		bool PatternMatching.INode.DoMatchCollection(Role? role, PatternMatching.INode? pos, PatternMatching.Match match, PatternMatching.BacktrackingInfo? backtrackingInfo)
+		bool PatternMatching.INode.DoMatchCollection(System.Collections.Generic.IReadOnlyList<PatternMatching.INode> other, int pos, PatternMatching.Match match, PatternMatching.BacktrackingInfo backtrackingInfo)
 		{
-			AstNode? o = pos as AstNode;
+			PatternMatching.INode? raw = pos < other.Count ? other[pos] : null;
-			return (pos == null || o != null) && DoMatch(o, match);
+			AstNode? o = raw as AstNode;
-		}
+			// matches a single element: succeeds only if the element is absent or an AstNode
-
+			return (raw == null || o != null) && DoMatch(o, match);
-		PatternMatching.INode? PatternMatching.INode.NextSibling {
-			get { return NextSibling; }
-		}
-
-		PatternMatching.INode? PatternMatching.INode.FirstChild {
-			get { return FirstChild; }
 		}
 
 		#endregion

ICSharpCode.Decompiler/CSharp/Syntax/AstNodeCollection.cs

@@ -36,6 +36,31 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
 		internal abstract void InsertNodeBefore(AstNode? existing, AstNode node);
 		internal abstract void InsertNodeAfter(AstNode? existing, AstNode node);
 		internal abstract bool RemoveNode(AstNode node);
+
+		// The pattern matcher consumes a collection as an IReadOnlyList<INode>. The view is a separate
+		// (cached) object rather than the collection itself, because having AstNodeCollection<T>
+		// implement IEnumerable<INode> as well as IEnumerable<T> would make every LINQ call on a typed
+		// collection ambiguous.
+		private protected abstract int NodeCount { get; }
+		private protected abstract INode NodeAt(int index);
+
+		IReadOnlyList<INode>? nodeListView;
+		internal IReadOnlyList<INode> AsNodeList() => nodeListView ??= new NodeListView(this);
+
+		sealed class NodeListView : IReadOnlyList<INode>
+		{
+			readonly AstNodeCollection collection;
+			public NodeListView(AstNodeCollection collection) => this.collection = collection;
+			public INode this[int index] => collection.NodeAt(index);
+			public int Count => collection.NodeCount;
+			public IEnumerator<INode> GetEnumerator()
+			{
+				int count = collection.NodeCount;
+				for (int i = 0; i < count; i++)
+					yield return collection.NodeAt(i);
+			}
+			IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
+		}
 	}
 
 	/// <summary>
@@ -60,6 +85,9 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
 			get { return list.Count; }
 		}
 
+		private protected override int NodeCount => list.Count;
+		private protected override INode NodeAt(int index) => list[index];
+
 		public T this[int index] {
 			get { return list[index]; }
 			set {
@@ -272,7 +300,8 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
 
 		internal bool DoMatch(AstNodeCollection<T> other, Match match)
 		{
-			return Pattern.DoMatchCollection(role, parent.FirstChild, other.parent.FirstChild, match);
+			// Both collections are already the per-role child lists, so the matcher walks them by index.
+			return Pattern.DoMatchCollection(AsNodeList(), other.AsNodeList(), match);
 		}
 
 		public void InsertAfter(T existingItem, T newItem)

ICSharpCode.Decompiler/CSharp/Syntax/PatternMatching/INode.cs

@@ -17,6 +17,7 @@
 // DEALINGS IN THE SOFTWARE.
 
 using System;
+using System.Collections.Generic;
 
 namespace ICSharpCode.Decompiler.CSharp.Syntax.PatternMatching
 {
@@ -25,13 +26,17 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax.PatternMatching
 	/// </summary>
 	public interface INode
 	{
-		Role Role { get; }
-		INode FirstChild { get; }
-		INode NextSibling { get; }
 		bool IsNull { get; }
 
 		bool DoMatch(INode other, Match match);
-		bool DoMatchCollection(Role role, INode pos, Match match, BacktrackingInfo backtrackingInfo);
+
+		/// <summary>
+		/// Matches this pattern node against the collection element at <paramref name="pos"/>.
+		/// Deterministic nodes return whether the element matched (the caller then advances by one);
+		/// non-deterministic nodes (Repeat, OptionalNode) push their alternative continuation indices
+		/// onto the backtracking stack instead.
+		/// </summary>
+		bool DoMatchCollection(IReadOnlyList<INode> other, int pos, Match match, BacktrackingInfo backtrackingInfo);
 	}
 
 	public static class PatternExtensions

ICSharpCode.Decompiler/CSharp/Syntax/PatternMatching/IdentifierExpressionBackreference.cs

@@ -44,10 +44,9 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax.PatternMatching
 			var ident = other as IdentifierExpression;
 			if (ident == null || ident.TypeArguments.Any())
 				return false;
-			var referenced = (AstNode)match.Get(referencedGroupName).Last();
+			if (match.Get(referencedGroupName).Last() is not IdentifierExpression referenced)
-			if (referenced == null)
 				return false;
-			return ident.Identifier == referenced.GetChildByRole(Roles.Identifier).Name;
+			return ident.Identifier == referenced.Identifier;
 		}
 	}
 }

ICSharpCode.Decompiler/CSharp/Syntax/PatternMatching/OptionalNode.cs

@@ -17,6 +17,7 @@
 // DEALINGS IN THE SOFTWARE.
 
 using System;
+using System.Collections.Generic;
 
 namespace ICSharpCode.Decompiler.CSharp.Syntax.PatternMatching
 {
@@ -39,10 +40,12 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax.PatternMatching
 		{
 		}
 
-		public override bool DoMatchCollection(Role role, INode pos, Match match, BacktrackingInfo backtrackingInfo)
+		public override bool DoMatchCollection(IReadOnlyList<INode> other, int pos, Match match, BacktrackingInfo backtrackingInfo)
 		{
+			// Push the "absent" alternative (resume the following pattern node at the same position),
+			// then try to consume the element here.
 			backtrackingInfo.backtrackingStack.Push(new PossibleMatch(pos, match.CheckPoint()));
-			return childNode.DoMatch(pos, match);
+			return childNode.DoMatch(pos < other.Count ? other[pos] : null, match);
 		}
 
 		public override bool DoMatch(INode other, Match match)

ICSharpCode.Decompiler/CSharp/Syntax/PatternMatching/Pattern.cs

@@ -38,12 +38,12 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax.PatternMatching
 
 		internal struct PossibleMatch
 		{
-			public readonly INode NextOther; // next node after the last matched node
+			public readonly int NextOtherIndex; // index of the next other element after the last matched one
 			public readonly int Checkpoint; // checkpoint
 
-			public PossibleMatch(INode nextOther, int checkpoint)
+			public PossibleMatch(int nextOtherIndex, int checkpoint)
 			{
-				this.NextOther = nextOther;
+				this.NextOtherIndex = nextOtherIndex;
 				this.Checkpoint = checkpoint;
 			}
 		}
@@ -52,60 +52,46 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax.PatternMatching
 			get { return false; }
 		}
 
-		Role INode.Role {
-			get { return null; }
-		}
-
-		INode INode.NextSibling {
-			get { return null; }
-		}
-
-		INode INode.FirstChild {
-			get { return null; }
-		}
-
 		public abstract bool DoMatch(INode other, Match match);
 
-		public virtual bool DoMatchCollection(Role role, INode pos, Match match, BacktrackingInfo backtrackingInfo)
+		public virtual bool DoMatchCollection(IReadOnlyList<INode> other, int pos, Match match, BacktrackingInfo backtrackingInfo)
 		{
-			return DoMatch(pos, match);
+			return DoMatch(pos < other.Count ? other[pos] : null, match);
 		}
 
-		public static bool DoMatchCollection(Role role, INode firstPatternChild, INode firstOtherChild, Match match)
+		/// <summary>
+		/// Matches the <paramref name="patternChildren"/> collection against the
+		/// <paramref name="otherChildren"/> collection, with backtracking over Repeat/OptionalNode.
+		/// Each collection is already the per-role child list, so the match walks them by index.
+		/// </summary>
+		public static bool DoMatchCollection(IReadOnlyList<INode> patternChildren, IReadOnlyList<INode> otherChildren, Match match)
 		{
 			BacktrackingInfo backtrackingInfo = new BacktrackingInfo();
-			Stack<INode> patternStack = new Stack<INode>();
+			Stack<int> patternStack = new Stack<int>();
 			Stack<PossibleMatch> stack = backtrackingInfo.backtrackingStack;
-			patternStack.Push(firstPatternChild);
+			patternStack.Push(0);
-			stack.Push(new PossibleMatch(firstOtherChild, match.CheckPoint()));
+			stack.Push(new PossibleMatch(0, match.CheckPoint()));
 			while (stack.Count > 0)
 			{
-				INode cur1 = patternStack.Pop();
+				int patternPos = patternStack.Pop();
-				INode cur2 = stack.Peek().NextOther;
+				int otherPos = stack.Peek().NextOtherIndex;
 				match.RestoreCheckPoint(stack.Pop().Checkpoint);
 				bool success = true;
-				while (cur1 != null && success)
+				while (patternPos < patternChildren.Count && success)
 				{
-					while (cur1 != null && cur1.Role != role)
+					INode cur1 = patternChildren[patternPos];
-						cur1 = cur1.NextSibling;
-					while (cur2 != null && cur2.Role != role)
-						cur2 = cur2.NextSibling;
-					if (cur1 == null)
-						break;
-
 					Debug.Assert(stack.Count == patternStack.Count);
-					success = cur1.DoMatchCollection(role, cur2, match, backtrackingInfo);
+					success = cur1.DoMatchCollection(otherChildren, otherPos, match, backtrackingInfo);
 					Debug.Assert(stack.Count >= patternStack.Count);
+					// For every alternative the pattern node pushed, queue the next pattern node so the
+					// continuation resumes after it.
 					while (stack.Count > patternStack.Count)
-						patternStack.Push(cur1.NextSibling);
+						patternStack.Push(patternPos + 1);
 
-					cur1 = cur1.NextSibling;
+					patternPos++;
-					if (cur2 != null)
+					otherPos++;
-						cur2 = cur2.NextSibling;
 				}
-				while (cur2 != null && cur2.Role != role)
+				if (success && otherPos >= otherChildren.Count)
-					cur2 = cur2.NextSibling;
-				if (success && cur2 == null)
 					return true;
 			}
 			return false;

ICSharpCode.Decompiler/CSharp/Syntax/PatternMatching/Repeat.cs

@@ -17,7 +17,7 @@
 // DEALINGS IN THE SOFTWARE.
 
 using System;
-using System.Diagnostics;
+using System.Collections.Generic;
 
 namespace ICSharpCode.Decompiler.CSharp.Syntax.PatternMatching
 {
@@ -44,20 +44,16 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax.PatternMatching
 			this.MaxCount = int.MaxValue;
 		}
 
-		public override bool DoMatchCollection(Role role, INode pos, Match match, BacktrackingInfo backtrackingInfo)
+		public override bool DoMatchCollection(IReadOnlyList<INode> other, int pos, Match match, BacktrackingInfo backtrackingInfo)
 		{
 			var backtrackingStack = backtrackingInfo.backtrackingStack;
-			Debug.Assert(pos == null || pos.Role == role);
 			int matchCount = 0;
 			if (this.MinCount <= 0)
 				backtrackingStack.Push(new PossibleMatch(pos, match.CheckPoint()));
-			while (matchCount < this.MaxCount && pos != null && childNode.DoMatch(pos, match))
+			while (matchCount < this.MaxCount && pos < other.Count && childNode.DoMatch(other[pos], match))
 			{
 				matchCount++;
-				do
+				pos++;
-				{
-					pos = pos.NextSibling;
-				} while (pos != null && pos.Role != role);
 				if (matchCount >= this.MinCount)
 					backtrackingStack.Push(new PossibleMatch(pos, match.CheckPoint()));
 			}

PHASE_3C_TODO.md

@@ -0,0 +1,103 @@
+# Phase 3c -- storage flip (linked list -> slot fields). Fresh-session execution checklist.
+
+> **UPDATE 2026-06-14: Phases 4 (token drop) + 5 (comments->trivia) are now DONE (20 commits on
+> `ast-source-generator-rebased`, byte-identical, unpushed) -- they were reordered AHEAD of 3c.
+> So this checklist is now SIMPLER than written: CSharpTokenNode/CSharpModifierToken are deleted
+> (no token children), InsertSpecialsDecorator is deleted, and there are NO positional comment
+> children or `Roles.Comma` trailing-comma markers any more -- IGNORE the "Risks/gotchas" notes
+> about comments/commas/token positional children below; they no longer exist. The only
+> comment-as-node forms are ErrorExpression + comment-bearing EmptyStatement (normal nodes).
+> StartLocation/EndLocation are ALREADY stored fields set at print (not computed) -- the location
+> part of 3c is done. The GetNodeAt-family nav methods are dead -> just DELETE them. Launch from
+> current clean HEAD (not `20a04825e`). The ILSpy-tests submodule is now fully checked out in the
+> worktree (net40 configs run; no nuget-symlink workaround needed).**
+
+Atomic change (corner C1): the build does NOT compile again until ALL of A+B+C land together.
+Spec: `ROLES_FREE_SLOT_AST_DESIGN.md` Part I.1 + Phase 3c.
+
+Build/test (every run): prefix `OPENSSL_ENABLE_SHA1_SIGNATURES=1`; build with
+`-p:RestoreEnablePackagePruning=false`; suite is
+`dotnet test ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj --report-trx`
+(needs the `ILSpy-tests/nuget` symlink -> `/home/siegfried/Projects/ILSpy/ILSpy-tests/nuget`,
+recreate if absent or every test Assert.Fails at fixture setup). After each commit discard any
+pruned `*packages.lock.json`. Let the pre-commit format hook run (it touches DecompilerVersionInfo.cs
+harmlessly). Use `Assisted-by:` trailer, never Co-Authored-By.
+
+## 0. Optional pre-flip cleanups (cheap, do first or skip)
+- [ ] Squash `f914d57b3` (tried slot-order reorder) + `7fea03be7` (reverted it) -- they cancel; branch
+      is local-only so an interactive rebase is safe. Optional, cosmetic.
+- [ ] `UsingAliasDeclaration` alias: `AliasRole` holds an Identifier child but is exposed only as a
+      string wrapper (no Identifier property). Add `[Slot("AliasRole")] public partial Identifier
+      AliasToken { get; set; }` so the schema is complete (or handle inside the flip).
+
+## A. AstNodeCollection<T> -> list-backed  (CSharp/Syntax/AstNodeCollection.cs)
+Currently a *view* over the linked list (iterates parent.firstChild filtering by role). Rewrite:
+- [ ] Back with `List<T>`; hold `AstNode parent` + `int slotIndex` (the collection's slot).
+- [ ] Keep the used surface (already enumerated in design Part I.1): Add, AddRange, Insert,
+      InsertBefore(item)/InsertAfter(item), Remove, Clear, ReplaceWith(IEnumerable), MoveTo, Detach,
+      AcceptVisitor, indexer/Count/Contains/CopyTo/GetEnumerator, FirstOrNullObject/LastOrNullObject.
+- [ ] On every mutation, set added child's parent + role + childIndex, detach old, and **renumber
+      following children's childIndex** (insert/remove shift). Strict-tree invariant.
+- [ ] Implement IReadOnlyList<T> AND explicit IReadOnlyList<INode> (for the matcher, Phase 6).
+
+## B. AstNode base rewrite  (CSharp/Syntax/AstNode.cs)
+- [ ] Remove `firstChild/lastChild/prevSibling/nextSibling`. Add `int childIndex = -1`. Keep `parent`,
+      keep `flags` (role index stays until Phase 7 -- a child's role is set when placed in a slot).
+- [ ] Make abstract (generator implements per leaf): `int GetChildCount()`, `AstNode? GetChild(int)`,
+      `void SetChild(int, AstNode?)`, `CSharpSlotInfo GetChildSlot(int)`. (Replaces the bridge's
+      SlotCount/GetSlotRole/IsCollectionSlot/GetChild/SetChild.)
+- [ ] `SetChildNode<T>(ref T field, T value, int slotIndex)`: frozen-free; validate not-parented /
+      not-in-another-tree / type-compatible; detach old; write field; set value.parent + childIndex.
+- [ ] Re-express the linked-list API over slots (flattened index, Part I.4 -- single slot = width 1
+      even when empty; collection slot = contiguous run of its count; offsets after a collection shift
+      by its length):
+      - FirstChild/LastChild/NextSibling/PrevSibling: computed from (parent, childIndex), skipping
+        empty optional single slots.
+      - Children / HasChildren: iterate flattened slots in order.
+      - Descendants/GetDescendantsImpl, GetNodeAt/GetNodesBetween/Contains: now go through Children
+        (already do) -- verify they work once Children is slot-based. (These location-nav methods are
+        NRefactory-inherited and currently have no in-tree callers, but the API is kept; location
+        correctness itself is covered by LocationsInAstTests' text-slice assertion, not via GetNodeAt.)
+- [ ] Re-express the **role API** (still used by consumers until Phase 7) over slots:
+      - GetChildByRole<T>(role): scan slots for GetChildSlot(i).Role == role; single -> (T)GetChild(i)
+        (role null object if empty); collection role -> first element.
+      - GetChildrenByRole<T>(role): return the collection for that role. NB the base needs to reach the
+        collection object generically -- emit a generator hook (e.g. virtual AstNodeCollection
+        GetCollection(int slot)) or have GetChildrenByRole map role->slot and return the field.
+      - SetChildByRole, AddChild(child, role), AddChildWithExistingRole, InsertChildBefore/After(sibling),
+        Remove(), Detach, ReplaceWith(node) = parent.SetChild(childIndex, node).
+- [ ] StartLocation/EndLocation: still compute from first/last child (over slots) for now -- the
+      stored-field-at-print version is coupled to the Phase-4 token drop, NOT this phase.
+- [ ] Clone: MemberwiseClone; reset parent/childIndex; per-slot deep clone (loop GetChildCount/GetChild
+      or generator-emitted Clone) + CloneAnnotations (+ trivia, once that lands in Phase 5).
+
+## C. Generator  (ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs)
+- [ ] From the existing [Slot] declarations, emit: backing fields (single `T f;` / collection
+      `readonly AstNodeCollection<T> f`); property bodies single `get => f; set => SetChildNode(ref f,
+      value, <slot>)`, collection `get => f`; the flat GetChildCount/GetChild/SetChild/GetChildSlot
+      with collection range arithmetic; CSharpSlotInfo statics; per-slot Clone.
+- [ ] Inherited (override) slots already declared on leaves (flatten) -- emit their field+body on the
+      leaf (the field lives on the leaf, the base contract member becomes abstract or stays virtual).
+- [ ] Drop/replace the transitional SlotCount/GetSlotRole/IsCollectionSlot bridge emission.
+
+## D. Iterate to green
+- [ ] Build; fix the cascade of compile errors (internal users of firstChild/nextSibling, any direct
+      linked-list pokes, AstNodeCollection ctor signature changes). Expect many.
+- [ ] Full suite -> must return to 1195/984 baseline, byte-identical, 0 diffs.
+- [ ] ilspycmd smoke-decompile a couple assemblies; perf re-check vs the ~33s CoreLib baseline (E2).
+
+## E. Owed at the flip (tracked tasks #15, #16)
+- [ ] #16: re-introduce the slot-order invariant (now holds by construction -- children stored by slot)
+      as a DEBUG CheckInvariant; wire into the IAstTransform pipeline (design Part I.13).
+- [ ] #15: revert the one bridge-era decompiler change -- `IntroduceExtensionMethods` ReplaceWith
+      (commit 6869ef7a7) -- now unnecessary; audit for any other decompiler-transform changes.
+
+## Risks / gotchas
+- The role API reimplementation (single vs collection, role->slot lookup) is the fiddly core -- get
+  GetChildByRole/GetChildrenByRole exactly right; lots of consumers depend on them.
+- Pattern matcher: `AstNodeCollection<T>` is used by the matcher; keep IReadOnlyList<INode> working
+  (full matcher port is Phase 6, but don't break it now).
+- Comments/trivia are still ordinary `Roles.Comment` children in the linked-list model; after the flip
+  they're collection/role children too -- verify comment output unchanged (Phase 5 moves them to the
+  trivia channel later).
+- This delivers the O(1) access + is where perf finally arrives (the bridge had neither).

ROLES_FREE_SLOT_AST_DESIGN.md

@@ -0,0 +1,919 @@
+# Roles-free slot-based C# AST -- complete design + implementation plan
+
+Branch: `ast-source-generator-rebased`. Authoritative, self-contained design + step list for
+moving the C# AST from roles + doubly-linked-list children to a slot-based model. Companion:
+line-by-line pattern-matcher equivalence proofs in
+`~/.claude/plans/rustling-bubbling-oasis-agent-ae8f0184ec5b19d68.md`.
+
+## Context (why)
+
+ILSpy's C# AST (`ICSharpCode.Decompiler/CSharp/Syntax/`) is NRefactory-derived: children live
+in a per-node doubly-linked list, each tagged with a `Role` (9-bit index packed in `flags`).
+Consequences: `GetChildByRole` is a linear scan; child order is insertion order, not schema
+order (forcing hacks like the `InsertChildAfter` splice in
+`TransformFieldAndConstructorInitializers.RemoveImplicitConstructor`); every property
+read/write walks/relinks the list. The ILAst (`IL/Instructions/*`) already uses the better
+model -- fixed *slots* with O(1) access and schema-ordered children -- generated from a
+template. We are porting that model onto the C# AST, and using the move to shed other
+NRefactory full-fidelity baggage the decompiler does not need.
+
+Foundation already in place on this branch: a Roslyn incremental generator
+(`ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs`) that, from a
+`[DecompilerAstNode]` attribute + reflection over each node's properties, generates the
+mechanical members -- `AcceptVisitor` x3, the three `IAstVisitor` interfaces, `DoMatch`, null
+nodes, and pattern placeholders. Storage is still role + linked-list (handwritten
+`GetChildByRole` properties). This plan replaces that storage with slots and removes roles.
+
+## Decisions locked with the user
+- Roslyn incremental source generator (in place).
+- Node declaration = **partial properties, source-order slots** (no explicit indices).
+- **Delete `Role` entirely** (and `Role<T>`, `Roles`, `TokenRole`, `GetChildByRole`,
+  `SetChildByRole`, `GetChildrenByRole`, `node.Role`); rewrite all consumers; accepted public-API
+  break. **Keep `AstNode` and `AstNodeCollection<T>` names** -- reimplement the collection over slots
+  rather than rename it (avoids a gratuitous API break and keeps the `AstNode`/`AstNodeCollection`/
+  `AstType` name family; the IL parallel is implementation, not naming).
+- **Drop token nodes** (`CSharpTokenNode`/`CSharpModifierToken` deleted); their meaning -> scalars.
+- **Comments and preprocessor directives = node trivia** over a `Trivia` base
+  (`Comment`/`PreProcessorDirective` derive from it). Stored in the existing annotation channel as a
+  lazily-created `NodeTrivia` holder -- **no per-node fields**, so the ~99.99% no-trivia nodes cost
+  nothing (`CloneAnnotations` copies it for free). `AddLeadingTrivia`/`AddTrailingTrivia`, printed
+  before/after the node. `#define` (the only directive emitted; no bracketing) becomes leading trivia
+  on the `SyntaxTree`. Doc comments = a leading `Documentation` slot (structural, separate).
+- **The AST models structure + content, never formatting/whitespace.** Whitespace, indentation,
+  spacing, blank lines, brace style are print-time, driven by `CSharpFormattingOptions` (independent
+  of the AST); `Trivia` is content (comments+directives), not whitespace.
+- **Drop `IFreezable`/`Freeze`/`IsFrozen`** -- dead NRefactory baggage (no consumer ever freezes;
+  not even the null singletons).
+- **Locations** = stored `StartLocation`/`EndLocation` (`TextLocation` each), assigned at print time;
+  no `Span` tuple, no compute-from-children recursion.
+- **Inheritance** = abstract contract on bases + per-leaf ordered slots, flat `sealed` dispatch.
+- **Pattern matcher** = faithful port to list-based collection matching; slim `INode`; tests first.
+- **Null-object -> NRT**: backing field uniformly nullable; *syntactically required* slot =
+  non-null accessor (the static contract), optional slot = nullable accessor; delete the null
+  objects. `CheckInvariant` enforces required-non-null at between-transform checkpoints.
+- `Identifier` stays a node (single fixed slot). `CSharpSlotInfo` is positional. Annotations
+  unchanged.
+
+==========================================================================================
+# PART I -- THE COMPLETE DESIGN
+==========================================================================================
+
+## 1. Native model
+
+`AstNode` base (handwritten):
+```csharp
+public abstract class AstNode : AbstractAnnotatable, ICloneable, PatternMatching.INode {
+    AstNode? parent;
+    int childIndex = -1;            // flattened index within parent
+    TextLocation startLocation, endLocation;   // set at print time
+    // flags: per-class bits only (e.g. Identifier.IsVerbatim); no role index, no frozen bit
+
+    public AstNode? Parent { get; }
+    public int ChildIndex { get; }
+    public CSharpSlotInfo? Slot => parent?.GetChildSlot(childIndex);     // replaces node.Role
+    public TextLocation StartLocation { get; }   // returns startLocation (no recursion)
+    public TextLocation EndLocation { get; }     // returns endLocation
+    internal void SetLocations(TextLocation start, TextLocation end);    // print-time writer
+
+    // generated per concrete leaf (flat, sealed):
+    protected abstract int GetChildCount();
+    protected abstract AstNode? GetChild(int index);   // null = empty optional slot
+    protected abstract void SetChild(int index, AstNode? value);
+    protected abstract CSharpSlotInfo GetChildSlot(int index);
+
+    public ChildrenCollection Children { get; }        // document order (struct view)
+    public void ReplaceWith(AstNode n) => parent!.SetChild(childIndex, n);
+    // NextSibling/PrevSibling/FirstChild/LastChild computed from (parent, childIndex),
+    // skipping empty optional slots.
+    protected void SetChildNode<T>(ref T field, T value, int index) where T : AstNode?; // see below
+
+    // Trivia (comments + preprocessor directives), Trivia = base of Comment/PreProcessorDirective.
+    // NO per-node trivia fields: stored in the existing AbstractAnnotatable annotation channel as a
+    // lazily-created NodeTrivia holder {leading, trailing}, so a node WITHOUT trivia (the ~99.99%
+    // common case) pays nothing extra. CloneAnnotations copies it for free (Part 6/8).
+    public void AddLeadingTrivia(Trivia t);   // get-or-create the NodeTrivia annotation, append
+    public void AddTrailingTrivia(Trivia t);
+    public IEnumerable<Trivia> LeadingTrivia { get; }   // empty (no alloc) when the annotation is absent
+    public IEnumerable<Trivia> TrailingTrivia { get; }
+}
+```
+`SetChildNode` (analog of IL's `SetChildInstruction`): validate value not parented / not in another
+tree / type-compatible with slot; write field; set value.parent/childIndex; detach old. Strict-tree
+invariant (old node detached immediately); no IL "stale positions". (No frozen check -- `IFreezable`
+is dropped.)
+
+`CSharpSlotInfo` (the `SlotInfo` analog; positional):
+```csharp
+public sealed class CSharpSlotInfo {
+    public string Name { get; }        // "Left", "Body", "Parameters"
+    public Type ChildType { get; }     // declared child type
+    public bool IsOptional { get; }    // syntactically optional => nullable accessor (Part 11);
+                                        // required => non-null accessor. Drives CheckInvariant.
+    public bool IsCollection { get; }
+}
+```
+Per-slot static instances generated per class (`BinaryOperatorExpression.LeftSlot`).
+`node.Slot == X.YSlot` replaces `node.Role == X.YRole` (~43 consumer sites).
+
+`AstNodeCollection<T>` (kept; reimplemented over slots, analog of `InstructionCollection<T>`):
+List-backed, knows parent + slot, implements `IReadOnlyList<T>` and explicitly
+`IReadOnlyList<INode>` (for the matcher). Insert/remove renumbers following children's `childIndex`.
+Must preserve the actually-used surface: `Add`, `AddRange`, `Insert`, `InsertBefore(item,...)`/
+`InsertAfter(item,...)` (by reference node), `Remove`, `Clear`, `ReplaceWith(IEnumerable<T>)`,
+`MoveTo(targetCollection)` (cross-collection move -- corner C6), `Detach`, `AcceptVisitor`,
+indexer/`Count`/`Contains`/`CopyTo`/`GetEnumerator`. `FirstOrNullObject`/`LastOrNullObject` become
+`FirstOrDefault`/`LastOrDefault` (returning `null`) once null objects are gone (Part 11).
+
+## 2. Node declaration (author writes)
+```csharp
+[DecompilerAstNode]
+public partial class BinaryOperatorExpression : Expression {
+    public partial Expression Left { get; set; }
+    public partial Expression Right { get; set; }
+    public BinaryOperatorType Operator { get; set; }   // operator text derives from this
+}
+```
+The generator emits the other half: backing fields, property bodies
+(`get => left; set => SetChildNode(ref left, value, <n>)`), the `CSharpSlotInfo` statics,
+`GetChildCount/GetChild/SetChild/GetChildSlot`, `Clone`, and (as today) AcceptVisitor / DoMatch
+/ null node (until Part 11) / pattern placeholder. Slot order = source declaration order
+(Roslyn preserves member order).
+
+Declaration mechanism (how the generator reads each slot):
+- Single child slot: `public partial T Name { get; set; }` (T : AstNode). Read-only `{ get; }`
+  for a printer-derived child (e.g. a computed token replacement) -- rare after the token drop.
+- Collection slot: `public partial AstNodeCollection<T> Name { get; }` (get-only; generator owns the
+  instance).
+- **Optionality (Part 11 NRT): the declared nullability IS the slot's `IsOptional`** --
+  `T Name` => *syntactically required* (non-null accessor = the static contract; getter returns
+  `field!`), `T? Name` => optional (nullable accessor, `null` when empty). The **backing field is
+  uniformly nullable either way** (so transforms may transiently empty any slot); only the accessor
+  signature and `IsOptional` differ. Pre-NRT phases keep non-null accessors with null-object
+  substitution; the nullability annotations are applied in Phase 8.
+- Inherited contract members are re-declared `override partial` in the leaf at their ordered
+  position (Part 3).
+
+## 3. Inheritance composition
+Base classes are the shared **contract** (for polymorphic consumption); each concrete node owns
+its **implementation and order**. Base declares shared *child-slot* members `abstract`
+(`EntityDeclaration.Attributes`); shared *scalars* (`Modifiers`, `SymbolKind`) stay concrete on
+the base. Each leaf declares its full slot set as partial properties in document order,
+`override`-implementing the contract members where they belong; the generator emits one flat
+`sealed` dispatch per leaf. No base-delegation, no offset arithmetic (this is the divergence
+from ILAst, which forbids split children via `sealed`; C# AST splits, so we use the
+abstract-contract approach instead).
+Confirmed by spike E4 (prototype `352d1eb3c`): base-delegation is not merely heavier but
+*structurally incorrect* here -- for TypeMembers the inherited shared slots (`ReturnType`/
+`NameToken`) are NON-CONTIGUOUS in source order (`NameToken` is emitted after the leaf-only
+`PrivateImplementationType`, `CSharpOutputVisitor.cs:2577-2582`), so base-delegation, which
+front-pins the inherited block to indices `0..BaseSlotCount-1`, cannot produce correct document
+order. Flatten is required, not merely preferred; it is also smaller in total generated LOC (family
+158 vs 171) and avoids a base virtual call + offset math on the hot child-access path. The one
+flatten risk (a leaf silently mis-ordering/omitting a re-declared contract slot) is
+generator-controllable: the generator owns ordering and emits a per-leaf contract-coverage
+self-check.
+
+## 4. Collections & flattened indexing
+A node's flattened child index space = slots in declaration order. A **single slot is always
+width-1** -- it occupies its flattened index even when empty (`GetChild` returns `null` at that
+fixed position); only **collection** slots vary their width, occupying a contiguous run of the
+collection's current length. So the index space is stable across the Phase 11 null->nullable
+transition (an emptied single slot does not shift the offsets of later slots). Generated
+`GetChild/SetChild/GetChildSlot` use range arithmetic (single slots at fixed offsets; offsets after
+a collection shift by its count). `AstNodeCollection<T>` insert/remove renumbers following children.
+This removes the `TransformFieldAndConstructorInitializers` reordering hack (a param added to
+`TypeDeclaration.Parameters` lands at its schema position regardless of insertion time).
+**Renumber scheme (decided by spike E2, prototype `dd58586b8`): plain eager renumber-on-mutation.**
+It beats the linked list on every realistic op mix (positional Get/IndexOf on a list are O(1) vs the
+list's O(n) scans); the O(n^2) front-insert-no-read worst case is not a decompiler workload and is
+negligible in absolute terms (14 ms at 5000 elements vs a ~33 s CoreLib decompile). A lazy
+dirty-watermark scheme (childIndex recomputed on first ordinal read, ~20 LOC, no API change) is the
+documented fallback, applied only if profiling ever flags a specific oversized collection.
+Open sub-point (from E4): when a leaf does not store a contract slot (e.g. `FieldDeclaration` has no
+`PrivateImplementationType`/`NameToken` storage), decide whether that slot occupies a flattened index
+or is contract-only-not-a-slot -- nail this down with the width-1 rule above.
+
+## 5. Drop token nodes
+The ~85 `CSharpTokenNode` child slots are removed (the printer's `WriteToken(Role, text)` is
+handed the text; it never reads a token child). `CSharpTokenNode` and `CSharpModifierToken`
+types are deleted. Meaning -> scalars (most already exist): operators ->
+`BinaryOperatorType`/`AssignmentOperatorType`/`UnaryOperatorType`; accessor keyword ->
+`AccessorKind`; member modifiers -> `Modifiers` flags (flip storage from `CSharpModifierToken`
+children to a field); `async`/`await` presence -> `bool IsAsync` on `UsingStatement`/
+`ForeachStatement`/`LambdaExpression`/`AnonymousMethodExpression`; attribute target -> an
+`AttributeTarget` enum (complete value set: `None, Assembly, Module, Type, Method, Property, Field,
+Event, Param, Return` -- note `Property` is used today, `TransformFieldAndConstructorInitializers`
+sets `"property"`/`"field"`); punctuation -> pure output.
+Verified by spike E3 (prototype `6e29b23ec`): the decompile-to-text printer never reads these token
+children -- dropping `OperatorToken` and `ReturnToken`/`SemicolonToken` produced byte-identical
+Pretty output (667/667). `VisitBinaryOperatorExpression` derives operator text from the `Operator`
+enum; `ComposedType` reads `HasRefSpecifier`/`HasNullableSpecifier`/`PointerRank`, not its token
+children; `InsertRequiredSpacesDecorator` branches on token TEXT, never the Role; and
+`InsertParenthesesVisitor` is type/precedence-driven (zero paren drift). The genuine coupling is
+`InsertMissingTokensDecorator` (rebuilds tokens + assigns Roles on the `CreateWriterThatSetsLocationsInAST`
+location path, NOT in the Pretty path) and `InsertSpecialsDecorator`'s role-keyed trivia placement --
+see Phase 4.
+
+## 6. Comments, doc comments, directives
+- Free comments = **node trivia**. Today a `Comment` attaches via `Roles.Comment` to almost any
+  node (the `InsertSpecialsDecorator` interleaves at print) -- six receiver categories in use:
+  block statement-list, an `Expression`, a non-block `Statement`, between `TypeDeclaration.Members`,
+  an `AstType`, a `VariableInitializer`, an `Attribute`, an `Accessor`. Two comment-node types
+  cannot reproduce that. Instead, every comment is re-homed onto **the node it annotates** as
+  leading or trailing trivia:
+  - Storage (memory-efficient -- comments/directives are the absolute outlier in decompiler output,
+    so the no-trivia case must cost ZERO): NO per-node trivia fields. Trivia lives in the existing
+    `AbstractAnnotatable` annotation channel (the one `object annotations` field every node already
+    has) as a lazily-created `NodeTrivia` holder `{leading, trailing}`, added only when a node first
+    gets trivia. A node without trivia (the ~99.99%) pays nothing -- no new field, no allocation. This
+    keeps trivia outside the flattened child-index space (no matcher interaction, `CheckInvariant`'s
+    child-range arithmetic stays clean).
+  - API: `AddLeadingTrivia(Trivia)` / `AddTrailingTrivia(Trivia)` (the distinction is the method
+    called, not a caller-set flag) get-or-create the `NodeTrivia` annotation and append; read-back via
+    `LeadingTrivia`/`TrailingTrivia` (empty, no allocation, when absent).
+  - Print: output visitor emits leading trivia at `StartNode` (before the node's children) and
+    trailing at `EndNode` (after), insertion order per group -- replacing the `InsertSpecialsDecorator`
+    `Roles.Comment` path. The "between members" case becomes a *leading* comment on the following
+    member (re-homed at the call site).
+  - `Clone` is automatic: trivia is an annotation, so the existing `CloneAnnotations()` copies it --
+    no special trivia-copy code.
+  - Guard: assert no site needs an *interior* comment between two real children where neither is the
+    annotated node (the enumeration shows none -- all are first/last/leading-on-member).
+- **Preprocessor directives are trivia too**, on the SAME channel. `Comment` and
+  `PreProcessorDirective` both derive from a `Trivia` base, so leading/trailing trivia is a list of
+  `Trivia`. `Trivia` is **content the printer must emit verbatim (comments + directives), NOT
+  whitespace** -- whitespace/blank-lines are never modeled in the AST (Part 7). This unifies what
+  were two mechanisms (trivia channel + "marker nodes in the collection") into one, and takes
+  directives off the child-index space (a `#define` is file-level trivia, not a semantic child).
+  `#define` becomes **leading trivia on the `SyntaxTree`** (top-of-file); `IntroduceUsingDeclarations`
+  reads `syntaxTree.LeadingTrivia` instead of `Children`. `PreProcessorDirective` carries a
+  `DirectiveKind` + payload. Grounded: only `#define` is emitted today (`CSharpDecompiler.cs:2263`)
+  and there is no bracketing (region/if/endif/pragma never constructed); if any are added later they
+  attach as leading/trailing trivia on the relevant node (the natural model).
+- Doc comments: a leading `Documentation` slot on `EntityDeclaration` (a `DocumentationComment`
+  node, multi-line text + span); rides with its member. Replaces today's fragile preceding
+  `Comment(Documentation)` siblings (`AddXmlDocumentationTransform`). Kept separate from the trivia
+  channel (it is structural -- a slot that rides with the member -- not free-floating trivia).
+
+## 7. Source locations & the AST/formatting boundary
+**Principle: the AST models structure + content, never formatting or whitespace.** Whitespace,
+indentation, spacing, blank lines, and brace style are *print-time* concerns -- the output visitor
+produces them when turning the AST into text, driven by `CSharpFormattingOptions`, which stays an
+**independent** object passed to the printer, not state on the nodes. The AST carries no whitespace
+nodes and no formatting flags. (This is why locations are assigned at print time, and why `Trivia`
+is comments+directives only, not whitespace.) Consequence for the rewrite: do not migrate any
+formatting concept into a node or slot; if a current node carries an incidental formatting hint, it
+belongs in the printer + options, not the AST.
+
+Locations: keep the existing `StartLocation`/`EndLocation` properties (`TextLocation` each), the primary
+consumer being PDB generation (`DebugInfo.SequencePoint` is `StartLine/StartColumn/EndLine/EndColumn`;
+`SequencePointBuilder` reads `node.StartLocation`/`EndLocation`) plus `AstNode`'s own tree-nav
+(`GetNodeAt`/`GetNodesBetween`/`Contains`). **No `Span` tuple** -- keeping the two property names is
+the facade that spares the ~33 readers any churn. Today these are *virtual, computed by recursion*
+to the token leaves (`AstNode.cs:183-199`); the token drop removes those leaves, so the new model
+**stores** the values in two fields and **drops the recursion**. Assigned by the existing mechanism:
+the output visitor brackets every node with `StartNode`/`EndNode`, and `WrapInWriterThatSetsLocationsInAST`
+reads the writer's tracked `Location` and calls the internal `SetLocations`. No computed fallback is
+needed: locations are only meaningful post-format today too (token-leaf locations are `Empty` until
+`InsertMissingTokensDecorator` runs), and all ~33 readers run post-format. Token-drop consequence:
+`SequencePointBuilder`'s `{`/`}` breakpoints move from brace-token locations to the
+`BlockStatement` start/end.
+
+## 8. Annotations
+Unchanged. `AstNode : AbstractAnnotatable` stays; annotations are orthogonal to the child model
+(keyed by node identity, not slots), and core to the decompiler (124 `.Annotation<...>` reads,
+incl. the `ILInstruction`-per-node mapping `SequencePointBuilder` uses). Generated `Clone` must
+keep calling `CloneAnnotations()` -- which also copies comment/directive trivia for free, since
+Part 6 stores the `NodeTrivia` holder as an annotation (same node-identity-keyed channel).
+
+## 9. Role deletion & node.Slot
+`Role`/`Role<T>`/`Roles`/`TokenRole`/`GetChildByRole`/`SetChildByRole`/`GetChildrenByRole` deleted.
+`AstNodeCollection<T>` is **kept** (reimplemented over slots, Part 1) -- not deleted/renamed.
+`node.Role` -> `node.Slot` (`CSharpSlotInfo`). Consumer rewrites: `GetChildByRole(R)` -> typed
+property or `GetChild(i)` (~14 external; ~439 internal vanish into generated bodies);
+`SetChildByRole(R, v)` -> typed property set; `node.Role == X.YRole` -> `node.Slot == X.YSlot` (~43);
+`GetChildrenByRole` -> the typed `AstNodeCollection<T>` property; `AddChild(c, R)`/
+`InsertChildBefore/After` -> `collection.Add`/`Insert`; `node.Role.IsValid(...)` -> slot/type check
+on `CSharpSlotInfo.ChildType`.
+
+## 10. Pattern-matching engine (faithful list-based port)
+Role-based collection matching becomes list-vs-list (each `AstNodeCollection<T>` already *is* the
+per-role sub-list). See Part II Phase 6 for the full design (slim `INode`; central matcher over
+`IReadOnlyList<INode>`; `Repeat`/`OptionalNode` over index cursor; `AstNodeCollection<T>.DoMatch`;
+generator `PatternPlaceholder` signature; `IdentifierExpressionBackreference` rewrite). Semantics
+preserved 1:1; unit tests added first.
+
+## 11. Null-object -> NRT
+**Backing field is uniformly nullable**; the accessor encodes the contract. Optional slot ->
+nullable accessor (`Expression? Body`, returns the field, `null` when empty); *syntactically
+required* slot -> non-null accessor (`Expression Left`, getter returns `field!`). The nullable field
+lets a transform transiently empty any slot ("optional during a transform"); the non-null accessor
+is the static contract that a finished tree has it filled, enforced by `CheckInvariant` at
+between-transform checkpoints (Part 13). Reading a transiently-empty required child throws at the
+bug site; an unrefilled one is caught at the next checkpoint -- so no special `Detach`/`Remove`
+rule is needed.
+Classification = *syntactically required* (the C# grammar is the oracle; mechanical proxy: a slot is
+required iff `CSharpOutputVisitor` dereferences it unconditionally). `default-to-optional` only as a
+tie-breaker. Drop the 18 generated null-node classes, `AstNode.IsNull`, and the `.Null` singletons.
+Consumer sweep: ~91 `.IsNull` -> `== null` (dead on required slots), ~99 `.Null` -> `null`; output
+visitor + `DepthFirstAstVisitor` drop `VisitNullNode` and null-check before recursing into optional
+children. Simplifies the matcher: `INode.IsNull` is then removed (empty = `null`).
+Verify before locking: no path emits a finished tree with a required slot empty (the decompiler
+emits an `ErrorExpression`/comment instead) -- a false-required there would throw where today it
+printed nothing.
+
+## 12. What the generator emits (end state)
+Per `[DecompilerAstNode]` node, from its partial-property declarations: backing fields; slot
+property bodies; `CSharpSlotInfo` statics; `GetChildCount/GetChild/SetChild/GetChildSlot` (flat,
+sealed, range arithmetic for collections); `Clone` (per-slot + `CloneAnnotations`); `AcceptVisitor`
+x3; the `IAstVisitor` interfaces; `DoMatch`; pattern placeholder (`DoMatchCollection` list-based).
+For required slots the getter returns `field!` (the static non-null contract over a nullable field);
+optional slots return the nullable field. No null-node classes after Part 11. Optionally generate
+`SymbolKind` from the model (replacing 14 handwritten overrides) -- deferred nicety.
+
+## 13. AST invariants (CheckInvariant)
+The slot model makes structural invariants *checkable* (the role + linked-list model could not
+validate a `childIndex` because there was none). Add a DEBUG-only recursive `CheckInvariant` on
+`AstNode`, the analog of `ILInstruction.CheckInvariant`, asserting per child:
+- `child.Parent == this` AND `this.GetChild(child.ChildIndex) == child` (parent/index round-trip);
+- child's runtime type is assignable to `GetChildSlot(child.ChildIndex).ChildType` (slot typing);
+- collection slots: elements occupy the slot's contiguous index range, in order, with matching
+  `childIndex`;
+- strict tree: every node reachable exactly once (no shared subtrees);
+- (post-NRT, Part 11) every slot with `CSharpSlotInfo.IsOptional == false` is non-null -- this is
+  the enforcement of "syntactically required" at the checkpoint (transforms may transiently empty it
+  between checkpoints; here it must be filled).
+Recurse into all children. Wire it into the C# transform pipeline to run after every
+`IAstTransform` in DEBUG (mirroring the IL pipeline's per-transform `CheckInvariant`). This is
+both a permanent correctness guard for transforms ("we want every transform to behave") and a
+migration accelerator -- a transform or a mis-generated slot that corrupts the tree fails at the
+exact transform, not as a downstream decompilation diff. Directly guards corners C1/C6.
+
+==========================================================================================
+# PART II -- IMPLEMENTATION STEPS
+==========================================================================================
+
+Guiding constraints: every phase ends green on the decompiler suites (`dotnet test --solution
+ILSpy.sln --report-trx`, prefixed `OPENSSL_ENABLE_SHA1_SIGNATURES=1`); the round-trip + Pretty
+suites are the real behavioral gate for the transforms/printer. The base-storage flip is the one
+unavoidably coordinated change; everything else is staged.
+
+**EXECUTION ORDER (revised 2026-06-13): Phase 4 -> Phase 5 -> Phase 3c.** The storage flip (3c) was
+originally sequenced before token-drop (4) and comments-to-trivia (5), but at the flip the still-live
+positional non-slot children (`Roles.Comment` / `Roles.Comma` / `Roles.PreProcessorDirective`,
+interleaved in document order by `InsertSpecialsDecorator` and attached to many no-collection nodes:
+CastExpression, ExpressionStatement, VariableInitializer, Attribute, AstType, ErrorExpression) have
+nowhere to live in a pure-slot tree. Landing Phase 4 + Phase 5 first removes every non-slot child, so
+the 3c flip becomes a clean pure-slot conversion with no interim sidecar. Trade-off accepted: the
+O(1)/perf payoff is deferred to 3c, and Phase 4's `InsertMissingTokensDecorator` location-path gating
+work is front-loaded. Phases 6-9 follow 3c as before.
+
+## Phase 0 -- Prerequisite (DONE)
+Reconciled `ast-source-generator` onto master; generator builds and runs; visitor/DoMatch/null/
+pattern generation in place. (Branch `ast-source-generator-rebased`, pushed.)
+
+## Phase S -- De-risking spikes (DONE)
+Ran E1-E4 in isolated worktrees (off the warmup commit `76a965ae2`); all four returned
+high-confidence verdicts. Prototype commits kept for reference (not landed):
+- **E1 migration mechanic -> BRIDGE** (commit `3050aee76`): slot accessors over the linked list,
+  family-by-family conversion, single later storage flip. Pretty output byte-identical to baseline
+  (667/667); DEBUG slot-order==document-order invariant held across all 667 fixtures. Prerequisites
+  surfaced: `Role.NullObjectUntyped`; collection slots need a separate design (ride to the flip on
+  `AstNodeCollection`); bridge gives the API but not O(1) perf (that arrives at the flip).
+- **E2 collection perf -> PLAIN EAGER renumber** (commit `dd58586b8`): the O(n^2) trap is
+  pathological (front-insert with no ordinal reads) only; on realistic op mixes eager List-backed
+  beats the linked list (whose positional Get/IndexOf are O(n)). Lazy dirty-watermark kept as a
+  documented ~20-LOC fallback. No smarter scheme up front.
+- **E3 token-drop -> LOW blast radius** (commit `6e29b23ec`): dropping token slots is byte-neutral
+  for the decompile-to-text printer (already scalar-driven). Real risk re-pointed to
+  `InsertMissingTokensDecorator` on the AST-with-locations (GUI) path, which Pretty does NOT cover.
+- **E4 inheritance -> KEEP FLATTEN** (commit `352d1eb3c`): base-delegation is structurally
+  incorrect -- inherited slots are non-contiguous in source order (`NameToken` after
+  `PrivateImplementationType`), which a front-pinned base block cannot represent. Flatten is also
+  smaller overall (family 158 vs 171 LOC).
+CheckInvariant (Phase 1) is brought up first so the spikes ran under it (the E1 invariant IS the
+Phase 3a slot-order guard).
+
+## Phase 1 -- Slot base infrastructure + CheckInvariant (handwritten)
+Reimplement `AstNodeCollection<T>` over slots (List-backed, parent+slot, renumbering,
+`IReadOnlyList<T>`+explicit `IReadOnlyList<INode>`; preserve the used surface per Part I.1); add
+`CSharpSlotInfo`. Add to `AstNode`: `childIndex`, `Slot`, stored `StartLocation`/`EndLocation` +
+internal `SetLocations`, abstract `GetChildCount/GetChild/SetChild/GetChildSlot`, `Children` struct
+view, `SetChildNode<T>`, computed sibling nav, slot-based `Clone`, the `AddLeading/TrailingTrivia`
+side-channel. Drop `IFreezable`/`Freeze`/`IsFrozen`. Implement `CheckInvariant`
+(Part I.13) and wire it into the C# `IAstTransform` pipeline (the transform runner in
+`CSharpDecompiler`) under DEBUG, so it guards every later phase and the spikes. Keep the
+linked-list API temporarily for unconverted nodes (bridge), or build on a sub-branch -- see
+Phase 3 / spike E1.
+Gate: compiles; CheckInvariant runs green on current (pre-migration) output; no node migrated yet.
+
+## Phase 2 -- Generator: slot storage emission
+Extend the generator to emit, from partial-property declarations: backing fields, property
+bodies, `CSharpSlotInfo` statics, and `GetChildCount/GetChild/SetChild/GetChildSlot` with the
+flat per-leaf dispatch + collection range arithmetic; adapt `Clone` to per-slot. Add a
+`SLOTAST` diagnostic for malformed declarations (mixed ordering, unsupported slot type).
+Gate: generator unit-builds; emits correct storage for a single pilot node.
+
+## Phase 3 -- Family-by-family node migration to slots
+Convert node declarations from handwritten `GetChildByRole` properties to partial properties,
+one family per step, regenerating storage. Order (smallest/most-isolated first):
+1. Expressions (leaf-heavy, few inter-family deps).
+2. Statements.
+3. TypeMembers (EntityDeclaration contract + leaves; exercises inheritance composition).
+4. GeneralScope + types (AstType hierarchy, NamespaceDeclaration, etc.).
+5. Remaining (SyntaxTree, VariableDesignation, etc.).
+Mechanic: **BRIDGE** (decided by spike E1, prototype `3050aee76`; the coordinated flip is a
+non-reviewable big-bang because all nodes share `AstNode`'s firstChild/lastChild traversal and
+`AstNodeCollection` is hardwired to it). Three sub-steps:
+- **3a.** Land the `AstNode` slot layer (`SlotCount`/`GetSlotRole(i)`/`IsCollectionSlot(i)`,
+  `GetChild(i)`/`SetChild(i)` over the linked list) PLUS the DEBUG slot-order==document-order
+  invariant FIRST, as its own commit, wired into `CSharpOutputVisitor.StartNode` so it runs the
+  whole suite on every PR. Prerequisite: add `Role.NullObjectUntyped` (`Role<T>` invariance blocks
+  `(Role<AstNode>)role`).
+- **3b.** Convert node declarations to slot accessors family-by-family (Expressions -> Statements ->
+  TypeMembers -> GeneralScope+types -> remaining). Each commit is small, mechanical, individually
+  revertible, and gated by the assert + byte-identical Pretty suite. Extend the slot-order assert to
+  optional/reordered-child families (modifiers, attributes, the EntityDeclaration interleave from E4)
+  as they convert -- it is proven only for simple Expressions so far.
+- **3c.** Flip storage to real backing fields as ONE coordinated change once all families route
+  through `GetChild`/`SetChild` -- linked list, `AstNodeCollection`, `Clone`, `StartLocation`/
+  `EndLocation`, and sibling nav flip together. **Collection slots** are not covered by the
+  single-slot bridge accessors (they stay on `GetChildrenByRole`/`AstNodeCollection` and move here) --
+  they need their own design step. The bridge yields the slot API but NOT O(1) access; perf only
+  arrives at 3c.
+Bridge is output-neutral in practice: E1 proved byte-identical Pretty output across all 667
+decompiled fixtures with the slot-order assert never tripping. The one known insertion-vs-schema
+divergence is the `TransformFieldAndConstructorInitializers` reordering hack; the assert is the tool
+that catches any other.
+Gate per family: full decompiler suite green; the slot-order==document-order assert holds for the
+converted family.
+
+## Phase 4 -- Drop tokens
+Convert token-carried meaning to scalars (modifiers field, `AccessorKind`, `IsAsync`,
+`AttributeTarget`, operator enums already present); remove token child slots from declarations;
+delete `CSharpTokenNode`/`CSharpModifierToken`; move `SequencePointBuilder` brace points to
+`BlockStatement` start/end.
+**Effort re-pointed by spike E3** (the decompile-to-text printer is already scalar-driven, so the
+spacing/parens rework the design feared is near-zero -- E3 dropped `OperatorToken` +
+`ReturnToken`/`SemicolonToken` with byte-identical Pretty output, and `InsertRequiredSpacesDecorator`
+branches on token TEXT not Role). The real, Pretty-INVISIBLE coupling is the gating work:
+- **(1, gating)** `InsertMissingTokensDecorator` reconstructs `CSharpTokenNode`/`CSharpModifierToken`
+  from the print stream and assigns Roles on the `CreateWriterThatSetsLocationsInAST` path (GUI
+  navigation / source maps), NOT exercised by Pretty. Build regression coverage for that path and run
+  it against the token-slot drop BEFORE committing the drop.
+- **(2)** `InsertSpecialsDecorator` role-keyed comment/preprocessor placement (low-risk: missing role
+  just doesn't advance) -- add comment/directive-bearing fixtures.
+- `InsertParenthesesVisitor` / `InsertRequiredSpacesDecorator` / spacing: verified inert by E3, no
+  change expected.
+Gate: Pretty + round-trip suites green; PDB sequence-point tests green; AND the
+AST-with-locations / token-navigation path green (Pretty alone does not exercise the at-risk
+component).
+
+## Phase 5 -- Comments + directives (trivia), doc comments
+Add the `Trivia` base (`Comment : Trivia`, `PreProcessorDirective : Trivia`) and the
+`AddLeading/TrailingTrivia` side-channel on `AstNode` (Part I.6); add `DocumentationComment` + the
+`Documentation` slot on `EntityDeclaration`. Migrate the 6 comment-receiver categories to
+`AddLeadingTrivia`/`AddTrailingTrivia` on the annotated node (re-home the between-members comments
+onto the following member); add the interior-comment assert. Move `#define` to leading trivia on the
+`SyntaxTree` and rewrite `IntroduceUsingDeclarations` to read `syntaxTree.LeadingTrivia`. Rewrite
+`AddXmlDocumentationTransform` to set the `Documentation` slot; update the printer to emit leading
+trivia at `StartNode` / trailing at `EndNode` + the doc node; remove the old `Comment`/
+`Roles.Comment` interleaving + `InsertSpecialsDecorator` trivia path.
+Gate: Pretty suite green (comment/doc/define output identical).
+
+## Phase 6 -- Pattern-matching engine port (TESTS FIRST)
+6.1 Add `ICSharpCode.Decompiler.Tests/PatternMatching/PatternMatchingTests.cs` (~18 NUnit cases,
+    substrate-agnostic; green on the current engine first). Cases: single-node + captures,
+    AnyNode/OrNull, NamedNode retrieval, Choice checkpoint-restore, Repeat min/max/greedy/
+    backtrack-to-satisfy-trailing-fixed, Optional present/absent/at-end, Backreference,
+    IdentifierExpressionBackreference (+ TypeArguments rejection), empty collections, nesting, and
+    real `forPattern` + trimmed `forOnArrayPattern` from PatternStatementTransform.
+6.2 Slim `INode`: keep `IsNull`+`DoMatch`; drop `Role`/`FirstChild`/`NextSibling`; new hook
+    `DoMatchCollection(IReadOnlyList<INode> other, int pos, Match, BacktrackingInfo)`.
+6.3 `Pattern.cs`: `PossibleMatch.NextOther` -> `int NextOtherIndex`; port the static core to
+    list+index (delete role-skip loops; `null` -> `pos>=Count`; `NextSibling` -> `+1`; preserve
+    the `stack.Count==patternStack.Count` invariant); new default instance hook
+    `DoMatch(pos<Count? other[pos] : null, match)`; drop dead INode explicit members.
+6.4 `Repeat`/`OptionalNode` `DoMatchCollection` over index cursor (`DoMatch` overloads unchanged).
+6.5 `AstNodeCollection<T>.DoMatch(other, match) => Pattern.DoMatchCollection(this, other, match)`
+    via the explicit `IReadOnlyList<INode>` view.
+6.6 `AstNode` INode impl: new list hook; remove `NextSibling`/`FirstChild`/old `DoMatchCollection`.
+6.7 Generator: `PatternPlaceholder.DoMatchCollection` new signature (member-type detection stays
+    `"AstNodeCollection`1"` -- the type name is unchanged).
+6.8 `IdentifierExpressionBackreference`: replace `referenced.GetChildByRole(Roles.Identifier).Name`
+    with `((IdentifierExpression)referenced).Identifier` guarded by `is`.
+Gate: 6.1 suite green after each sub-step; full decompiler suite green (transforms unchanged at
+the API level).
+
+## Phase 7 -- Delete Role + consumer rewrites
+Delete `Role`/`Role<T>`/`Roles`/`TokenRole`/`GetChildByRole`/`SetChildByRole`/
+`GetChildrenByRole`/`node.Role`/`AddChild(role)`/`InsertChildBefore/After` (KEEP
+`AstNodeCollection<T>` -- reimplemented over slots, not deleted). Rewrite the ~14
+external `GetChildByRole`, ~43 `.Role` reads (-> `.Slot`), and the `AddChild`/`InsertChild*`/
+`MoveTo` sites (-> collection ops; delete the reordering hack). Remove the role bridge from
+Phase 1/3.
+Gate: compiles with no role references; full suite green; public-API diff reviewed (this is the
+intended break).
+
+## Phase 8 -- Null-object -> NRT
+**Required/optional classification is NEW information** (the uniformly-null-object model records no
+such distinction), but the rule is objective: a slot is *syntactically required* iff the C# grammar
+cannot omit it -- mechanical proxy: `CSharpOutputVisitor` dereferences it unconditionally (no
+`if (x != null)` guard / alternate-token branch). Read the printer rather than judging per slot;
+`default-to-optional` only as a tie-breaker. Model: **backing field uniformly nullable**; required
+slot -> non-null accessor (`field!`, the static contract), optional -> nullable accessor; transforms
+may transiently empty any slot, and `CheckInvariant` enforces required-non-null at the
+between-transform checkpoints (so no special `Detach`/`Remove` rule).
+Generator: emit required vs optional accessors from the declared nullability (= `IsOptional`); stop
+emitting null-node classes + `hasNullNode`; `DoMatch`/matcher use `== null`. Delete the 18 `.Null`
+singletons + `AstNode.IsNull` + `INode.IsNull` + `VisitNullNode`. Consumer sweep: ~91 `.IsNull` ->
+`== null` (dead on required slots), ~99 `.Null` -> `null`/empty; output visitor +
+`DepthFirstAstVisitor` null-guards.
+**Classification rule, refined (verified):** "printer dereferences the slot unconditionally" is
+*necessary but not sufficient* for required -- the null-object's silent `VisitNullNode` no-op can
+mask a legitimately-empty slot. **Also check construction sites.** Confirmed landmine:
+`IndexerExpression.Target` is visited unconditionally (`CSharpOutputVisitor.cs:1012`) yet is
+intentionally empty in finished trees -- object initializers `new C { [i] = v }`
+(`CallBuilder.cs:1690`) and property-pattern index paths (`ExpressionBuilder.cs:3585`). So
+`Target` is **optional** (`Expression? Target`), and `VisitIndexerExpression` must guard it (or
+`Target?.AcceptVisitor`) once the null-object no-op is gone. Verified safe: `ErrorExpression` is
+always a complete top-level node (never a stand-in child, so error paths never empty a required
+slot); the truly-required slots (binary/assignment/cast/unary/condition/body) are always filled;
+the printer's explicit `IsNull` guards already mark the known optionals (`ReturnStatement.Expression`,
+`CaseLabel.Expression`, `ThrowStatement.Expression`, `IfElseStatement.FalseStatement`).
+`IndexerExpression.Target` is the **sole** null-`Target` case (verified: of ~17 construction sites
+only `CallBuilder.cs:1690` and `ExpressionBuilder.cs:3585` pass null; `MemberReferenceExpression`/
+`InvocationExpression` `Target` are never null-constructed -- `this.A` is an MRE with a
+`ThisReferenceExpression` target, bare `A` is an `IdentifierExpression`, so there is no implicit-`this`
+null-target). It exists only because an implicit-target indexer (dictionary/object initializer, and
+the property-pattern path) is *modeled* as `IndexerExpression` with a null target. Phase-8 choice:
+(A) classify `IndexerExpression.Target` optional and guard it in `VisitIndexerExpression` (cheap,
+honest); (B) give the implicit-target indexer its own node and keep `Target` required (removes the
+wart, adds a node type). Recommend (A).
+Gate: builds under `#nullable enable` with no new AST nullable warnings; full suite green;
+spot-check optional-child output (abstract methods, expression-bodied members, non-await
+using/foreach).
+
+## Phase 9 -- Cleanup & niceties
+**Decided, deferred (generator-internal, zero public-API impact): bit-pack declared scalars into
+`flags`** (ILAst-style). The generator backs each declared `bool` (1 bit) and *narrow non-`[Flags]`
+enum* (`BinaryOperatorType`/`UnaryOperatorType`/`AssignmentOperatorType` ~5-6 bits, `AccessorKind`,
+`VarianceModifier`, ...) with a mask+shift over `flags` instead of a field, allocating bit ranges
+per class above the base's used bits (generalize `AstNodeFlagsUsedBits` to a generated per-class
+`const`), with an overflow diagnostic if a node exceeds the word. **Carve-out: `[Flags]` enums stay
+their own field** -- `Modifiers` is ~19 flag bits, already a bitset, and would overflow the word
+(esp. while the 9-bit role index is still present pre-Phase-7). The accessor signature is unchanged
+(`public bool IsAsync { get; set; }`), so this is a pure storage swap doable any time after Phase 2.
+Delete dead decorators/visitor methods; optionally generate `SymbolKind` from the model; re-add
+the `Generators` project to `ILSpy.sln` (already done); regenerate lock files; final
+public-API-diff pass; update docs.
+
+## Files (representative, not exhaustive)
+- Base/infra: `CSharp/Syntax/AstNode.cs`, new `CSharpSlotInfo.cs`, `AstNodeCollection.cs`
+  (reimplemented over slots, not deleted), delete `Role.cs`/`Roles.cs`/`TokenRole.cs`/
+  `CSharpTokenNode.cs`/`CSharpModifierToken.cs`.
+- Nodes: every file under `CSharp/Syntax/Expressions|Statements|TypeMembers|GeneralScope` +
+  the type/`*AstType` files (partial-property conversion, generator-assisted).
+- Generator: `ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs`.
+- Matcher: `CSharp/Syntax/PatternMatching/*` (per Phase 6).
+- Printer/debug: `CSharp/OutputVisitor/CSharpOutputVisitor.cs`, `TextWriterTokenWriter.cs`,
+  `InsertMissingTokensDecorator.cs`, `InsertParenthesesVisitor.cs`, `InsertSpecialsDecorator.cs`,
+  `SequencePointBuilder.cs`, `DepthFirstAstVisitor.cs`.
+- Transforms: `AddXmlDocumentationTransform.cs`, `IntroduceUsingDeclarations.cs`,
+  `TransformFieldAndConstructorInitializers.cs` (drop reordering hack), `PatternStatementTransform.cs`
+  (no API change), plus the ~57 role/`GetChildByRole` consumer sites across `CSharp/`.
+- Tests: new `ICSharpCode.Decompiler.Tests/PatternMatching/PatternMatchingTests.cs`.
+
+## Verification (end to end)
+- Per phase: full decompiler suite green (`dotnet test --solution ILSpy.sln --report-trx`,
+  `OPENSSL_ENABLE_SHA1_SIGNATURES=1`); round-trip + Pretty are the behavioral gate; PDB
+  sequence-point tests guard locations after the token drop.
+- Pattern engine: the new unit suite (Phase 6.1) green throughout.
+- Final: public-API-diff review (intended break: roles/tokens/null-objects removed); build clean
+  under `#nullable enable`; ilspycmd smoke decompile of a few representative assemblies.
+
+## Weird corners (honest risk list) & de-risking spikes
+
+C1. **The base-storage flip is not cleanly incremental** (highest risk). All ~110 nodes share
+   `AstNode`, so storage cannot be half-slots/half-linked-list at runtime. The "bridge" (slot
+   accessors implemented over the linked list, then flip to fields) is awkward precisely where
+   the models differ most: `childIndex` is not naturally stored in a linked list, and sibling
+   nav is computed differently. The "coordinated flip" avoids the awkward bridge but is a big-
+   bang change. This single choice shapes the entire migration. -> SPIKE E1.
+   RESOLVED (E1, `3050aee76`): BRIDGE. Byte-neutral on the tested families (667/667 Pretty
+   identical, slot-order assert never tripped). Sibling nav stays on the linked list during the
+   bridge; `childIndex`/O(1) access + `AstNodeCollection`/`Clone`/sibling-nav all flip together at
+   3c. Prerequisite `Role.NullObjectUntyped`.
+C2. **Collection insert/remove renumbering is potentially O(n^2)** where the linked list was
+   O(1). `AstNodeCollection<T>` renumbers following children's `childIndex` on every insert/remove.
+   Large collections (a `SyntaxTree`'s members, a big `BlockStatement`) under insert-heavy
+   transforms could regress decompile time. IL's `InstructionCollection` has the same model and
+   is fine, but C# AST collections can be larger. -> SPIKE E2 (benchmark).
+   RESOLVED (E2, `dd58586b8`) -- and the framing was inverted: the linked list is the *slower*
+   model for realistic transforms (O(n) positional Get/IndexOf); plain eager renumber wins on every
+   real op mix. The O(n^2) trap is front-insert-with-no-reads only and negligible in absolute terms.
+   Decision: plain eager renumber, lazy dirty-watermark as a documented fallback.
+C3. **Printer fidelity after the token drop.** The spacing/parenthesis machinery
+   (`InsertRequiredSpacesDecorator`, `InsertParenthesesVisitor`, `InsertMissingTokensDecorator`)
+   may branch on token roles/presence; dropping token nodes could shift output whitespace or
+   parens subtly. Pretty tests are exact-text, so any drift fails loudly -- good, but the blast
+   radius is unknown until tried. -> SPIKE E3 (drop tokens on a slice, diff Pretty).
+   RESOLVED (E3, `6e29b23ec`): the spacing/parens concern is inert -- the printer is already
+   scalar-driven; dropping tokens was byte-neutral. The residual risk MOVED to
+   `InsertMissingTokensDecorator` on the AST-with-locations (GUI) path, which Pretty does not
+   exercise -- that is now the Phase 4 gating item.
+C4. **Inheritance re-declaration.** The chosen flatten model makes each of ~30 declarations
+   re-declare inherited contract slots (`override partial ... Attributes`). Repetitive, and a
+   mistake (wrong order/omission) is a silent document-order bug. The base-delegation
+   alternative trades this for offset arithmetic + non-sealed dispatch. -> SPIKE E4 (compare on
+   TypeMembers).
+   RESOLVED (E4, `352d1eb3c`): KEEP FLATTEN. Base-delegation is structurally incorrect here
+   (inherited slots are non-contiguous in source order -- `NameToken` after
+   `PrivateImplementationType` -- which a front-pinned base block cannot represent). The flatten
+   mis-order risk is generator-controlled (owns ordering + per-leaf contract-coverage self-check).
+C5. **Pattern placeholders in collection-initializer syntax** (`Statements = { new Repeat(...) }`)
+   must implicitly convert `Pattern -> Statement -> PatternPlaceholder` and `Add` to a
+   `AstNodeCollection<T>`. Lower risk -- the Phase 6 tests-first approach exercises it directly.
+C6. **Transform idioms**: `Detach`/`MoveTo`/`ReplaceWith`/cross-collection moves get subtle
+   slot-model rewrites; the renumbering must stay correct under move-then-readd. Covered by the
+   suite, but a source of fiddly bugs.
+C7. **NRT required/optional classification** (Phase 8): the current null-object model records no
+   such distinction. RESOLVED via R7 -- the rule is *syntactically required* (the C# grammar is the
+   objective oracle; the mechanical proxy is "`CSharpOutputVisitor` dereferences the slot
+   unconditionally" -- necessary but not sufficient: also check construction sites, since the
+   null-object no-op masks intentionally-empty slots, e.g. `IndexerExpression.Target`). Required ->
+   non-null accessor (the static contract); optional -> `T?`. Backing
+   fields are uniformly nullable (transform-friendly); `CheckInvariant` enforces required-non-null at
+   between-transform checkpoints. default-to-optional only as tie-breaker. Lowered from "main NRT
+   risk" to mechanical work.
+
+De-risking spikes (each isolated, parallel-worktree-friendly):
+- **E1 -- migration mechanic**: implement the Expressions family both ways (bridge over the
+  linked list vs coordinated flip to fields) and compare pain + reviewability. Decides Phase 3.
+- **E2 -- collection perf**: micro-benchmark `AstNodeCollection<T>` insert/remove vs the linked list
+  on a large method/file; decompile a big assembly and compare wall-clock. Decides whether
+  renumbering needs a smarter scheme (e.g. lazy/index-on-demand).
+- **E3 -- token-drop printer probe**: drop tokens on one expression + one statement, run the
+  Pretty suite, inspect the diff to size the spacing/parens rework.
+- **E4 -- inheritance shape**: flatten vs base-delegation on TypeMembers; compare generated-code
+  volume + ergonomics; confirm or revisit the flatten decision.
+
+## Open items
+- Phase 3 mechanic: RESOLVED to BRIDGE by spike E1 (see Phase 3 / Phase S).
+- Collection-renumber scheme: RESOLVED to plain eager + documented lazy fallback by spike E2.
+- `SymbolKind` generation (Phase 9) is optional; flags bit-packing (Phase 9) decided + deferred.
+- Remaining within-plan probes (not decisions): extend the slot-order assert to optional/reordered-
+  child families before the 3c flip (E1/N2); add regression coverage for the
+  `InsertMissingTokensDecorator` location path before the token drop (E3/N1); re-time a CoreLib
+  decompile after 3c against the ~33 s baseline (E2).
+- See PART III for the full review-findings backlog (the items below were folded in there).
+- **Post-rewrite (future): audit the AST against Roslyn's model and remove warts.** The slot model
+  + trivia channel + required/optional nullability already converge on Roslyn's shape, so a parity
+  pass becomes tractable once the rewrite lands. Known seed: `IndexerExpression` with null `Target`
+  for implicit-target indexers (Roslyn: a distinct `ImplicitElementAccess`). Other candidates to
+  check: object-initializer member names (bare `IdentifierExpression` vs an assignment shape),
+  node-kind granularity where one node + a flag collapses distinct C# constructs. Gated on the
+  rewrite being done; not a near-term task.
+
+==========================================================================================
+# PART III -- REVIEW FINDINGS & RESOLUTIONS (added 2026-06-13)
+==========================================================================================
+
+A code-grounded review of PARTS I/II surfaced the following holes, gaps, and decisions.
+Each is a tracked task; the "Amends" line names the section to update when the task is handled.
+Ordered as we will handle them.
+
+## R0 (task #4) -- Delete `IFreezable`/`Freeze`/`IsFrozen` [LEAD, decided]
+**Finding (verified):** `Freeze`/`IsFrozen`/`IFreezable` is entirely confined to `AstNode.cs`.
+No transform, builder, or consumer ever calls `Freeze()` (the only `.Freeze()` in the tree is an
+unrelated Avalonia brush in `CSharpHighlightingTokenWriter.cs`). The generated null-node singletons
+do not freeze themselves either, so every `IsFrozen` guard in `SetChild`/`AddChild` only ever
+evaluates false. The decompiler is single-threaded; nothing shares or freezes nodes.
+**Decision:** Drop the whole `IFreezable` apparatus in this rewrite (it is exactly the NRefactory
+full-fidelity baggage the Context section says to shed). Consequences to fold in:
+- `AstNode` no longer implements `IFreezable`; remove `Freeze`/`IsFrozen`/`frozenBit`.
+- `SetChildNode` (Part I.1) drops its frozen check.
+- `CheckInvariant` (Part I.13) drops the "frozen consistency" clause.
+- The freed flag bit is moot -- the `flags` word loses the role index anyway.
+**Amends:** Part I.1, Part I.13.
+
+## R1 (task #1) -- Comment carriers beyond statement/expression [base-model; before Phase 1/5]
+**Finding (verified, full enumeration):** today a `Comment` attaches via `Roles.Comment` to almost
+any node (the `InsertSpecialsDecorator` interleaves them at print time). `CommentStatement` +
+`CommentedExpression` (Part I.6) cover only 2 of the **6** receiver categories in use. (Doc
+comments -- `CommentType.Documentation` from `AddXmlDocumentationTransform` -- are separate and go
+to the `Documentation` slot.)
+Covered: block statement-list (-> `CommentStatement`: `StatementBuilder.cs:494,1362,1415`,
+`CSharpDecompiler.cs:1436,2049,2103`, `ExpressionBuilder.cs:2445,2623`); on an `Expression`
+(-> `CommentedExpression`: `CallBuilder.cs:567`, `ErrorExpression.cs:57`,
+`ExpressionBuilder.cs:2065,4967`).
+**Not covered -- six gaps:**
+- A. trailing comment on a **non-block `Statement`** -- `StatementBuilder.cs:140,1526,1544`.
+- B. **`TypeDeclaration.Members`** (sibling of `EntityDeclaration`) -- `CSharpDecompiler.cs:1698,
+  1727` ("error: enumerator has no value", "nested types are not permitted"). A `CommentStatement`
+  is a `Statement`, so it cannot live in a `Members` collection -- the one position that cannot be
+  faked with the two proposed nodes. (Corrects an earlier mistaken "never between members" claim.)
+- C. on an **`AstType`** -- `DeclareVariables.cs:616` ("pinned"), `TypeSystemAstBuilder.cs:414`.
+- D. on a **`VariableInitializer`** -- `CSharpDecompiler.cs:2322` (the "could not decompile" field
+  message).
+- E. on an **`Attribute`** -- `TypeSystemAstBuilder.cs:813` ("Could not decode attribute
+  arguments.").
+- F. on an **`Accessor`** (an `EntityDeclaration`, printed inline) -- `TypeSystemAstBuilder.cs:2123`
+  ("init").
+**RESOLVED -- comments become node trivia (leading/trailing), off the child model.**
+(LATER REFINED -- see authoritative Part I.6: the element type is a `Trivia` base covering BOTH
+`Comment` AND `PreProcessorDirective` (directives folded in, not separate marker nodes); storage is
+the existing annotation channel via a lazy `NodeTrivia` holder -- NO per-node fields -- so `Clone`
+is automatic via `CloneAnnotations`. The original sketch below predates those two refinements.)
+- **Storage:** a single per-node trivia collection on `AstNode`, a side-channel like annotations --
+  *outside* the flattened child-index space (so no `Comments` slot bloats every leaf's
+  `GetChild/SetChild` dispatch, no pattern-matcher interaction, `CheckInvariant`'s child-range
+  arithmetic stays clean). Each entry is tagged leading or trailing.
+- **Public API:** `AddLeadingTrivia(Trivia)` / `AddTrailingTrivia(Trivia)` -- the
+  leading/trailing distinction is encoded by which method is called; callers never set a flag.
+  Read-back via filtered `LeadingTrivia` / `TrailingTrivia` views.
+- **Print mechanic:** the output visitor emits leading trivia at `StartNode` (before the node's
+  children) and trailing at `EndNode` (after), preserving insertion order within each group. No
+  interleaving into the sibling sequence, no anchor index. Replaces `InsertSpecialsDecorator`'s
+  `Roles.Comment` interleaving + the `AstNode.cs` comment-skip walks.
+- **`Clone`** copies trivia alongside `CloneAnnotations()`; `CheckInvariant` treats trivia as
+  outside the child range (parent set, no `ChildIndex`).
+- **Doc comments** keep the dedicated `Documentation` slot (structural, rides with the member);
+  they are not folded into this trivia channel.
+- **Migration mapping** (the six categories -> the node each comment annotates):
+  A `StatementBuilder.cs:140` -> `AddTrailingTrivia`; `:1526,1544` -> `AddLeadingTrivia`.
+  B `CSharpDecompiler.cs:1698,1727` -> re-home from `typeDecl.InsertChildBefore(member,...)` to
+  `member.AddLeadingTrivia(...)`.
+  C `DeclareVariables.cs:616`, `TypeSystemAstBuilder.cs:414` -> `AddLeadingTrivia` on the `AstType`.
+  D `CSharpDecompiler.cs:2322` -> `AddTrailingTrivia` on the `VariableInitializer`.
+  E `TypeSystemAstBuilder.cs:813` -> `AddTrailingTrivia` on the `Attribute`.
+  F `TypeSystemAstBuilder.cs:2123` -> `AddTrailingTrivia` on the `Accessor`.
+- **Implementation guard:** assert no migrated site needs an *interior* comment (between two real
+  children of the same node where neither is the annotated node); the enumeration shows all are
+  first/last/leading-on-member, so the model is sufficient -- the assert catches a regression.
+**Amends:** Part I.6 (rewrite "comments as slot children" -> trivia channel), Phase 5 (drop
+`InsertSpecialsDecorator` trivia path; add the `AddLeading/TrailingTrivia` migration).
+
+## R2 (task #2) -- Locations: keep `StartLocation`/`EndLocation`, drop the `Span` tuple [RESOLVED]
+**Finding (verified, `AstNode.cs:183-199`):** today `StartLocation`/`EndLocation` are *virtual,
+computed by recursion* -- a non-leaf returns `firstChild.StartLocation` / `lastChild.EndLocation`,
+bottoming out at the **token leaves** (`CSharpTokenNode` etc.), which store the real location
+written at print time by `InsertMissingTokensDecorator` (`node.Location = ...`). The Phase-4 token
+drop removes those leaves, so the recursion loses its base case -- *that* is the only reason
+Part I.7 proposed explicit per-node storage. The stored values are still exactly
+`(TextLocation start, TextLocation end)`.
+**Decision -- no `Span` tuple; keep the two existing properties:**
+- **Public API:** keep `StartLocation` / `EndLocation` (each `TextLocation`). This *is* the facade
+  resolution -- zero churn across the ~33 readers (`SequencePointBuilder` + the `GetNodeAt`/
+  `GetNodesBetween`/`Contains` tree-nav in `AstNode` itself), and consistent with #11. A `Span`
+  tuple would force rewriting all 33 sites for no semantic gain. `Span` is removed from the design.
+- **Storage:** two backing `TextLocation` fields (a private struct is an impl detail, not exposed)
+  with an `internal` setter / `SetLocations(start, end)`, written by the existing `StartNode`/
+  `EndNode` + `WrapInWriterThatSetsLocationsInAST` path Part I.7 already keeps.
+- **Drop the `virtual` compute-from-children recursion:** every printed node is bracketed by
+  `StartNode`/`EndNode` and stores its own span directly; nothing needs to derive from children
+  once leaves no longer carry locations.
+- **Timing -- no computed fallback needed:** locations are only meaningful after the format pass
+  *today too* (token-leaf locations are `Empty` until `InsertMissingTokensDecorator` runs). All
+  ~33 readers are post-format (`SequencePointBuilder` on formatted output; UI nav on the printed
+  tree); no mid-pipeline reader exists.
+- Token-drop consequence (already in Part I.7): `SequencePointBuilder`'s brace breakpoints move
+  from `LBraceToken`/`RBraceToken` locations to `BlockStatement` start/end.
+**Amends:** Part I.1 (replace the `Span` property with stored `StartLocation`/`EndLocation` +
+internal setter; drop the `virtual` recursion), Part I.7 (s/`Span`/`StartLocation`+`EndLocation`/).
+
+## R3 (task #3) -- Full `NodeCollection<T>` API surface [before Phase 1]
+**Finding (verified):** Parts I.1/I.4 mention only Add/Insert/remove. The actually-used
+`AstNodeCollection<T>` surface a replacement must cover: `MoveTo` (cross-collection move -- the C6
+case), `ReplaceWith(IEnumerable)`, `AddRange`, `InsertBefore`/`InsertAfter(item,...)` (by reference
+node, **not** index), `Detach`, `AcceptVisitor`, indexer/`Count`/`Contains`/`CopyTo`, and
+`FirstOrNullObject`/`LastOrNullObject`. The last two are entangled with Phase 8 -- they become
+`FirstOrDefault`-returning-`null` once null objects die.
+**Amends:** Part I.1.
+
+## R4 (task #5) -- `AttributeTarget` enum completeness [before Phase 4]
+**Finding (verified):** Part I.5 converts the string target to an enum, but the old commented-out
+enum `{None,Assembly,Module,Type,Param,Field,Return,Method}` omits `property`, which
+`TransformFieldAndConstructorInitializers.cs` actually sets (`= "property"`).
+**Action:** enumerate the complete set (assembly, module, type, param, field, return, method,
+property, event -- verify `event`) before generating.
+**Amends:** Part I.5.
+
+## R5 (task #6) -- Add `SetChildByRole` to the deletion list [Phase 7]
+**Finding (verified):** Part I.9 lists `GetChildByRole`/`GetChildrenByRole`/`AddChild`/
+`InsertChild*` but omits `SetChildByRole` (used in `AttributeSection.AttributeTarget` setter, etc.).
+**Amends:** Part I.9, Phase 7.
+
+## R6 (task #7) -- Empty single-slot index occupancy [doc clarity; Phase 1/8]
+**Finding:** For O(1) fixed-slot access plus the Part I.13 round-trip invariant
+(`GetChild(child.ChildIndex) == child`), an empty optional **single** slot must still occupy its
+flattened index (return `null` at a fixed position); only **collection** slots vary the offset.
+Part I.4 only describes collections shifting offsets, leaving two readings.
+**Action:** state that single slots are always width-1, so the flattened index space is stable
+across the Phase 8 null->nullable transition.
+**Amends:** Part I.4.
+
+## R7 (task #8) -- Required/optional model: nullable field, required accessor, checkpoint-enforced [RESOLVED]
+**Question raised:** are any slots actually "required"? Yes -- binary operands, `IfStatement.Condition`/
+`TrueStatement`, loop condition/body, `CastExpression.Type`/`Expression`, assignment sides,
+`MemberReferenceExpression`/`InvocationExpression.Target`, etc. are always filled in a finished tree
+and an empty one is a bug. (Genuinely optional: `ReturnStatement.Expression`, `MethodDeclaration.Body`,
+`IfStatement.FalseStatement`, property accessors.) But every node has a parameterless ctor + settable
+slots and the decompiler builds incrementally (`BinaryOperatorExpression.cs:63-91`), so a required
+field is transiently empty during construction -- "required" cannot mean non-null for the node's whole
+lifetime.
+**Principle (from the user):** during a transform every slot is effectively optional (may be empty);
+`CheckInvariant` -- which runs *between* transforms -- guarantees the required ones exist at those
+checkpoints.
+**Resolution -- split storage from contract:**
+- **Backing field is always nullable.** This is "optional during a transform": any slot may be
+  detached/left empty mid-pipeline; transforms never fight the type system; construction stays
+  incremental; no `= null!` lie. (Note: this means single optional/required slots both store a
+  nullable field -- the field is uniformly nullable; only the *accessor* differs.)
+- **Accessor nullability IS the required marker** (single source of truth, per Part I.2: declared
+  nullability = `IsOptional`). Required -> non-null accessor `Expression Left` (getter returns
+  `field!` -- throws if read while empty); optional -> `Expression? Body`.
+- **`CheckInvariant` reads `CSharpSlotInfo.IsOptional`** and asserts required slots non-null at each
+  post-transform checkpoint -- "make sure the nodes exist," at the stable boundaries, not every instant.
+- **Contract:** between transforms the tree is whole (required filled); within a transform anything
+  goes (field tolerates null). Reading a transiently-empty required child throws at the bug site;
+  forgetting to refill is caught by `CheckInvariant` at the boundary.
+**This closes #8:** no special `Detach()`/`Remove()` rule -- transient-null is allowed by the field,
+existence is enforced at checkpoints, misuse throws at read.
+**Payoff preserved:** ergonomic non-null reads at the hundreds of `node.Left.AcceptVisitor(...)`
+sites (no `T?` warning flood -- the only reason null objects existed); still deletes the 18 null-node
+classes + `IsNull`/`VisitNullNode` plumbing.
+**Classification rule -- "syntactically required" (objective oracle, replaces per-slot judgment):**
+a slot is required (non-null accessor = the static contract) iff the **C# grammar** cannot omit it
+(`BinaryOperatorExpression.Left`/`Right`, `IfStatement.Condition`/`TrueStatement`, loop condition+
+body, `CastExpression.Type`/`Expression`, assignment sides, `...Target`); optional iff the grammar
+allows it absent (`ReturnStatement.Expression`, `MethodDeclaration.Body`, `IfStatement.FalseStatement`,
+`VariableInitializer` initializer, property accessors, attribute args). **Mechanical proxy:** a slot
+is required iff `CSharpOutputVisitor` dereferences it *unconditionally* (no `if (x != null)` guard /
+no alternate-token branch). The printer is ground truth -- it is the consumer that would NRE, and its
+access pattern necessarily matches the grammar -- so Phase 8 reads the output visitor rather than
+guessing. This de-risks C7. `default-to-optional` survives only as the tie-breaker for a genuinely
+ambiguous slot.
+**Residual:** classification is still per-slot work, but now objective and single-homed (accessor
+nullability feeds both the type and `CheckInvariant`).
+**Verified (and rule refined):** the "unconditional-deref" proxy is necessary but not sufficient --
+the null-object no-op can mask a legitimately-empty slot, so construction sites must also be checked.
+Landmine found: `IndexerExpression.Target` is visited unconditionally yet intentionally empty in
+object initializers (`CallBuilder.cs:1690`) and property-pattern index paths
+(`ExpressionBuilder.cs:3585`) -> classify **optional**, and guard it in `VisitIndexerExpression`.
+This is the **sole** null-`Target` case: `MemberReferenceExpression`/`InvocationExpression` `Target`
+are never null-constructed (`this.A` is an MRE with a `ThisReferenceExpression` target, bare `A` is
+an `IdentifierExpression` -- no implicit-`this` null-target), so both stay required. `ErrorExpression`
+is always a complete top-level node (never a stand-in child), so error paths never empty a required
+slot. The IndexerExpression null-target is a modeling wart (implicit-target indexer as
+`IndexerExpression`); Phase-8 may either make `Target` optional (recommended) or give the
+implicit-target indexer its own node.
+**Amends:** Part I.11 (nullable field + required-accessor model; drop "required slot = non-null
+field"), C7, Part I.13 (the "required slots non-null" clause sources `IsOptional`), Phase 8.
+
+## R8 (task #9) -- The bridge is not output-neutral [Phase 3]
+**Finding:** The Phase 3a bridge maps `GetChild(i)` to "the i-th role-ordered child," but
+linked-list children are in **insertion** order while slot order is **schema** order. The moment a
+family converts, the printer (walking `Children`) sees schema order. Today these coincide *except*
+the `TransformFieldAndConstructorInitializers` reordering hack -- so the bridge should be neutral,
+but any other latent insertion-vs-schema divergence surfaces as a Pretty diff at **bridge** time,
+not flip time. The per-family gate ("slot accessors agree with old role accessors") checks per-role
+identity, not document order.
+**Action:** lean on the Pretty suite at bridge time, or add a document-order assertion to the gate.
+**CONFIRMED by E1 (`3050aee76`):** across all 667 decompiled fixtures the bridge produced
+byte-identical Pretty output and the document-order assert never tripped for the tested families --
+so the gate IS the slot-order==document-order invariant of Phase 3a, prototyped and proven for
+simple Expressions. Still to do: extend the assert to optional/reordered-child families
+(modifiers/attributes, the EntityDeclaration interleave from E4) before the 3c flip.
+**Amends:** C1, Phase 3.
+
+## R10 (task #11) -- Keep original type names; drop the `NodeCollection` rename [decided]
+**Finding:** The design is asymmetric -- it keeps `AstNode` but renames `AstNodeCollection<T>` ->
+`NodeCollection<T>` to mirror IL's `InstructionCollection<T>` (Parts I.1, I.9, I.10, Phase
+6.5/6.7, Phase 7, Files, R3).
+**Decision:** Keep **both** original names. Reimplement `AstNodeCollection<T>` over slots
+(list-backed, parent+slot, renumbering) rather than delete+rename it. Rationale: (1) renaming a
+heavily-referenced public type is a gratuitous API break on top of the necessary roles/tokens/
+nulls removal, against `reduce-api-diff` and noisier in the Phase 7/9 API-diff gate; (2) `AstNode`/
+`AstNodeCollection<T>`/`AstType` are a name family -- keeping `AstNode` while renaming the
+collection breaks the symmetry; (3) the IL parallel is an implementation detail, not a name.
+**New public names that DO stay:** `CSharpSlotInfo` (no prior type) and `node.Slot` (replaces
+`node.Role`).
+**Sweep:** replace every `NodeCollection<T>` with `AstNodeCollection<T>` and change "delete
+`AstNodeCollection<T>`" to "reimplement over slots" in Parts I.1, I.9, I.10, Phase 6.5/6.7,
+Phase 7, the Files list, and R3 above.
+**Amends:** Parts I.1, I.9, I.10, Phase 6, Phase 7, Files.
+
+## R9 (task #10) -- Resolved open items (recorded)
+- **Directives: CONFIRMED no bracketing.** Only `#define` is emitted today; region/if/endif/pragma
+  are never constructed (the printer's generic `WritePreProcessorDirective` *can* emit them, but no
+  site does). (LATER REFINED -- see Part I.6: directives are NOT separate marker nodes; they are
+  folded into the leading/trailing `Trivia` channel alongside comments. `#define` -> leading trivia
+  on the `SyntaxTree`. The no-bracketing finding is what makes trivia sufficient.)
+- **Partial properties / LangVersion: non-issue.** `ICSharpCode.Decompiler` is single-target
+  `netstandard2.0` with `<LangVersion>14</LangVersion>`; partial properties (C# 13+) are
+  compile-time and available. (The lib is *not* multi-targeted, contrary to the CLAUDE.md framing;
+  the `init`-breakage constraint is a `netstandard2.0`-runtime `IsExternalInit` concern, orthogonal
+  to this.)
+- **Companion proof file** (`~/.claude/plans/rustling-bubbling-oasis-...md`) is load-bearing for
+  Phase 6 -- confirm it still exists and matches this revision before starting Phase 6.

Spikes/phaseS-workflow.js

@@ -0,0 +1,166 @@
+export const meta = {
+  name: 'phase-s-spikes',
+  description: 'Phase S de-risking spikes (E1-E4) for the slot-based C# AST rewrite, in isolated worktrees, then synthesis',
+  phases: [
+    { title: 'Spikes', detail: 'E1 migration mechanic, E2 collection perf, E3 token-drop printer, E4 inheritance shape' },
+    { title: 'Synthesis', detail: 'fold spike findings into the Phase 1/3 decisions' },
+  ],
+}
+
+// Shared context handed to every spike: build discipline + branch state.
+// (The design doc ROLES_FREE_SLOT_AST_DESIGN.md is untracked, so worktrees won't have it;
+//  each prompt carries the spike's relevant slice inline.)
+const COMMON = `
+Repo: ICSharpCode ILSpy decompiler. You are on a fresh git worktree branched off commit 76a965ae2
+("Drop dead IFreezable/Freeze apparatus from the C# AST"). The C# AST lives in
+ICSharpCode.Decompiler/CSharp/Syntax/ and currently uses the NRefactory role + doubly-linked-list
+child model (per-node firstChild/nextSibling, each child tagged with a Role; GetChildByRole is a
+linear scan). The goal of the larger rewrite is a slot-based model (fixed source-ordered slots,
+O(1) access) generated by the Roslyn incremental generator in
+ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs.
+
+BUILD DISCIPLINE (critical on this box):
+- Prefix every dotnet build/test with  OPENSSL_ENABLE_SHA1_SIGNATURES=1
+- Pass  -p:RestoreEnablePackagePruning=false  on restore/build to avoid pruning packages.lock.json.
+- Do NOT commit packages.lock.json changes. Keep your prototype commits to source only.
+- The decompiler lib is single-target netstandard2.0, LangVersion 14.
+
+TEST SETUP (required before running ANY decompiler test in your worktree):
+- The ILSpy-tests submodule is uninitialized; the test harness needs <repo>/ILSpy-tests/nuget to
+  exist (matrix compiler / reference-assembly cache) or every test Assert.Fails at fixture setup.
+  Run once in your worktree root:  ln -s /home/siegfried/Projects/ILSpy/ILSpy-tests/nuget ILSpy-tests/nuget
+- That cache is INCOMPLETE and there is NO network: matrix configs needing absent packages
+  (UseRoslyn4_14_0, UseRoslynLatest, TargetNet40, ...) fail at COMPILE setup with a Path.Exists
+  assert in RefAssembliesToolset.GetPath. Those failures are ENVIRONMENTAL, not decompiler bugs --
+  ignore them and focus on the toolset configs that actually compile (the default/cached compiler;
+  ~1195 cases run today). Filter your test runs accordingly and never attribute a Path.Exists /
+  RefAssembliesToolset failure to a code change.
+
+This is a SPIKE: prototype only enough to answer the question with evidence. Commit your prototype
+in your worktree (so the diff is inspectable) but it will NOT be merged. Be concrete and quantitative.
+`
+
+const FINDINGS = {
+  type: 'object',
+  additionalProperties: false,
+  required: ['spike', 'question', 'whatIDid', 'findings', 'recommendation', 'confidence'],
+  properties: {
+    spike: { type: 'string' },
+    question: { type: 'string', description: 'the decision this spike informs' },
+    whatIDid: { type: 'string' },
+    findings: { type: 'string', description: 'key observations, with file refs and numbers where relevant' },
+    recommendation: { type: 'string' },
+    confidence: { type: 'string', enum: ['low', 'medium', 'high'] },
+    metrics: { type: 'string', description: 'concrete numbers: LOC, diff size, perf timings, generated-code volume; "n/a" if none' },
+    prototypeCommit: { type: 'string', description: 'sha of the prototype commit in this worktree, or "none"' },
+    risksFound: { type: 'string', description: 'anything that surprised you or threatens the design' },
+  },
+}
+
+const SPIKES = [
+  {
+    key: 'E1', label: 'E1-migration-mechanic',
+    prompt: `${COMMON}
+SPIKE E1 -- migration mechanic (gates the Phase 3 plan; HIGHEST VALUE).
+Because all ~110 nodes share AstNode, the base storage cannot be half-slots/half-linked-list at
+runtime. Two candidate mechanics:
+ (a) BRIDGE: implement slot accessors (GetChild(i)/SetChild/GetChildCount) OVER the existing linked
+     list first -- GetChild(i) returns the i-th child in slot/schema order (map slot index -> Role,
+     then find that role's child/children). Convert node *declarations* to partial properties
+     family-by-family while storage stays linked-list, then a single later "flip to fields" step.
+ (b) COORDINATED FLIP: convert storage to real backing fields in one coordinated change on a
+     sub-branch, generator-assisted, per-family commits.
+Take the Expressions family as the testbed. Actually PROTOTYPE the bridge (a) for 2-3 representative
+nodes: BinaryOperatorExpression (two single slots), InvocationExpression (single slot + a collection
+slot), and IdentifierExpression (leaf). Implement the slot accessors over the linked list, add a
+transitional debug assert that slot order == document order, build green. Then assess what the
+coordinated-flip (b) would require for the same nodes (sketch, don't fully build). Report: which is
+more reviewable, where the bridge is awkward (childIndex over a linked list, sibling nav), whether
+document-order diverges from insertion order anywhere you touched, and a clear recommendation for
+the Phase 3 mechanic with confidence.`,
+  },
+  {
+    key: 'E2', label: 'E2-collection-perf',
+    prompt: `${COMMON}
+SPIKE E2 -- collection insert/remove renumbering perf (decides whether AstNodeCollection<T> needs a
+smarter index scheme). The slot model gives each node a flattened childIndex; a List-backed
+AstNodeCollection<T> renumbers following children's childIndex on every insert/remove -> potentially
+O(n^2) on large collections (a SyntaxTree's members, a big BlockStatement) under insert-heavy
+transforms, where the linked list was O(1). Write a standalone micro-benchmark (a small console
+project or a BenchmarkDotNet harness under a temp folder) comparing: (1) List-backed with
+renumber-on-every-mutation, (2) a linked-list baseline, across realistic sizes (e.g. 100, 1000,
+5000 elements) and realistic op mixes (append-heavy build, mid-insert-heavy transform, detach/move).
+Run it, collect timings. Then, as a sanity check on real workloads, decompile a large assembly with
+the existing ilspycmd (build it: ICSharpCode.ILSpyCmd) and note wall-clock (this is the linked-list
+baseline today). Report concrete numbers and whether renumbering is a real risk or a non-issue, with
+a recommendation (e.g. plain renumber vs lazy/index-on-demand vs gap-buffer).`,
+  },
+  {
+    key: 'E3', label: 'E3-token-drop-printer',
+    prompt: `${COMMON}
+SPIKE E3 -- token-drop printer probe (sizes the Phase 4 spacing/parens rework). The rewrite drops
+CSharpTokenNode/CSharpModifierToken child slots; their meaning moves to scalars and the printer is
+handed text directly. Risk: the spacing/paren machinery (InsertRequiredSpacesDecorator,
+InsertParenthesesVisitor, InsertMissingTokensDecorator, InsertSpecialsDecorator) may branch on token
+roles/presence, so dropping tokens could shift whitespace/parens. PROBE: pick ONE expression node
+(e.g. BinaryOperatorExpression) and ONE statement, remove their token child slots / token-role
+plumbing in the printer path for just those, and run the Pretty test suite
+(ICSharpCode.Decompiler.Tests, the Pretty/exact-text cases) to see what breaks. Inspect the diffs.
+Report: how many Pretty cases drift, the categories of drift (spacing vs parens vs missing
+punctuation), which decorators branch on token roles, and an estimate of the Phase 4 blast radius
+with confidence. You do NOT need the full suite green; you need the size and shape of the fallout.`,
+  },
+  {
+    key: 'E4', label: 'E4-inheritance-shape',
+    prompt: `${COMMON}
+SPIKE E4 -- inheritance shape (confirm or revisit the "flatten" decision). The design flattens:
+base classes declare shared child-slot members abstract (the contract), each concrete leaf
+re-declares its FULL slot set as partial properties in document order (override-implementing the
+contract), and the generator emits one flat sealed dispatch per leaf -- no base-delegation, no
+offset arithmetic. The alternative is base-delegation (base owns its slots, derived adds an offset).
+Use the TypeMembers family (EntityDeclaration contract + leaves like MethodDeclaration,
+PropertyDeclaration, FieldDeclaration) as the testbed. Hand-write (or generator-sketch) BOTH shapes
+for 2-3 of those leaves: (1) flatten -- leaf re-declares Attributes/Modifiers/Name + its own slots,
+flat sealed GetChild/SetChild; (2) base-delegation -- base implements its slots, leaf delegates +
+offsets. Compare generated-code volume (lines per leaf), ergonomics, and error-proneness (the
+flatten model risks a silent document-order bug if a leaf mis-orders/omits a re-declared contract
+slot). Report which is better for the C# AST and whether to keep the flatten decision, with
+confidence.`,
+  },
+]
+
+phase('Spikes')
+const results = await parallel(SPIKES.map(s => () =>
+  agent(s.prompt, { label: s.label, phase: 'Spikes', schema: FINDINGS, isolation: 'worktree' })
+))
+const findings = results.filter(Boolean)
+log(`${findings.length}/4 spikes returned`)
+
+phase('Synthesis')
+const synthesis = await agent(
+  `You are synthesizing the Phase S de-risking spikes for the slot-based C# AST rewrite. Here are the
+${findings.length} spike findings as JSON:
+
+${JSON.stringify(findings, null, 2)}
+
+Produce a synthesis that resolves the open Phase-1/Phase-3 decisions the spikes were meant to settle:
+- Phase 3 mechanic: bridge vs coordinated-flip (from E1).
+- AstNodeCollection<T> renumbering scheme: plain vs smarter (from E2).
+- Phase 4 token-drop blast radius and any reordering of work (from E3).
+- Inheritance shape: keep flatten vs switch to base-delegation (from E4).
+For each, give the decision, the evidence that drove it, and the confidence. Then list any NEW risks
+the spikes surfaced and any proposed edits to ROLES_FREE_SLOT_AST_DESIGN.md (by section). Be decisive
+where the evidence allows; flag where a spike was inconclusive and what further probe would settle it.`,
+  { label: 'synthesis', phase: 'Synthesis', schema: {
+    type: 'object', additionalProperties: false,
+    required: ['decisions', 'newRisks', 'docEdits'],
+    properties: {
+      decisions: { type: 'string' },
+      newRisks: { type: 'string' },
+      docEdits: { type: 'string', description: 'proposed edits to the design doc, by section' },
+      inconclusive: { type: 'string', description: 'spikes that did not settle their question + the follow-up probe' },
+    },
+  } }
+)
+
+return { findings, synthesis }