Walk the whole decompiler pipeline in the Debug Steps pane by siegfriedpammer · Pull Request #4029 · icsharpcode/ILSpy

siegfriedpammer · 2026-08-18T20:23:41Z

The pane used to split the pipeline across two languages: the ILAst language stepped the IL
transforms, the C# language stepped the AST transforms, and nothing showed the seam between
them. A step index therefore meant a different thing depending on which language happened to be
selected.

Recording both halves into one Stepper makes an index replayable across the whole pipeline. A
limit that lands in the IL phase has no C# to print, so the halted function is rendered as ILAst
instead.

What is left of the ILAst language is its typed-IL dump, which runs no transforms at all. That
stays, as TypedILLanguage. IDebugStepProvider was down to a single implementation and is
removed.

Which function a halt is attributed to

A member group's EndStep is the next member's first step, so stopping after a member's last IL
step lands on the next member's StepStartGroup. The halt is therefore attributed to the last
member whose IL phase completed, preferring the function that actually holds the halted step's
instruction - the outermost ILFunction ancestor of LimitReachedStep.Position. That also
covers functions that are not yet attached to their parent when the limit hits, such as
ProxyCallReplacer's proxy function.

A transform that throws never reaches the limit, so the crashing member used to fall out of the
recording entirely. It is now recorded when the crash site is already at the step limit, which
brings back the retired ILAst language's "ILAst after the crash" view.

Cost

