From 2bd41edfafd31719e17005ffc5bc417cad3caf22 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Wed, 1 Jul 2026 12:06:00 +0200 Subject: [PATCH] Bridge only the debug-step marker in NodeLookup NodeLookup.AddNode indexed every annotation of every rendered node by reference identity, but the debug-step highlighter only ever looks up the DebugStepMarker; the rest were dead keys, and a shared annotation (ResolveResult and friends, copied across nodes) would resolve to whichever node rendered last. Make DebugStepMarker public and bridge only it -- behaviour-preserving for resolution while dropping the per-annotation dictionary churn on every rendered node. Assisted-by: Claude:claude-opus-4-8:Claude Code --- .../CSharp/Transforms/TransformContext.cs | 12 ++++++++--- ILSpy.Tests/Views/DebugStepsTests.cs | 12 ++++++++++- ILSpy/TextView/NodeLookup.cs | 20 ++++++++++++------- 3 files changed, 33 insertions(+), 11 deletions(-) diff --git a/ICSharpCode.Decompiler/CSharp/Transforms/TransformContext.cs b/ICSharpCode.Decompiler/CSharp/Transforms/TransformContext.cs index 9adf66b96..5e6ee99c6 100644 --- a/ICSharpCode.Decompiler/CSharp/Transforms/TransformContext.cs +++ b/ICSharpCode.Decompiler/CSharp/Transforms/TransformContext.cs @@ -144,9 +144,15 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms else step.ModifiedNodeCandidates.Add(candidate); } + } - sealed class DebugStepMarker - { - } + /// + /// Annotation attached to a step's modified node so the debug-step highlighter can still locate + /// that node's rendered range after a later transform copies its annotations onto a replacement + /// (via CopyAnnotationsFrom). This is the only annotation NodeLookup bridges to a text + /// range; indexing every annotation would add dead keys and let shared ones collide by identity. + /// + public sealed class DebugStepMarker + { } } diff --git a/ILSpy.Tests/Views/DebugStepsTests.cs b/ILSpy.Tests/Views/DebugStepsTests.cs index 1690169e3..8e0dc076c 100644 --- a/ILSpy.Tests/Views/DebugStepsTests.cs +++ b/ILSpy.Tests/Views/DebugStepsTests.cs @@ -32,6 +32,7 @@ using AwesomeAssertions; using ICSharpCode.Decompiler.CSharp; using ICSharpCode.Decompiler.CSharp.Syntax; +using ICSharpCode.Decompiler.CSharp.Transforms; using ICSharpCode.ILSpy; using ICSharpCode.ILSpy.AppEnv; @@ -275,7 +276,7 @@ public class DebugStepsTests [AvaloniaTest] public Task NodeLookup_Resolves_Copied_Ast_Annotations() { - var marker = new object(); + var marker = new DebugStepMarker(); var original = new IdentifierExpression("old"); original.AddAnnotation(marker); var replacement = new IdentifierExpression("new").CopyAnnotationsFrom(original); @@ -287,6 +288,15 @@ public class DebugStepsTests "C# debug-step markers copied by AST replacements must still resolve to emitted text"); range.Start.Should().Be(12); range.Length.Should().Be(3); + + // A non-marker annotation is not bridged: only the debug-step marker is queried by the + // resolver, and indexing arbitrary shared annotations would collide by reference identity. + var otherAnnotation = new object(); + var otherNode = new IdentifierExpression("x"); + otherNode.AddAnnotation(otherAnnotation); + lookup.AddNode(otherNode, 30, 1); + lookup.TryGetRange(otherAnnotation, out _).Should().BeFalse( + "only DebugStepMarker annotations are bridged to a text range"); return Task.CompletedTask; } diff --git a/ILSpy/TextView/NodeLookup.cs b/ILSpy/TextView/NodeLookup.cs index b68f0f4ba..e2603bef4 100644 --- a/ILSpy/TextView/NodeLookup.cs +++ b/ILSpy/TextView/NodeLookup.cs @@ -19,6 +19,7 @@ using System.Collections.Generic; using ICSharpCode.Decompiler.CSharp.Syntax; +using ICSharpCode.Decompiler.CSharp.Transforms; namespace ICSharpCode.ILSpy.TextView { @@ -34,15 +35,20 @@ namespace ICSharpCode.ILSpy.TextView public void AddNode(object node, int start, int length) { - if (length > 0) + if (length <= 0) + return; + var range = new TextRange(start, length); + nodes[node] = range; + // Bridge only the debug-step marker: it is the sole annotation the resolver ever looks up + // (it rides CopyAnnotationsFrom so a replaced node's step still resolves to the emitted + // text). Indexing every annotation would add dead keys never queried, and would let a + // shared annotation resolve to whichever node rendered last. + if (node is IAnnotatable annotatable) { - nodes[node] = new TextRange(start, length); - if (node is IAnnotatable annotatable) + foreach (var annotation in annotatable.Annotations) { - foreach (var annotation in annotatable.Annotations) - { - nodes[annotation] = new TextRange(start, length); - } + if (annotation is DebugStepMarker) + nodes[annotation] = range; } } }