diff --git a/ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs b/ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs index 851fd65ce..f79cb4535 100644 --- a/ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs +++ b/ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs @@ -30,7 +30,7 @@ namespace ICSharpCode.Decompiler.Generators; [Generator] internal class DecompilerSyntaxTreeGenerator : IIncrementalGenerator { - record AstNodeAdditions(string NodeName, bool NeedsAcceptImpls, bool NeedsVisitor, bool NeedsNullNode, bool NeedsPatternPlaceholder, int NullNodeBaseCtorParamCount, bool IsTypeNode, string VisitMethodName, string VisitMethodParamType, EquatableArray<(string Member, string TypeName, bool RecursiveMatch, bool MatchAny)>? MembersToMatch, EquatableArray<(string RoleExpr, bool IsCollection, string PropertyName, string PropertyType, bool IsOverride)>? Slots); + record AstNodeAdditions(string NodeName, bool NeedsAcceptImpls, bool NeedsVisitor, bool NeedsNullNode, bool NeedsPatternPlaceholder, int NullNodeBaseCtorParamCount, bool IsTypeNode, string VisitMethodName, string VisitMethodParamType, EquatableArray<(string Member, string TypeName, bool RecursiveMatch, bool MatchAny)>? MembersToMatch, EquatableArray<(string RoleExpr, bool IsCollection, string PropertyName, string PropertyType, string ElementType, bool IsOverride, bool IsNullable)>? Slots); AstNodeAdditions GetAstNodeAdditions(GeneratorAttributeSyntaxContext context, CancellationToken ct) { @@ -78,7 +78,7 @@ internal class DecompilerSyntaxTreeGenerator : IIncrementalGenerator // Collect the slot schema: child properties tagged [Slot], in declaration order. The // attribute names the Role expression to use; single vs collection comes from the type. - List<(string RoleExpr, bool IsCollection, string PropertyName, string PropertyType, bool IsOverride)>? slots = null; + List<(string RoleExpr, bool IsCollection, string PropertyName, string PropertyType, string ElementType, bool IsOverride, bool IsNullable)>? slots = null; foreach (var m in targetSymbol.GetMembers()) { if (m is not IPropertySymbol property) @@ -89,8 +89,14 @@ internal class DecompilerSyntaxTreeGenerator : IIncrementalGenerator slots ??= new(); string roleExpr = (string)slotAttr.ConstructorArguments[0].Value!; bool isCollection = property.Type.MetadataName == "AstNodeCollection`1"; - string propertyType = property.Type.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat); - slots.Add((roleExpr, isCollection, property.Name, propertyType, property.IsOverride)); + bool isNullable = property.Type.NullableAnnotation == NullableAnnotation.Annotated; + var unannotated = property.Type.WithNullableAnnotation(NullableAnnotation.NotAnnotated); + string propertyType = unannotated.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat); + // For a collection slot, the element type is the single type argument of AstNodeCollection. + string elementType = isCollection + ? ((INamedTypeSymbol)property.Type).TypeArguments[0].ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat) + : propertyType; + slots.Add((roleExpr, isCollection, property.Name, propertyType, elementType, property.IsOverride, isNullable)); } return new(targetSymbol.Name, !targetSymbol.MemberNames.Contains("AcceptVisitor"), @@ -102,10 +108,16 @@ internal class DecompilerSyntaxTreeGenerator : IIncrementalGenerator visitMethodName, paramTypeName, membersToMatch?.ToEquatableArray(), slots?.ToEquatableArray()); } + // Backing-field name for a slot property. Prefixed to avoid colliding with hand-written fields + // in the same partial class; the generated file carries an auto-generated header, so naming-style + // rules do not apply. + static string FieldName(string propertyName) => "slot_" + propertyName; + void WriteGeneratedMembers(SourceProductionContext context, AstNodeAdditions source) { var builder = new StringBuilder(); + builder.AppendLine("// "); builder.AppendLine("namespace ICSharpCode.Decompiler.CSharp.Syntax;"); builder.AppendLine(); @@ -277,37 +289,107 @@ internal class DecompilerSyntaxTreeGenerator : IIncrementalGenerator { var slots = slotsArray.ToList(); - // Implementing half of each [Slot] partial property: stored over the linked list for now. - // A slot re-declared from an inherited contract member (Part I.3 flatten) is an override. - foreach (var (roleExpr, isCollection, name, type, isOverride) in slots) + // Backing fields and the partial-property bodies. A single slot stores a nullable field and + // substitutes the role's null object when empty (pre-NRT); a collection slot owns a lazily + // created AstNodeCollection bound to this node. A slot re-declared from an inherited contract + // member (Part I.3 flatten) is an override. + foreach (var (roleExpr, isCollection, name, type, elementType, isOverride, isNullable) in slots) { - builder.AppendLine($"\tpublic {(isOverride ? "override " : "")}partial {type} {name}"); - builder.AppendLine("\t{"); - builder.AppendLine($"\t\tget {{ return GetChild{(isCollection ? "ren" : "")}ByRole({roleExpr}); }}"); - if (!isCollection) - builder.AppendLine($"\t\tset {{ SetChildByRole({roleExpr}, value); }}"); - builder.AppendLine("\t}"); + string field = FieldName(name); + if (isCollection) + { + builder.AppendLine($"\tAstNodeCollection<{elementType}>? {field};"); + builder.AppendLine($"\tpublic {(isOverride ? "override " : "")}partial AstNodeCollection<{elementType}> {name} => {field} ??= new AstNodeCollection<{elementType}>(this, {roleExpr});"); + } + else + { + builder.AppendLine($"\t{type}? {field};"); + builder.AppendLine($"\tpublic {(isOverride ? "override " : "")}partial {type} {name}"); + builder.AppendLine("\t{"); + builder.AppendLine($"\t\tget => {field}{(isNullable ? "" : $" ?? {roleExpr}.NullObject")};"); + builder.AppendLine($"\t\tset => SetChildNode(ref {field}, value, {roleExpr});"); + builder.AppendLine("\t}"); + } } - // Slot schema (transitional bridge): source-ordered slots mapped to roles. - builder.AppendLine($"\tinternal override int SlotCount => {slots.Count};"); - builder.AppendLine("\tinternal override Role GetSlotRole(int slotIndex)"); + // Flattened child-index space: slots in declaration order, a single slot occupying one index + // (even when empty), a collection slot a contiguous run of its current length. + builder.Append("\tinternal override int GetChildCount() => "); + builder.Append(string.Join(" + ", slots.Select(s => s.IsCollection ? $"({FieldName(s.PropertyName)}?.Count ?? 0)" : "1"))); + builder.AppendLine(";"); + + builder.AppendLine("\tinternal override AstNode? GetChild(int index)"); builder.AppendLine("\t{"); - builder.AppendLine("\t\tswitch (slotIndex)"); - builder.AppendLine("\t\t{"); - for (int i = 0; i < slots.Count; i++) - builder.AppendLine($"\t\t\tcase {i}: return {slots[i].RoleExpr};"); - builder.AppendLine("\t\t\tdefault: throw new System.ArgumentOutOfRangeException(nameof(slotIndex));"); - builder.AppendLine("\t\t}"); + builder.AppendLine("\t\tint i = index;"); + foreach (var s in slots) + { + if (s.IsCollection) + { + string field = FieldName(s.PropertyName); + builder.AppendLine($"\t\t{{ int n = {field}?.Count ?? 0; if (i < n) return {field}![i]; i -= n; }}"); + } + else + { + builder.AppendLine($"\t\tif (i == 0) return {FieldName(s.PropertyName)}; i -= 1;"); + } + } + builder.AppendLine("\t\tthrow new System.ArgumentOutOfRangeException(nameof(index));"); builder.AppendLine("\t}"); - builder.AppendLine("\tinternal override bool IsCollectionSlot(int slotIndex)"); + + builder.AppendLine("\tinternal override void SetChild(int index, AstNode? value)"); builder.AppendLine("\t{"); - builder.AppendLine("\t\tswitch (slotIndex)"); - builder.AppendLine("\t\t{"); - for (int i = 0; i < slots.Count; i++) - builder.AppendLine($"\t\t\tcase {i}: return {(slots[i].IsCollection ? "true" : "false")};"); - builder.AppendLine("\t\t\tdefault: throw new System.ArgumentOutOfRangeException(nameof(slotIndex));"); - builder.AppendLine("\t\t}"); + builder.AppendLine("\t\tint i = index;"); + foreach (var s in slots) + { + if (s.IsCollection) + { + string field = FieldName(s.PropertyName); + builder.AppendLine($"\t\t{{ int n = {field}?.Count ?? 0; if (i < n) {{ {field}![i] = ({s.ElementType})value!; return; }} i -= n; }}"); + } + else + { + builder.AppendLine($"\t\tif (i == 0) {{ SetChildNode(ref {FieldName(s.PropertyName)}, ({s.PropertyType}?)value, {s.RoleExpr}); return; }} i -= 1;"); + } + } + builder.AppendLine("\t\tthrow new System.ArgumentOutOfRangeException(nameof(index));"); + builder.AppendLine("\t}"); + + builder.AppendLine("\tinternal override Role GetChildSlot(int index)"); + builder.AppendLine("\t{"); + builder.AppendLine("\t\tint i = index;"); + foreach (var s in slots) + { + if (s.IsCollection) + builder.AppendLine($"\t\t{{ int n = {FieldName(s.PropertyName)}?.Count ?? 0; if (i < n) return {s.RoleExpr}; i -= n; }}"); + else + builder.AppendLine($"\t\tif (i == 0) return {s.RoleExpr}; i -= 1;"); + } + builder.AppendLine("\t\tthrow new System.ArgumentOutOfRangeException(nameof(index));"); + builder.AppendLine("\t}"); + + if (slots.Any(s => s.IsCollection)) + { + builder.AppendLine("\tinternal override AstNodeCollection? GetCollectionByRole(Role role)"); + builder.AppendLine("\t{"); + foreach (var s in slots.Where(s => s.IsCollection)) + builder.AppendLine($"\t\tif (role == {s.RoleExpr}) return {s.PropertyName};"); + builder.AppendLine("\t\treturn base.GetCollectionByRole(role);"); + builder.AppendLine("\t}"); + } + + builder.AppendLine("\tinternal override void CloneChildrenInto(AstNode copyNode)"); + builder.AppendLine("\t{"); + builder.AppendLine($"\t\tvar copy = ({source.NodeName})copyNode;"); + foreach (var s in slots) + builder.AppendLine($"\t\tcopy.{FieldName(s.PropertyName)} = null;"); + foreach (var s in slots) + { + string field = FieldName(s.PropertyName); + if (s.IsCollection) + builder.AppendLine($"\t\tif ({field} != null) foreach (var c in {field}) copy.{s.PropertyName}.Add(({s.ElementType})c.Clone());"); + else + builder.AppendLine($"\t\tif ({field} != null) copy.{s.PropertyName} = ({s.PropertyType}){field}.Clone();"); + } builder.AppendLine("\t}"); } diff --git a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs index aeb9133d5..ca3861fb4 100644 --- a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs +++ b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs @@ -716,6 +716,9 @@ namespace ICSharpCode.Decompiler.CSharp { CancellationToken.ThrowIfCancellationRequested(); transform.Run(rootNode, context); + // Verify the slot structure survived the transform (DEBUG only); mirrors the IL + // pipeline's per-transform ILInstruction.CheckInvariant. + rootNode.CheckInvariant(); } CancellationToken.ThrowIfCancellationRequested(); rootNode.AcceptVisitor(new InsertParenthesesVisitor { InsertParenthesesForReadability = true }); @@ -1433,9 +1436,10 @@ namespace ICSharpCode.Decompiler.CSharp // Constraints are not copied because explicit interface implementations cannot have constraints. CS0460 methodDecl.Body = new BlockStatement(); - methodDecl.Body.AddChild(new Comment( - "ILSpy generated this explicit interface implementation from .override directive in " + memberDecl.Name), - Roles.Comment); + var commentStatement = new EmptyStatement(); + commentStatement.AddTrailingTrivia(new Comment( + "ILSpy generated this explicit interface implementation from .override directive in " + memberDecl.Name)); + methodDecl.Body.Add(commentStatement); var forwardingCall = new InvocationExpression(new MemberReferenceExpression(new ThisReferenceExpression(), memberDecl.Name, methodDecl.TypeParameters.Select(tp => new SimpleType(tp.Name))), methodDecl.Parameters.Select(ForwardParameter) diff --git a/ICSharpCode.Decompiler/CSharp/StatementBuilder.cs b/ICSharpCode.Decompiler/CSharp/StatementBuilder.cs index 9847bbba9..8d6d71168 100644 --- a/ICSharpCode.Decompiler/CSharp/StatementBuilder.cs +++ b/ICSharpCode.Decompiler/CSharp/StatementBuilder.cs @@ -1361,9 +1361,10 @@ namespace ICSharpCode.Decompiler.CSharp methodDecl.Parameters.Add(new ParameterDeclaration { ParameterModifier = ReferenceKind.In, Type = new SimpleType("T"), Name = "temp" }); methodDecl.Body = new BlockStatement(); - methodDecl.Body.AddChild(new Comment( - "ILSpy generated this function to help ensure overload resolution can pick the overload using 'in'"), - Roles.Comment); + var commentStatement = new EmptyStatement(); + commentStatement.AddTrailingTrivia(new Comment( + "ILSpy generated this function to help ensure overload resolution can pick the overload using 'in'")); + methodDecl.Body.Add(commentStatement); methodDecl.Body.Add(new ReturnStatement(new DirectionExpression(FieldDirection.Ref, new IdentifierExpression("temp")))); blockStatement.Statements.Add( diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs b/ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs index 163e6b28c..9d54a6275 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs +++ b/ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs @@ -44,10 +44,9 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax internal static readonly Role RootRole = new Role("Root", null); AstNode? parent; - AstNode? prevSibling; - AstNode? nextSibling; - AstNode? firstChild; - AstNode? lastChild; + // Flattened index of this node within its parent's child-index space (-1 when unparented). + // Recomputed lazily by the parent (EnsureChildIndices) after a structural mutation. + internal int childIndex = -1; // Flags, from least significant to most significant bits: // - Role.RoleIndexBits: role index @@ -192,36 +191,76 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax } public AstNode? NextSibling { - get { return nextSibling; } + get { + if (parent == null) + return null; + parent.EnsureChildIndices(); + int count = parent.GetChildCount(); + for (int i = childIndex + 1; i < count; i++) + { + AstNode? c = parent.GetChild(i); + if (c != null) + return c; + } + return null; + } } public AstNode? PrevSibling { - get { return prevSibling; } + get { + if (parent == null) + return null; + parent.EnsureChildIndices(); + for (int i = childIndex - 1; i >= 0; i--) + { + AstNode? c = parent.GetChild(i); + if (c != null) + return c; + } + return null; + } } public AstNode? FirstChild { - get { return firstChild; } + get { + int count = GetChildCount(); + for (int i = 0; i < count; i++) + { + AstNode? c = GetChild(i); + if (c != null) + return c; + } + return null; + } } public AstNode? LastChild { - get { return lastChild; } + get { + for (int i = GetChildCount() - 1; i >= 0; i--) + { + AstNode? c = GetChild(i); + if (c != null) + return c; + } + return null; + } } public bool HasChildren { get { - return firstChild != null; + return FirstChild != null; } } public IEnumerable Children { get { AstNode? next; - for (AstNode? cur = firstChild; cur != null; cur = next) + for (AstNode? cur = FirstChild; cur != null; cur = next) { Debug.Assert(cur.parent == this); // Remember next before yielding cur. - // This allows removing/replacing nodes while iterating through the list. - next = cur.nextSibling; + // This allows removing/replacing nodes while iterating. + next = cur.NextSibling; yield return cur; } } @@ -286,16 +325,18 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax Stack nextStack = new Stack(); nextStack.Push(null); - AstNode? pos = firstChild; + AstNode? pos = FirstChild; while (pos != null) { // Remember next before yielding pos. - // This allows removing/replacing nodes while iterating through the list. - if (pos.nextSibling != null) - nextStack.Push(pos.nextSibling); + // This allows removing/replacing nodes while iterating. + AstNode? posNext = pos.NextSibling; + if (posNext != null) + nextStack.Push(posNext); yield return pos; - if (pos.firstChild != null && (descendIntoChildren == null || descendIntoChildren(pos))) - pos = pos.firstChild; + AstNode? posFirstChild = pos.FirstChild; + if (posFirstChild != null && (descendIntoChildren == null || descendIntoChildren(pos))) + pos = posFirstChild; else pos = nextStack.Pop(); } @@ -309,11 +350,14 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax { if (role == null) throw new ArgumentNullException(nameof(role)); - uint roleIndex = role.Index; - for (var cur = firstChild; cur != null; cur = cur.nextSibling) + int count = GetChildCount(); + for (int i = 0; i < count; i++) { - if ((cur.flags & roleIndexMask) == roleIndex) - return (T)cur; + if (GetChildSlot(i) == role) + { + AstNode? c = GetChild(i); + return c != null ? (T)c : role.NullObject; + } } return role.NullObject; } @@ -330,81 +374,134 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax public AstNodeCollection GetChildrenByRole(Role role) where T : AstNode { - return new AstNodeCollection(this, role); + if (role == null) + throw new ArgumentNullException(nameof(role)); + AstNodeCollection? collection = GetCollectionByRole(role); + // A node has no children of a role it does not declare a collection slot for. Reads of such + // a role (e.g. the parameters of a non-indexer property) get a detached empty collection; + // writes go through AddChild/SetChildByRole, which reject a missing role. + return collection != null ? (AstNodeCollection)collection : new AstNodeCollection(this, role); } protected void SetChildByRole(Role role, T newChild) where T : AstNode { - AstNode oldChild = GetChildByRole(role); - if (oldChild.IsNull) - AddChild(newChild, role); - else - oldChild.ReplaceWith(newChild); + SetChildByRoleUntyped(role, newChild); } - #region Slot bridge - // Transitional slot-accessor layer implemented OVER the linked-list child model. The - // eventual slot model assigns each node type a fixed, source-ordered set of slots; here we - // expose that schema while storage is still the linked list. A "single slot" maps to one - // Role (at most one child of that role); a "collection slot" maps to a Role that may hold - // many children. GetChild returns the canonical child for a single slot, or the first child - // of a collection slot. - // - // Converted nodes override SlotCount/GetSlotRole/IsCollectionSlot. The default implementation - // reports zero slots, so unconverted nodes keep working unchanged and the slot-order - // invariant skips them. + #region Slot storage contract + // Each concrete node's slots form a flattened child-index space, in source-declaration order. + // A single slot occupies exactly one index (even when empty, where GetChild returns null); a + // collection slot occupies a contiguous run of its current length. The generator emits these + // four members per node from its [Slot] partial-property declarations; nodes without children + // (and the null/placeholder nodes) keep the zero-child defaults below. + + internal virtual int GetChildCount() => 0; + + internal virtual AstNode? GetChild(int index) => throw new ArgumentOutOfRangeException(nameof(index)); + + internal virtual void SetChild(int index, AstNode? value) => throw new ArgumentOutOfRangeException(nameof(index)); + + internal virtual Role GetChildSlot(int index) => throw new ArgumentOutOfRangeException(nameof(index)); - internal virtual int SlotCount => 0; + // Returns the collection occupying the slot with the given role, or null if there is none. + // Overridden by the generator for nodes that have collection slots. + internal virtual AstNodeCollection? GetCollectionByRole(Role role) => null; - internal virtual Role GetSlotRole(int slotIndex) + // Deep-copies this node's children into the (memberwise-cloned) copy, which initially shares + // this node's child references. Overridden by the generator for nodes that have slots. + internal virtual void CloneChildrenInto(AstNode copy) { } + + // Whether this node's children currently carry correct flattened childIndex values. Cleared on + // every structural mutation and restored lazily on the first index read, so bulk construction + // (many appends with no interleaved index reads) renumbers once instead of once per mutation. + bool childIndicesValid = true; + + // Marks this node's children's flattened indices stale. Called by the slot setters and the + // collection after any structural change. + internal void InvalidateChildIndices() { - throw new ArgumentOutOfRangeException(nameof(slotIndex)); + childIndicesValid = false; } - internal virtual bool IsCollectionSlot(int slotIndex) + // Assigns each child its flattened index if a mutation has invalidated them. The flattened-index + // arithmetic lives in the generated GetChild/GetChildCount, which depend only on the backing + // fields, so this is well-defined regardless of the current childIndex values. + void EnsureChildIndices() { - throw new ArgumentOutOfRangeException(nameof(slotIndex)); + if (childIndicesValid) + return; + childIndicesValid = true; + int count = GetChildCount(); + for (int i = 0; i < count; i++) + { + AstNode? c = GetChild(i); + if (c != null) + c.childIndex = i; + } } /// - /// Gets the child occupying the single slot at . - /// For collection slots this returns the first child of the slot's role. + /// Recursively verifies the slot structure of this subtree (DEBUG only): every child's Parent + /// points back here, its stored flattened index matches its slot position, and its runtime type + /// is valid in the slot's role. The transform pipeline runs this after each transform, the + /// analog of ILInstruction.CheckInvariant, so a transform that corrupts the tree fails at + /// the exact transform rather than as a downstream output diff. /// - internal AstNode GetChild(int slotIndex) + [System.Diagnostics.Conditional("DEBUG")] + internal void CheckInvariant() { - Role role = GetSlotRole(slotIndex); - for (var cur = firstChild; cur != null; cur = cur.nextSibling) + EnsureChildIndices(); + int count = GetChildCount(); + for (int i = 0; i < count; i++) { - if ((cur.flags & roleIndexMask) == role.Index) - return cur; + AstNode? child = GetChild(i); + if (child == null) + continue; // empty optional single slot + Debug.Assert(child.parent == this, "child's Parent must point back to this node"); + Debug.Assert(child.childIndex == i, "child's flattened index must match its slot position"); + Debug.Assert(GetChildSlot(i).IsValid(child), "child's type must be valid in its slot's role"); + child.CheckInvariant(); } - // Every role used for a single slot has a null object, so an AST property never returns null. - return (AstNode)role.NullObjectUntyped!; } - internal void SetChild(int slotIndex, AstNode? newChild) + // Writes a single-slot backing field: detaches the old child, attaches the new one, renumbers. + // A null or null-object value empties the slot. Called by generated single-slot property setters. + internal void SetChildNode(ref T? field, T? value, Role role) where T : AstNode { - if (IsCollectionSlot(slotIndex)) - throw new InvalidOperationException("SetChild is not valid for collection slots; mutate the collection instead."); - Role role = GetSlotRole(slotIndex); - AstNode oldChild = GetChild(slotIndex); - if (oldChild.IsNull) - { - if (newChild != null && !newChild.IsNull) - AddChildUnsafe(newChild, role); - } - else + T? newValue = (value == null || value.IsNull) ? null : value; + if (field == newValue) + return; + if (newValue != null) { - oldChild.ReplaceWith(newChild); + if (newValue == this) + throw new ArgumentException("Cannot add a node to itself as a child.", nameof(value)); + if (newValue.parent != null) + { + // Allow lifting a node out of the subtree being replaced, e.g. + // "assignment.Right = ((BinaryOperatorExpression)assignment.Right).Right;". + if (field != null && newValue.Ancestors.Contains(field)) + newValue.Remove(); + else + throw new ArgumentException("Node is already used in another tree.", nameof(value)); + } } + field?.ClearParentAndIndex(); + field = newValue; + newValue?.SetParentAndRole(this, role); + InvalidateChildIndices(); } - // Note: document-order == slot-order is a POST-FLIP invariant, not a bridge invariant. - // During the transitional bridge the linked list still carries positional token children - // (e.g. the Roles.Comma trailing-comma marker) and comments whose document position encodes - // output meaning, so it legitimately diverges from slot order. The invariant is re-introduced - // once storage flips to slots (Phase 3c), where it holds by construction; until then the - // Pretty suite gates output. See ROLES_FREE_SLOT_AST_DESIGN.md (R8 / Phase 3c). + internal void SetParentAndRole(AstNode newParent, Role role) + { + parent = newParent; + SetRole(role); + } + + internal void ClearParentAndIndex() + { + parent = null; + childIndex = -1; + } #endregion public void AddChild(T child, Role role) where T : AstNode @@ -413,10 +510,6 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax throw new ArgumentNullException(nameof(role)); if (child == null || child.IsNull) return; - if (child == this) - throw new ArgumentException("Cannot add a node to itself as a child.", nameof(child)); - if (child.parent != null) - throw new ArgumentException("Node is already used in another tree.", nameof(child)); AddChildUnsafe(child, role); } @@ -424,76 +517,72 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax { if (child == null || child.IsNull) return; - if (child == this) - throw new ArgumentException("Cannot add a node to itself as a child.", nameof(child)); - if (child.parent != null) - throw new ArgumentException("Node is already used in another tree.", nameof(child)); AddChildUnsafe(child, child.Role); } /// - /// Adds a child without performing any safety checks. + /// Adds a child into the slot matching (appending to a collection slot, + /// or filling a single slot). /// internal void AddChildUnsafe(AstNode child, Role role) { - child.parent = this; - child.SetRole(role); - if (firstChild == null) - { - lastChild = firstChild = child; - } + AstNodeCollection? collection = GetCollectionByRole(role); + if (collection != null) + collection.AddNode(child); else + SetChildByRoleUntyped(role, child); + } + + // Sets the single slot matching the role (used by the non-generic mutation API). Symmetric with + // GetChildByRole, which returns the null object for a role this node does not declare a slot for: + // a node has no child of a role it has no slot for, so writing one is a no-op. + internal void SetChildByRoleUntyped(Role role, AstNode? child) + { + int count = GetChildCount(); + for (int i = 0; i < count; i++) { - lastChild!.nextSibling = child; - child.prevSibling = lastChild; - lastChild = child; + if (GetChildSlot(i) == role) + { + SetChild(i, child == null || child.IsNull ? null : child); + return; + } } + throw new InvalidOperationException($"{GetType().Name} has no slot for role '{role}'."); } public void InsertChildBefore(AstNode? nextSibling, T child, Role role) where T : AstNode { if (role == null) throw new ArgumentNullException(nameof(role)); - if (nextSibling == null || nextSibling.IsNull) - { - AddChild(child, role); - return; - } - if (child == null || child.IsNull) return; - if (child.parent != null) - throw new ArgumentException("Node is already used in another tree.", nameof(child)); - if (nextSibling.parent != this) - throw new ArgumentException("NextSibling is not a child of this node.", nameof(nextSibling)); - // No need to test for "Cannot add children to null nodes", - // as there isn't any valid nextSibling in null nodes. - InsertChildBeforeUnsafe(nextSibling, child, role); + AstNodeCollection? collection = GetCollectionByRole(role); + if (collection != null) + collection.InsertNodeBefore((nextSibling == null || nextSibling.IsNull) ? null : nextSibling, child); + else + SetChildByRoleUntyped(role, child); } internal void InsertChildBeforeUnsafe(AstNode nextSibling, AstNode child, Role role) { - child.parent = this; - child.SetRole(role); - child.nextSibling = nextSibling; - child.prevSibling = nextSibling.prevSibling; - - if (nextSibling.prevSibling != null) - { - Debug.Assert(nextSibling.prevSibling.nextSibling == nextSibling); - nextSibling.prevSibling.nextSibling = child; - } + AstNodeCollection? collection = GetCollectionByRole(role); + if (collection != null) + collection.InsertNodeBefore(nextSibling, child); else - { - Debug.Assert(firstChild == nextSibling); - firstChild = child; - } - nextSibling.prevSibling = child; + SetChildByRoleUntyped(role, child); } public void InsertChildAfter(AstNode? prevSibling, T child, Role role) where T : AstNode { - InsertChildBefore((prevSibling == null || prevSibling.IsNull) ? firstChild : prevSibling.nextSibling, child, role); + if (role == null) + throw new ArgumentNullException(nameof(role)); + if (child == null || child.IsNull) + return; + AstNodeCollection? collection = GetCollectionByRole(role); + if (collection != null) + collection.InsertNodeAfter((prevSibling == null || prevSibling.IsNull) ? null : prevSibling, child); + else + SetChildByRoleUntyped(role, child); } /// @@ -501,32 +590,15 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax /// public void Remove() { - if (parent != null) - { - if (prevSibling != null) - { - Debug.Assert(prevSibling.nextSibling == this); - prevSibling.nextSibling = nextSibling; - } - else - { - Debug.Assert(parent.firstChild == this); - parent.firstChild = nextSibling; - } - if (nextSibling != null) - { - Debug.Assert(nextSibling.prevSibling == this); - nextSibling.prevSibling = prevSibling; - } - else - { - Debug.Assert(parent.lastChild == this); - parent.lastChild = prevSibling; - } - parent = null; - prevSibling = null; - nextSibling = null; - } + if (parent == null) + return; + parent.EnsureChildIndices(); + Role role = parent.GetChildSlot(childIndex); + AstNodeCollection? collection = parent.GetCollectionByRole(role); + if (collection != null) + collection.RemoveNode(this); + else + parent.SetChild(childIndex, null); } /// @@ -545,11 +617,13 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax { throw new InvalidOperationException(this.IsNull ? "Cannot replace the null nodes" : "Cannot replace the root node"); } + parent.EnsureChildIndices(); + Role role = parent.GetChildSlot(childIndex); // Because this method doesn't statically check the new node's type with the role, // we perform a runtime test: - if (!this.Role.IsValid(newNode)) + if (!role.IsValid(newNode)) { - throw new ArgumentException(string.Format("The new node '{0}' is not valid in the role {1}", newNode.GetType().Name, this.Role.ToString()), nameof(newNode)); + throw new ArgumentException(string.Format("The new node '{0}' is not valid in the role {1}", newNode.GetType().Name, role.ToString()), nameof(newNode)); } if (newNode.parent != null) { @@ -565,34 +639,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax throw new ArgumentException("Node is already used in another tree.", nameof(newNode)); } } - newNode.parent = parent; - newNode.SetRole(this.Role); - newNode.prevSibling = prevSibling; - newNode.nextSibling = nextSibling; - - if (prevSibling != null) - { - Debug.Assert(prevSibling.nextSibling == this); - prevSibling.nextSibling = newNode; - } - else - { - Debug.Assert(parent.firstChild == this); - parent.firstChild = newNode; - } - if (nextSibling != null) - { - Debug.Assert(nextSibling.prevSibling == this); - nextSibling.prevSibling = newNode; - } - else - { - Debug.Assert(parent.lastChild == this); - parent.lastChild = newNode; - } - parent = null; - prevSibling = null; - nextSibling = null; + parent.SetChild(childIndex, newNode); } public AstNode? ReplaceWith(Func replaceFunction) @@ -604,7 +651,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax throw new InvalidOperationException(this.IsNull ? "Cannot replace the null nodes" : "Cannot replace the root node"); } AstNode oldParent = parent; - AstNode? oldSuccessor = nextSibling; + AstNode? oldSuccessor = NextSibling; Role oldRole = this.Role; Remove(); AstNode? replacement = replaceFunction(this); @@ -634,22 +681,13 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax public AstNode Clone() { AstNode copy = (AstNode)MemberwiseClone(); - // First, reset the shallow pointer copies copy.parent = null; - copy.firstChild = null; - copy.lastChild = null; - copy.prevSibling = null; - copy.nextSibling = null; - - // Then perform a deep copy: - for (AstNode? cur = firstChild; cur != null; cur = cur.nextSibling) - { - copy.AddChildUnsafe(cur.Clone(), cur.Role); - } - + copy.childIndex = -1; + // Deep-copy the children (CloneChildrenInto first drops the shallow field copies that + // MemberwiseClone left pointing at this node's children). + CloneChildrenInto(copy); // Finally, clone the annotation, if necessary copy.CloneAnnotations(); - return copy; } @@ -686,11 +724,11 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax } PatternMatching.INode? PatternMatching.INode.NextSibling { - get { return nextSibling; } + get { return NextSibling; } } PatternMatching.INode? PatternMatching.INode.FirstChild { - get { return firstChild; } + get { return FirstChild; } } #endregion @@ -777,222 +815,6 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax return prev; } - #region GetNodeAt - /// - /// Gets the node specified by T at the location line, column. This is useful for getting a specific node from the tree. For example searching - /// the current method declaration. - /// (End exclusive) - /// - public AstNode? GetNodeAt(int line, int column, Predicate? pred = null) - { - return GetNodeAt(new TextLocation(line, column), pred); - } - - /// - /// Gets the node specified by pred at location. This is useful for getting a specific node from the tree. For example searching - /// the current method declaration. - /// (End exclusive) - /// - public AstNode? GetNodeAt(TextLocation location, Predicate? pred = null) - { - AstNode? result = null; - AstNode node = this; - while (node.LastChild != null) - { - var child = node.LastChild; - while (child != null && child.StartLocation > location) - child = child.prevSibling; - if (child != null && location < child.EndLocation) - { - if (pred == null || pred(child)) - result = child; - node = child; - } - else - { - // found no better child node - therefore the parent is the right one. - break; - } - } - return result; - } - - /// - /// Gets the node specified by T at the location line, column. This is useful for getting a specific node from the tree. For example searching - /// the current method declaration. - /// (End exclusive) - /// - public T? GetNodeAt(int line, int column) where T : AstNode - { - return GetNodeAt(new TextLocation(line, column)); - } - - /// - /// Gets the node specified by T at location. This is useful for getting a specific node from the tree. For example searching - /// the current method declaration. - /// (End exclusive) - /// - public T? GetNodeAt(TextLocation location) where T : AstNode - { - T? result = null; - AstNode node = this; - while (node.LastChild != null) - { - var child = node.LastChild; - while (child != null && child.StartLocation > location) - child = child.prevSibling; - if (child != null && location < child.EndLocation) - { - if (child is T) - result = (T)child; - node = child; - } - else - { - // found no better child node - therefore the parent is the right one. - break; - } - } - return result; - } - - #endregion - - #region GetAdjacentNodeAt - /// - /// Gets the node specified by pred at the location line, column. This is useful for getting a specific node from the tree. For example searching - /// the current method declaration. - /// (End inclusive) - /// - public AstNode? GetAdjacentNodeAt(int line, int column, Predicate? pred = null) - { - return GetAdjacentNodeAt(new TextLocation(line, column), pred); - } - - /// - /// Gets the node specified by pred at location. This is useful for getting a specific node from the tree. For example searching - /// the current method declaration. - /// (End inclusive) - /// - public AstNode? GetAdjacentNodeAt(TextLocation location, Predicate? pred = null) - { - AstNode? result = null; - AstNode node = this; - while (node.LastChild != null) - { - var child = node.LastChild; - while (child != null && child.StartLocation > location) - child = child.prevSibling; - if (child != null && location <= child.EndLocation) - { - if (pred == null || pred(child)) - result = child; - node = child; - } - else - { - // found no better child node - therefore the parent is the right one. - break; - } - } - return result; - } - - /// - /// Gets the node specified by T at the location line, column. This is useful for getting a specific node from the tree. For example searching - /// the current method declaration. - /// (End inclusive) - /// - public T? GetAdjacentNodeAt(int line, int column) where T : AstNode - { - return GetAdjacentNodeAt(new TextLocation(line, column)); - } - - /// - /// Gets the node specified by T at location. This is useful for getting a specific node from the tree. For example searching - /// the current method declaration. - /// (End inclusive) - /// - public T? GetAdjacentNodeAt(TextLocation location) where T : AstNode - { - T? result = null; - AstNode node = this; - while (node.LastChild != null) - { - var child = node.LastChild; - while (child != null && child.StartLocation > location) - child = child.prevSibling; - if (child != null && location <= child.EndLocation) - { - if (child is T t) - result = t; - node = child; - } - else - { - // found no better child node - therefore the parent is the right one. - break; - } - } - return result; - } - #endregion - - /// - /// Gets the node that fully contains the range from startLocation to endLocation. - /// - public AstNode GetNodeContaining(TextLocation startLocation, TextLocation endLocation) - { - for (AstNode? child = firstChild; child != null; child = child.nextSibling) - { - if (child.StartLocation <= startLocation && endLocation <= child.EndLocation) - return child.GetNodeContaining(startLocation, endLocation); - } - return this; - } - - /// - /// Returns the root nodes of all subtrees that are fully contained in the specified region. - /// - public IEnumerable GetNodesBetween(int startLine, int startColumn, int endLine, int endColumn) - { - return GetNodesBetween(new TextLocation(startLine, startColumn), new TextLocation(endLine, endColumn)); - } - - /// - /// Returns the root nodes of all subtrees that are fully contained between and (inclusive). - /// - public IEnumerable GetNodesBetween(TextLocation start, TextLocation end) - { - AstNode? node = this; - while (node != null) - { - AstNode? next; - if (start <= node.StartLocation && node.EndLocation <= end) - { - // Remember next before yielding node. - // This allows iteration to continue when the caller removes/replaces the node. - next = node.GetNextNode(); - yield return node; - } - else - { - if (node.EndLocation <= start) - { - next = node.GetNextNode(); - } - else - { - next = node.FirstChild; - } - } - - if (next != null && next.StartLocation > end) - yield break; - node = next; - } - } - /// /// Gets the node as formatted C# output. /// diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/AstNodeCollection.cs b/ICSharpCode.Decompiler/CSharp/Syntax/AstNodeCollection.cs index ca5028fd4..6db7fe77a 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/AstNodeCollection.cs +++ b/ICSharpCode.Decompiler/CSharp/Syntax/AstNodeCollection.cs @@ -1,14 +1,14 @@ // Copyright (c) 2010-2013 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 @@ -16,51 +16,82 @@ // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER // DEALINGS IN THE SOFTWARE. +#nullable enable + using System; using System.Collections; using System.Collections.Generic; -using System.Diagnostics; -using System.Linq; using ICSharpCode.Decompiler.CSharp.Syntax.PatternMatching; namespace ICSharpCode.Decompiler.CSharp.Syntax { /// - /// Represents the children of an AstNode that have a specific role. + /// Non-generic base of , letting the base + /// manipulate a collection slot (remove an element by reference) without knowing its element type. /// - public class AstNodeCollection : ICollection, IReadOnlyCollection + public abstract class AstNodeCollection + { + internal abstract void AddNode(AstNode node); + internal abstract void InsertNodeBefore(AstNode? existing, AstNode node); + internal abstract void InsertNodeAfter(AstNode? existing, AstNode node); + internal abstract bool RemoveNode(AstNode node); + } + + /// + /// Represents the children of an that occupy one collection slot. + /// The elements are stored in a list owned by the collection; the parent's flattened child-index + /// space contains them as a contiguous run, renumbered lazily by the parent after a mutation. + /// + public class AstNodeCollection : AstNodeCollection, ICollection, IReadOnlyList where T : AstNode { - readonly AstNode node; + readonly AstNode parent; readonly Role role; + readonly List list = new List(); - public AstNodeCollection(AstNode node, Role role) + public AstNodeCollection(AstNode parent, Role role) { - if (node == null) - throw new ArgumentNullException(nameof(node)); - if (role == null) - throw new ArgumentNullException(nameof(role)); - this.node = node; - this.role = role; + this.parent = parent ?? throw new ArgumentNullException(nameof(parent)); + this.role = role ?? throw new ArgumentNullException(nameof(role)); } public int Count { - get { - int count = 0; - uint roleIndex = role.Index; - for (AstNode cur = node.FirstChild; cur != null; cur = cur.NextSibling) - { - if (cur.RoleIndex == roleIndex) - count++; - } - return count; + get { return list.Count; } + } + + public T this[int index] { + get { return list[index]; } + set { + T old = list[index]; + if (old == value) + return; + ValidateNewChild(value); + old.ClearParentAndIndex(); + list[index] = value; + value.SetParentAndRole(parent, role); + parent.InvalidateChildIndices(); } } + void ValidateNewChild(T child) + { + if (child == null) + throw new ArgumentNullException(nameof(child)); + if (child == parent) + throw new ArgumentException("Cannot add a node to itself as a child.", nameof(child)); + if (child.Parent != null) + throw new ArgumentException("Node is already used in another tree.", nameof(child)); + } + public void Add(T element) { - node.AddChild(element, role); + if (element == null || element.IsNull) + return; + ValidateNewChild(element); + list.Add(element); + element.SetParentAndRole(parent, role); + parent.InvalidateChildIndices(); } public void AddRange(IEnumerable nodes) @@ -69,7 +100,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax // Example: collection.AddRange(collection); if (nodes != null) { - foreach (T node in nodes.ToList()) + foreach (T node in nodes is ICollection ? nodes : new List(nodes)) Add(node); } } @@ -88,12 +119,11 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax { // Evaluate 'nodes' first, since it might change when we call Clear() // Example: collection.ReplaceWith(collection); - if (nodes != null) - nodes = nodes.ToList(); + List? newNodes = nodes != null ? new List(nodes) : null; Clear(); - if (nodes != null) + if (newNodes != null) { - foreach (T node in nodes) + foreach (T node in newNodes) Add(node); } } @@ -102,7 +132,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax { if (targetCollection == null) throw new ArgumentNullException(nameof(targetCollection)); - foreach (T node in this) + foreach (T node in list.ToArray()) { node.Remove(); targetCollection.Add(node); @@ -111,47 +141,82 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax public bool Contains(T element) { - return element != null && element.Parent == node && element.RoleIndex == role.Index; + return element != null && element.Parent == parent && element.RoleIndex == role.Index; + } + + public int IndexOf(T element) + { + if (element == null || element.Parent != parent) + return -1; + return list.IndexOf(element); } public bool Remove(T element) { - if (Contains(element)) - { - element.Remove(); - return true; - } - else - { + int index = IndexOf(element); + if (index < 0) return false; - } + list.RemoveAt(index); + element.ClearParentAndIndex(); + parent.InvalidateChildIndices(); + return true; + } + + internal override void AddNode(AstNode node) + { + Add((T)node); + } + + internal override void InsertNodeBefore(AstNode? existing, AstNode node) + { + // 'existing' may belong to a different slot (e.g. the node after the last collection + // element); in that case it is not found here and the new node is appended. + if (existing is T e && IndexOf(e) >= 0) + InsertBefore(e, (T)node); + else + Add((T)node); + } + + internal override void InsertNodeAfter(AstNode? existing, AstNode node) + { + if (existing is T e && IndexOf(e) >= 0) + InsertAfter(e, (T)node); + else + Insert(0, (T)node); + } + + internal override bool RemoveNode(AstNode node) + { + return node is T typed && Remove(typed); } public void CopyTo(T[] array, int arrayIndex) { - foreach (T item in this) - array[arrayIndex++] = item; + list.CopyTo(array, arrayIndex); } public void Clear() { - foreach (T item in this) - item.Remove(); + foreach (T item in list) + item.ClearParentAndIndex(); + list.Clear(); + parent.InvalidateChildIndices(); } public IEnumerable Detach() { - foreach (T item in this) - yield return item.Detach(); + T[] items = list.ToArray(); + Clear(); + return items; } /// /// Returns the first element for which the predicate returns true, /// or the null node (AstNode with IsNull=true) if no such object is found. /// - public T FirstOrNullObject(Func predicate = null) + public T FirstOrNullObject(Func? predicate = null) { - foreach (T item in this) + foreach (T item in list) if (predicate == null || predicate(item)) return item; return role.NullObject; @@ -161,10 +226,10 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax /// Returns the last element for which the predicate returns true, /// or the null node (AstNode with IsNull=true) if no such object is found. /// - public T LastOrNullObject(Func predicate = null) + public T LastOrNullObject(Func? predicate = null) { T result = role.NullObject; - foreach (T item in this) + foreach (T item in list) if (predicate == null || predicate(item)) result = item; return result; @@ -174,18 +239,17 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax get { return false; } } + // Tolerates removing or replacing the current element during enumeration (the common + // transform pattern), matching the previous linked-list collection's behavior. public IEnumerator GetEnumerator() { - uint roleIndex = role.Index; - AstNode next; - for (AstNode cur = node.FirstChild; cur != null; cur = next) + int pos = 0; + while (pos < list.Count) { - Debug.Assert(cur.Parent == node); - // Remember next before yielding cur. - // This allows removing/replacing nodes while iterating through the list. - next = cur.NextSibling; - if (cur.RoleIndex == roleIndex) - yield return (T)cur; + T cur = list[pos]; + yield return cur; + if (pos < list.Count && ReferenceEquals(list[pos], cur)) + pos++; } } @@ -197,31 +261,39 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax #region Equals and GetHashCode implementation public override int GetHashCode() { - return node.GetHashCode() ^ role.GetHashCode(); + return parent.GetHashCode() ^ role.GetHashCode(); } - public override bool Equals(object obj) + public override bool Equals(object? obj) { - AstNodeCollection other = obj as AstNodeCollection; - if (other == null) - return false; - return this.node == other.node && this.role == other.role; + return obj is AstNodeCollection other && this.parent == other.parent && this.role == other.role; } #endregion internal bool DoMatch(AstNodeCollection other, Match match) { - return Pattern.DoMatchCollection(role, node.FirstChild, other.node.FirstChild, match); + return Pattern.DoMatchCollection(role, parent.FirstChild, other.parent.FirstChild, match); } public void InsertAfter(T existingItem, T newItem) { - node.InsertChildAfter(existingItem, newItem, role); + Insert(IndexOf(existingItem) + 1, newItem); } public void InsertBefore(T existingItem, T newItem) { - node.InsertChildBefore(existingItem, newItem, role); + int index = IndexOf(existingItem); + Insert(index < 0 ? list.Count : index, newItem); + } + + void Insert(int index, T newItem) + { + if (newItem == null || newItem.IsNull) + return; + ValidateNewChild(newItem); + list.Insert(index, newItem); + newItem.SetParentAndRole(parent, role); + parent.InvalidateChildIndices(); } /// @@ -229,17 +301,8 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax /// public void AcceptVisitor(IAstVisitor visitor) { - uint roleIndex = role.Index; - AstNode next; - for (AstNode cur = node.FirstChild; cur != null; cur = next) - { - Debug.Assert(cur.Parent == node); - // Remember next before yielding cur. - // This allows removing/replacing nodes while iterating through the list. - next = cur.NextSibling; - if (cur.RoleIndex == roleIndex) - cur.AcceptVisitor(visitor); - } + foreach (T item in this) + item.AcceptVisitor(visitor); } } } diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/Accessor.cs b/ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/Accessor.cs index 5b68e0715..30c49b336 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/Accessor.cs +++ b/ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/Accessor.cs @@ -56,6 +56,20 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax public AccessorKind Kind { get; set; } + // An accessor is printed as its keyword (get/set/init/add/remove), never an identifier, so it + // carries no name. The contract members are overridden to no-ops: shared decompiler code sets a + // name on every method-like entity (e.g. explicit interface implementations), which is irrelevant + // here and must not throw. + public override string Name { + get { return string.Empty; } + set { } + } + + public override Identifier NameToken { + get { return Identifier.Null; } + set { } + } + [Slot("AttributeRole")] public override partial AstNodeCollection Attributes { get; } diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/EventDeclaration.cs b/ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/EventDeclaration.cs index 09e37d1d1..39208fee3 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/EventDeclaration.cs +++ b/ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/EventDeclaration.cs @@ -85,6 +85,12 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax get { return SymbolKind.Event; } } + [Slot("AttributeRole")] + public override partial AstNodeCollection Attributes { get; } + + [Slot("Roles.Type")] + public override partial AstType ReturnType { get; set; } + /// /// Gets/Sets the type reference of the interface that is explicitly implemented. /// Null node if this member is not an explicit interface implementation. @@ -92,6 +98,9 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax [Slot("PrivateImplementationTypeRole")] public partial AstType PrivateImplementationType { get; set; } + [Slot("Roles.Identifier")] + public override partial Identifier NameToken { get; set; } + [Slot("AddAccessorRole")] public partial Accessor AddAccessor { get; set; } diff --git a/ICSharpCode.Decompiler/CSharp/Transforms/IntroduceExtensionMethods.cs b/ICSharpCode.Decompiler/CSharp/Transforms/IntroduceExtensionMethods.cs index 460413473..460d85996 100644 --- a/ICSharpCode.Decompiler/CSharp/Transforms/IntroduceExtensionMethods.cs +++ b/ICSharpCode.Decompiler/CSharp/Transforms/IntroduceExtensionMethods.cs @@ -116,10 +116,9 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms } if (invocationExpression.Target is IdentifierExpression identifierExpression) { + identifierExpression.Detach(); memberRefExpr = new MemberReferenceExpression(firstArgument.Detach(), method.Name, identifierExpression.TypeArguments.Detach()); - // Replace in place so the target keeps its slot position; assigning after a - // Detach would re-append it behind the arguments in document order. - identifierExpression.ReplaceWith(memberRefExpr); + invocationExpression.Target = memberRefExpr; } else {