Retention is opt-in, and now gated on the pane being open (DebugStepsPaneModel.IsRecording,
flipped by the view's logical-tree attach/detach) rather than on every Debug-build decompile.
Every kept step pins the ILAst it captured, which is affordable for the single type the pane
shows but not for a whole-module decompile. Measured on System.Linq.Enumerable with recording
on vs off: 84,712 step nodes vs 2,776, 67 MB vs 33 MB live, 8.6 s vs 5.0 s.

Verification

Rebased onto master (76ec7588e). ILSpy.Desktop.slnf builds clean, and both suites pass on
Linux:

ILSpy.Tests - 1237 total, 0 failed, 3 skipped
ICSharpCode.Decompiler.Tests - 3499 total, 0 failed, 45 skipped

26 of those cover this change: 17 pane tests in ILSpy.Tests/Views/DebugStepsTests.cs and 9
recording tests in ICSharpCode.Decompiler.Tests/DebugStepRecordingTests.cs.

Written by an AI agent (Claude) on Siegfried's behalf.

christophwille

Review summary (recall-biased pass, findings verified individually).

The core idea is sound: one Stepper across the IL and AST halves keeps step indices replayable, and the halted-IL rendering path (StepLimitHaltedFunction / TryWriteILAst) is wired consistently. Step numbering is identical between the full run and the step-limited replay as far as I can trace. Findings, most severe first (inline comments on the respective lines):

Bug - Show state after on a non-last member's Convert ILAst to C# step or member group halts at the next member's StepStartGroup(method.FullName) and renders that member's raw, untransformed IL with no highlight (CSharpDecompiler.cs:2396).
Debug-build regression - RecordILTransformSteps = true on every UI C# decompile retains every IL step of every member (each Node pins ILAst incl. removed instructions) on the shared CSharpLanguage.stepper, and StepNodeViewModel.Wrap eagerly builds a VM per node on the UI thread even with the pane hidden (CSharpLanguage.cs:333).
Lost capability - the deleted ILAst language's ILAst after the crash view is unreachable via the pane: replaying the crashed group's state after re-throws before any step reaches the limit (CSharpDecompiler.cs:2435).
Test - AFailingTransformDoesNotNestLaterMembersUnderIt asserts on DecompileProject, which is declared before CleanUpFileName in metadata order, so the sibling assertion passes with or without EndOpenGroups (DebugStepRecordingTests.cs:145).
Pre-existing limitation now advertised as replayable - a step limit hit in a detached helper function (ProxyCallReplacer's proxyFunction, nested functions before attachment) attributes the halt to the top-level function, whose ILAst does not contain the halted instruction (CSharpDecompiler.cs:2424).
Reuse - the per-transform loop duplicates ILFunction.RunTransforms naming/invariant logic (CSharpDecompiler.cs:2368).
Simplification - x:CompileBindings="False" + Options forwarder + runtime binding test could be {Binding Options.UseFieldSugar} etc. with compiled bindings (DebugSteps.axaml:22).
Test duplication - StripStepNumber twice in DebugStepsTests.cs; CreateDecompiler/ThrowingILTransform copied from DecompilationErrorRecoveryTests.cs.
Post-merge stale docs (not in this diff) - master's CLAUDE.md:94 (like the UI's ILAst language), ICSharpCode.ILSpyCmd/ILAstDumper.cs:40/47 and IlspyCmdProgram.cs:122 still point at the retired ILAst language; worth rewording on rebase.
Conventions - comments describing the change relative to the previous version (which the C# path used to discard, used to live on a separate ILAst language), and behaviour (en-US per CLAUDE.md) in DebugStepRecordingTests.cs.

Minor, not commented inline: Should().Equal(astTransformNames) weakened to EndWith, so no test pins the exact top-level tree shape; the ref bool handled partial in WriteCode could be an inline StepLimitHaltedFunction is { } halted branch; EndOpenGroups() popping to zero relies on nothing ever opening a group around DecompileBody (a depth-restoring close or a finally would be more robust).

Review by an AI agent (Claude) on Christoph's behalf.

The pane used to split the pipeline across two languages: the ILAst language stepped the IL transforms, the C# language stepped the AST transforms, and nothing showed the seam between them, so a step index meant a different thing depending on which language happened to be selected. Recording both halves into one Stepper makes an index replayable across the whole pipeline; a limit that lands in the IL phase has no C# to print, so the halted function is rendered as ILAst instead. Which function that is takes some care, because a member group's EndStep is the next member's first step: a halt standing on a member's opening step belongs to the member that just finished, a transform that throws where the limit was aimed has to hand over the ILAst it half-transformed (what the ILAst language showed as "ILAst after the crash"), and a step recorded on a helper function the pipeline has not attached yet belongs to that function's own tree. Retention stays opt-in twice over: the decompiler records IL steps only when asked to, and the pane asks only while its view is on screen. Every kept step pins the ILAst it captured, which for one type runs to tens of thousands of nodes, so a closed pane would be paying for a tree nobody displays. What is left of the ILAst language is its typed-IL dump, which runs no transforms at all. That stays, as TypedILLanguage. IDebugStepProvider was down to a single implementation and is removed. Assisted-by: Claude:claude-opus-5[1m]:Claude Code

The ilspycmd dump and the contributor notes described themselves by reference to the UI's ILAst language; the pane is what walks that pipeline now. Assisted-by: Claude:claude-opus-5[1m]:Claude Code

christophwille

Review summary

High-effort pass: line-by-line, removed-behavior and cross-file correctness angles plus reuse, efficiency and altitude angles, each candidate verified independently against the base tree. The simplification and CLAUDE.md-conventions angles did not complete in time; the one conventions item found on the way (TypedILLanguage.cs header) is noted inline.

The core decompiler change verified clean: every exit path of DecompileBody closes its groups, HaltedStepFunction cannot resolve to an unrelated member's function (every separately read function is either attached before a counted step or transformed on a private stepper), the reset in CreateDecompileRun is reached from every public entry point, and startup with a persisted-open pane is safe (DecompileAsync no-ops with no nodes).

What did not hold up is the UI-side wiring of the recording flag.

Correctness (confirmed)

CSharpDecompiler.cs:2477 - crash-at-limit check reads the shared Stepper even when IL steps went to the throwaway stepper; StepLimit == 0 plus any throwing IL transform renders an unrelated member's ILAst.
CSharpLanguage.DebugSteps.cs:78 - replay numbering is decided by the static IsRecording at CreateDecompiler time; the AST-only tree stays clickable after pane-open flips the flag, so a click in that window replays an AST index under IL-inclusive numbering.
CSharpLanguage.DebugSteps.cs:65 - multi-node tabs: each node halts at StepLimit in its own numbering; the first ILAst dump sets .il for the whole document and later nodes' C# is highlighted as IL.
DebugStepsPaneModel.cs:411 - closing the pane does not stop the in-flight recording run; OnCSharpDecompiled re-pins the full tree after ReleaseSteps(); and ReleaseSteps goes through activeLanguage, null whenever the language is not C#.
DebugStepsPaneModel.cs:406 - enable path re-decompiles unconditionally, even for non-C# languages.
StepNodeViewModel.cs:51 - lazy Children is fully materialised by ApplyFilter/SnapshotExpansion on the first filter keystroke.

Plausible
7. DebugSteps.axaml.cs:90 - keying recording to logical-tree attach/detach means any dock relayout (sibling tab switch, float, re-dock) drops the tree and re-decompiles.
8. CSharpDecompiler.cs:2341 - after an IL-phase halt every remaining member still gets its full AST declaration built and then discarded (bounded, but it is the hot path of step selection).

Cleanup
9. TreeTraversal.PreOrder replaces the three hand-rolled node walkers in the test files; StepperTesting.CreateDecompiler is the existing file-name CSharpDecompiler ctor.
10. TypedILLanguage.cs is a new file per git with an AlphaSierraPapa header; CLAUDE.md wants the contributor's name on new files.

Items 1, 2, 4, 5 and 7 share a root: the recording flag lives in a view-flipped static rather than travelling in DecompilationOptions next to StepLimit/IsDebug. Carrying it there (set by RestartDecompileWithStepLimit from the mode the displayed tree was recorded in) makes full run and replay agree by construction and removes the cross-thread static read; item 4 additionally needs the VM to ignore StepperUpdated while the pane is closed.

christophwille · 2026-08-26T07:38:42Z

+				// throws before any step can reach the limit, so without this the halt would be attributed
+				// to the next member and the half-transformed ILAst the crash left behind - the one thing
+				// worth looking at when debugging a throwing transform - would be unreachable.
+				if (function != null && Stepper.CurrentStep == Stepper.StepLimit)


Bug. Stepper here is the shared stepper, but when RecordILTransformSteps is false the IL phase recorded into the throwaway context.Stepper, so Stepper.CurrentStep is still 0 for the entire IL phase. With StepLimit == 0 ("show state before" on the first tree node, or lastSelectedStep replayed by OnWritingOptionsChanged after the pane's flag flipped) any member whose IL transform throws satisfies 0 == 0, is attributed as the halted function, RunTransforms(AstNode) bails, and the document becomes that unrelated member's half-transformed ILAst.

The same equality also holds when a member completes all its steps and then crashes in a transform that recorded nothing while the limit is aimed at the next group's opener: the crash is reported as the halt of the wrong index.

Minimal guard: RecordILTransformSteps && function != null && .... Deeper: let the Stepper own the predicate (set LimitReachedStep = LastStep when step == StepLimit on crash) so both catch clauses use the same HaltedStepFunction() line and CurrentStep need not be public.

christophwille · 2026-08-26T07:38:42Z

+		/// </summary>
+		static partial void ConfigureStepRecording(CSharpDecompiler decompiler)
+		{
+			decompiler.RecordILTransformSteps = DebugStepsPaneModel.IsRecording;


Bug (numbering race). IsRecording is sampled here inside Task.Run at CreateDecompiler time, not when the replay was requested. OnCSharpDecompiled publishes the stepper on any full run regardless of recording mode, so an AST-only tree is displayed and clickable while the pane is closed. Opening the pane flips IsRecording and requests a refresh, but nothing invalidates the old tree until StepperUpdated lands. A "show state before" click on AST step k in that window cancels the refresh (DecompileAsync -> activeCts.Cancel()) and replays k under IL-inclusive numbering: it halts on an unrelated IL step and renders ILAst. The same window is entered via the "Show Steps" button. The mirror case (pane closed in the window between click and task start) replays an IL+AST index against AST-only numbering.

The RecordILTransformSteps doc names the invariant ("requires the same value on the full run and on the step-limited re-run"); the code does not enforce it. Carrying the flag in DecompilationOptions next to StepLimit/IsDebug, set in RestartDecompileWithStepLimit from the mode the displayed tree was recorded in, makes it hold by construction.

christophwille · 2026-08-26T07:38:42Z

+			if (output is AvaloniaEditTextOutput avaloniaOutput)
+			{
+				// The dump is IL, not C#; without this the editor would highlight it as C#.
+				avaloniaOutput.SyntaxExtensionOverride = ".il";


Bug (multi-node tabs). DecompileAsync writes all CurrentNodes into one AvaloniaEditTextOutput with one options.StepLimit, and each node gets its own CSharpDecompiler/Stepper. Select method A (long IL phase) plus a field or short method B: A halts in its IL phase and sets SyntaxExtensionOverride = ".il" for the whole document; B halts in its AST phase (or never) and appends C# that is then highlighted as IL, with DebugStepHighlighter resolving against whichever node's ranges matched. The doc comment only considers members within one node. Simplest fix: restrict step replay to single-node tabs.

christophwille · 2026-08-26T07:38:42Z

+			else
+			{
+				SetStepsSource(null);
+				activeLanguage?.ReleaseSteps();


Bug (retention after close). Two issues:

activeLanguage is null whenever the current language is not C# (DetachFromLanguage). Open the pane on C# with a large type, switch to IL, close the pane: ReleaseSteps() never runs and the MEF-shared CSharpLanguage keeps the full IL tree until the next C# full run. DetachFromLanguage does not release either. Resolve CSharpLanguage via languageService.Languages.OfType<CSharpLanguage>() (as the tests do).

Closing the pane does not cancel an in-flight run created with RecordILTransformSteps = true. When it finishes, OnCSharpDecompiled does stepper = decompiler.Stepper and raises StepperUpdated; the VM's OnStepperUpdated (still attached, language unchanged) re-pins the ~35 MB tree into Steps of a closed pane. OnStepperUpdated should drop updates while !IsRecording.

christophwille · 2026-08-26T07:38:42Z

+			IsRecording = enabled;
+			if (enabled)
+			{
+				RequestRedecompile(int.MaxValue, isDebug: false);


RequestRedecompile runs unconditionally, even when the current language is not C# (the only language that reads IsRecording) or the active tab shows a step-limited replay. Opening the pane on the IL language re-decompiles the tab for nothing and discards the current view. Guard on activeLanguage != null.

christophwille · 2026-08-26T07:38:42Z

+		/// thousands of steps, so materialising the whole tree up front would put that many view-models
+		/// on the UI thread for the handful of rows an expanded path actually shows.
+		/// </summary>
+		public IReadOnlyList<StepNodeViewModel> Children => children ??= Wrap(Step.Children, this);


The laziness is defeated for the filter case: ApplyFilterToNode, SnapshotExpansion and RestoreExpansion in DebugStepsPaneModel recurse through Children for every root, so the first character typed into "Filter steps" (and every SetStepsSource while filtering) materialises all wrappers synchronously on the UI thread (85k for Enumerable), then re-walks them per keystroke. Either filter over the raw Stepper.Node tree and only touch children where it is already non-null, or drop the doc comment's claim.

christophwille · 2026-08-26T07:38:42Z

 				model.SelectionRevealRequested += OnSelectionRevealRequested;
+				// The view is in the tree exactly while the pane is open, which is the only time the
+				// recorded IL steps are worth their memory.
+				model.SetRecordingEnabled(true);


Recording is keyed to logical-tree attach/detach, not pane open/close. The Debug Steps view shares the bottom ToolDock with Search/Analyzer as tabs; switching to a sibling tab, floating, re-docking or a layout reset fires detach (tree nulled, ReleaseSteps) then attach (RequestRedecompile(int.MaxValue)): selection and expansion state are lost and the whole type re-decompiles with recording on. The base VM comment still says "it doesn't matter when the matching View materialises". Consider keying on the dockable's visibility/close instead, and skipping the redecompile when the language's current stepper already contains IL groups.

christophwille · 2026-08-26T07:38:42Z

 		{
+			// An earlier member's IL phase already hit the step limit, so the pipeline is stopped for
+			// good: reading IL for every remaining member only to throw on its first step is wasted work.
+			if (StepLimitHaltedFunction != null)


Efficiency (bounded): this skips ILReader for later members, but DoDecompile(IMethod/IProperty/IEvent) and DecompileTypeDefinition still build every remaining member's TypeSystemAstBuilder declaration, attributes and accessors, and Decompile() assembles a SyntaxTree that TryWriteILAst discards. Every step selection in the pane pays this. A break in the member loops when StepLimitHaltedFunction != null cuts it. (Verified nothing downstream dereferences the missing body/annotation; body-less members are an already-supported state.)

christophwille · 2026-08-26T07:38:42Z

+			return decompiler.Stepper.Steps.Single(n => n.Description.EndsWith("." + methodName, StringComparison.Ordinal));
+		}
+
+		static IEnumerable<Stepper.Node> AllNodes(IEnumerable<Stepper.Node> nodes)


Reuse: AllNodes here, Descriptions below it, and AllDescriptions in ILSpy.Tests/Views/DebugStepsTests.cs are three copies of the same pre-order walk. ICSharpCode.Decompiler.Util.TreeTraversal.PreOrder(decompiler.Stepper.Steps, n => n.Children) (already used by ILSpy/Commands/ExtractPackageEntryContextMenuEntry.cs) replaces all three; .Select(n => n.Description) covers the description variants. Likewise StepperTesting.CreateDecompiler is new CSharpDecompiler("ICSharpCode.Decompiler.dll", new UniversalAssemblyResolver(null, false, null), new DecompilerSettings()) via the existing file-name ctor (only difference: LoadInMemory/prefetch, harmless here).

christophwille · 2026-08-26T07:38:42Z

@@ -0,0 +1,72 @@
+// Copyright (c) 2026 AlphaSierraPapa for the SharpDevelop Team


Conventions (CLAUDE.md, "File headers"): this is a new file per git (new file mode), and the rule says line 1 carries "the name of the human contributing the change ... never AlphaSierraPapa". The content was moved out of ILAstLanguage.cs, which carried this header, so keeping the original holder is defensible, but the PR's other new file (StepperTesting.cs) uses Siegfried Pammer; pick one convention for the PR.

christophwille

Follow-up: the simplification and CLAUDE.md-conventions angles that were missing from the review above are now complete. Five additional low-severity items inline (no new correctness bugs): the halted-function fallback chain is mostly derivable from Stepper state, the ref bool handled partial can be a partial bool, EndOpenGroups's unused default, duplicated boot sequence in DebugStepsTests, and two conventions nits (Assert.Ignore bypass under Release, one en-GB spelling).

christophwille · 2026-08-26T07:42:22Z

+			// Declared out here so the StepLimitReachedException handler below can report the function
+			// its transforms were halted in, and tell a halt inside this member from one on its boundary.
+			ILFunction? function = null;
+			bool haltBelongsToThisMember = false;


Simplification: haltBelongsToThisMember + lastCompletedFunction (reset in CreateDecompileRun) exist to reconstruct what the Stepper already knows. When the limit hits a member's opener, Stepper.LastStep is the previous member's last recorded step, so FunctionOf(Stepper.LimitReachedStep) ?? FunctionOf(Stepper.LastStep) ?? function covers the boundary case, the detached-function case and the first-member case, and drops the flag, the field and its reset. One caveat to keep in mind: with DecompileMemberBodies == false there is no seam step, so LastStep is a transform group opener whose Position is null; the fallback then lands on function (the untouched next member) instead of the previous one. If that path matters, give group openers in ILFunction.RunTransforms a near: this so their Position resolves.

christophwille · 2026-08-26T07:42:22Z

-
-			syntaxTree.AcceptVisitor(new CSharpOutputVisitor(tokenWriter, settings.CSharpFormattingOptions));
-			bookmarkCollector?.Publish();
+			bool handled = false;


Simplification: the ref bool handled out-channel is only needed for partial void. Extended partial methods (C# 9+) may return a value when declared with an access modifier: private static partial bool TryWriteILAst(ITextOutput, DecompilationOptions, CSharpDecompiler); with a Release stub => false, then if (!TryWriteILAst(...)) { ...C# path... }. Or drop the partial entirely: StepLimitHaltedFunction is public and always null in Release, so if (decompiler.StepLimitHaltedFunction is { } fn) WriteILAst(...) else WriteCSharp(...) reads the same in both configurations and only WriteILAst needs #if DEBUG.

christophwille · 2026-08-26T07:42:22Z

+		/// Closing stops at <paramref name="targetDepth"/>: a group that was already open before the
+		/// unwinding code ran belongs to whoever opened it, not to the unwind.
+		/// </summary>
+		public void EndOpenGroups(int targetDepth = 0)


The targetDepth = 0 default has no caller (both sites pass groupDepth) and is the one value that closes groups the caller does not own - the case AFailingTransformLeavesAGroupOpenedAroundItAlone guards against. Make it required.

christophwille · 2026-08-26T07:42:22Z

+		}
+	}
+
+	static async Task CoverILTransformsAndReplay()


CoverILTransformsAndReplay is a single-use extraction so the caller can wrap it in try/finally; inlining the body into the test (with the same try/finally) is shorter. The boot sequence (GetExport, Show, WaitForAssembliesAsync, LanguageService, find the Enumerable node, WaitForDecompiledTextAsync) now appears four times in this file; one shared BootAndSelectEnumerableAsync() helper would cover all four.

christophwille · 2026-08-26T07:42:22Z

+		public void AFailingTransformDoesNotNestLaterMembersUnderIt()
+		{
+			if (!Stepper.SteppingAvailable)
+				Assert.Ignore("Transform stepping is compiled out without the STEP symbol, so there are no groups to check.");


Conventions (CLAUDE.md, "No silent returns in tests. When an expected component is missing, assert and fail"): Assert.Ignore when !Stepper.SteppingAvailable (here and at lines 158, 184, 203) makes a Release-configuration run report green with zero step-recording coverage. The repo does use Assert.Ignore for the missing ILSpy-tests submodule, so this may be an accepted exception; if so, a single [SetUp] guard (or Assume.That) beats repeating it in four tests. Also en-US: ILSpy.Tests/Views/DebugStepsTests.cs:237 has "realises" in a new comment ("realizes").

christophwille reviewed Aug 19, 2026

View reviewed changes

siegfriedpammer added 2 commits August 26, 2026 09:06

Stop pointing the ILAst docs at a language that is not there

973621d

The ilspycmd dump and the contributor notes described themselves by reference to the UI's ILAst language; the pane is what walks that pipeline now. Assisted-by: Claude:claude-opus-5[1m]:Claude Code

siegfriedpammer force-pushed the refactor/debug-steps-typed-il branch from 9656088 to 973621d Compare August 26, 2026 07:10

christophwille reviewed Aug 26, 2026

View reviewed changes

		@@ -0,0 +1,72 @@
		// Copyright (c) 2026 AlphaSierraPapa for the SharpDevelop Team

Uh oh!

Uh oh!

Conversation

siegfriedpammer commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which function a halt is attributed to

Cost

Verification

Uh oh!

christophwille left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

christophwille left a comment

Choose a reason for hiding this comment

Review summary

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

christophwille left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

siegfriedpammer commented Aug 18, 2026 •

edited

Loading