Record the whole pipeline or none of it

Recording only gated the IL half, so a run with it off still numbered the C# AST
transforms into the shared stepper. That gave one pipeline two numbering scales,
and a step index is only meaningful against the scale it was recorded on: a tree
captured under one and replayed under the other selects a different step. It
also let the crashed-member attribution fire on a counter that had never moved -
a limit of zero matched at every throwing transform and rendered an unrelated
member's ILAst.

The flag now gates both halves, so steps exist exactly when recording is on, and
it lives on the pane instance rather than a static the background decompile read
across threads.

Assisted-by: Claude:claude-opus-5[1m]:Claude Code

ICSharpCode.Decompiler.Tests/DebugStepRecordingTests.cs

@@ -146,6 +146,45 @@ namespace ICSharpCode.Decompiler.Tests
 			}
 		}
 
+		/// <summary>
+		/// A step index only means anything against the run that produced it, so there must not be a
+		/// second numbering to confuse it with: with recording off the pipeline counts nothing at all,
+		/// neither the IL transforms nor the C# AST transforms that used to be recorded regardless.
+		/// </summary>
+		[Test]
+		public void NothingIsRecordedWhileRecordingIsOff()
+		{
+			var decompiler = StepperTesting.CreateDecompiler();
+
+			decompiler.DecompileTypeAsString(SampleType);
+
+			using (Assert.EnterMultipleScope())
+			{
+				Assert.That(decompiler.RecordSteps, Is.False, "recording is off unless a caller asks for it");
+				Assert.That(decompiler.Stepper.Steps, Is.Empty, "no step may be recorded while recording is off");
+				Assert.That(decompiler.Stepper.CurrentStep, Is.Zero, "the step counter must not advance either");
+			}
+		}
+
+		/// <summary>
+		/// The crashed-member attribution compares the step counter against the limit, which is only
+		/// meaningful while the counter is actually counting. With recording off it sits at zero, so a
+		/// limit of zero would otherwise match at every throwing transform and hand the pane an
+		/// unrelated member's ILAst.
+		/// </summary>
+		[Test]
+		public void ACrashWhileRecordingIsOffAttributesNoHaltedFunction()
+		{
+			var decompiler = StepperTesting.CreateDecompiler();
+			decompiler.ILTransforms.Add(new StepperTesting.ThrowingILTransform("CleanUpFileName"));
+			decompiler.Stepper.StepLimit = 0;
+
+			decompiler.DecompileTypeAsString(SampleType);
+
+			Assert.That(decompiler.StepLimitHaltedFunction, Is.Null,
+				"a step limit cannot be reached by a run that records no steps");
+		}
+
 		/// <summary>
 		/// Closing the groups an unwind abandoned must stop at the depth the member started from: a
 		/// group opened around the decompile still belongs to whoever opened it, and swallowing it
@@ -270,7 +309,7 @@ namespace ICSharpCode.Decompiler.Tests
 		static CSharpDecompiler CreateRecordingDecompiler()
 		{
 			var decompiler = StepperTesting.CreateDecompiler();
-			decompiler.RecordILTransformSteps = true;
+			decompiler.RecordSteps = true;
 			return decompiler;
 		}
 

ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs

@@ -236,20 +236,20 @@ namespace ICSharpCode.Decompiler.CSharp
 		public Stepper Stepper { get; set; } = new Stepper();
 
 		/// <summary>
-		/// Gets or sets whether the IL transform phase records its steps into <see cref="Stepper"/>,
-		/// so that one step tree spans the whole pipeline: IL transforms, the ILAst-to-C# conversion,
+		/// Gets or sets whether the pipeline records its steps into <see cref="Stepper"/>, so that one
+		/// step tree spans all of it: the IL transforms of every member, the ILAst-to-C# conversion,
 		/// and the C# AST transforms.
-		/// Off by default. The recording itself already happens in debug builds - into a per-member
-		/// stepper that is thrown away - so what this turns on is <i>retention</i>: the step history of
-		/// every member of the decompiled type stays alive, and each step pins the ILAst it captured,
-		/// including the instructions its transform removed. That is affordable for the single type a
-		/// step view displays and not for a whole-module decompile, so only callers that show the steps
-		/// opt in.
-		/// Replaying a step by index requires the same value on the full run and on the step-limited
-		/// re-run, since the recording decides how the steps are numbered.
+		/// Off by default, and off means the whole pipeline records nothing: every phase keeps the
+		/// stepper its context creates for itself, which nothing reads. Turning it on is what makes the
+		/// steps <i>retained</i>, and each retained step pins the ILAst it captured, including the
+		/// instructions its transform removed. That is affordable for the single type a step view
+		/// displays and not for a whole-module decompile, so only callers that show the steps opt in.
+		/// A step index is only meaningful against a run with the same value: numbering the whole
+		/// pipeline and numbering nothing are not the same scale, so a full run and the step-limited
+		/// re-run that replays one of its indices have to agree on it.
 		/// Has no effect unless <see cref="Stepper.SteppingAvailable"/>.
 		/// </summary>
-		public bool RecordILTransformSteps { get; set; }
+		public bool RecordSteps { get; set; }
 
 		/// <summary>
 		/// The <see cref="ILFunction"/> whose IL transforms were halted by <see cref="Stepper.StepLimit"/>,
@@ -893,9 +893,13 @@ namespace ICSharpCode.Decompiler.CSharp
 			if (StepLimitHaltedFunction != null)
 				return;
 			var typeSystemAstBuilder = CreateAstBuilder(decompileRun.Settings);
-			var context = new TransformContext(typeSystem, decompileRun, decompilationContext, typeSystemAstBuilder) {
-				Stepper = Stepper
-			};
+			var context = new TransformContext(typeSystem, decompileRun, decompilationContext, typeSystemAstBuilder);
+			// Off means off for the whole pipeline: leaving the AST half recording would give the same
+			// pipeline two numbering bases, and an index recorded under one of them means a different
+			// step under the other. Without this the context keeps a stepper of its own, which nothing
+			// reads.
+			if (RecordSteps)
+				context.Stepper = Stepper;
 			// The tree handed to the pipeline must already be well-formed; check it once up front so a
 			// malformed builder output is caught here rather than blamed on the first transform (DEBUG only).
 			rootNode.CheckInvariant();
@@ -2397,7 +2401,7 @@ namespace ICSharpCode.Decompiler.CSharp
 					CancellationToken = CancellationToken,
 					DecompileRun = decompileRun
 				};
-				if (RecordILTransformSteps)
+				if (RecordSteps)
 					context.Stepper = Stepper;
 				context.StepStartGroup(method.FullName);
 				// From here on a halt belongs to this member: it is standing on one of this member's own
@@ -2474,7 +2478,7 @@ namespace ICSharpCode.Decompiler.CSharp
 				// 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)
+				if (RecordSteps && function != null && Stepper.CurrentStep == Stepper.StepLimit)
 					StepLimitHaltedFunction = function;
 				entityDecl.GetChild(Slots.Body)?.Remove();
 				if (settings.DecompileMemberBodies)

ILSpy.Tests/Views/DebugStepsTests.cs

@@ -73,7 +73,7 @@ public class DebugStepsTests
 		// The default writing options match the WPF baseline: field sugar and
 		// logic-operation sugar on; IL ranges and child-index-in-block off. The four
 		// CheckBoxes in DebugSteps.axaml bind two-way against these defaults.
-		var options = DebugStepsPaneModel.WritingOptions;
+		var options = AppComposition.Current.GetExport<DebugStepsPaneModel>().WritingOptions;
 		options.UseFieldSugar.Should().BeTrue();
 		options.UseLogicOperationSugar.Should().BeTrue();
 		options.ShowILRanges.Should().BeFalse();
@@ -94,6 +94,9 @@ public class DebugStepsTests
 		// view-realisation timing — which is the whole point of the fix that moved state
 		// from the View into the VM. If `Steps` is populated, any view that binds to it (now
 		// or later) will render the correct content.
+		//
+		// Recording is what produces steps at all, and no view is realised to switch it on here.
+		AppComposition.Current.GetExport<DebugStepsPaneModel>().SetRecordingEnabled(true);
 
 		var window = AppComposition.Current.GetExport<MainWindow>();
 		window.Show();
@@ -125,6 +128,10 @@ public class DebugStepsTests
 	[AvaloniaTest]
 	public async Task CSharp_DebugSteps_Are_Grouped_By_Ast_Transform()
 	{
+		// Steps exist only while the pane asks for them, and nothing realises the pane's view here,
+		// so this test asks the same way the view does.
+		AppComposition.Current.GetExport<DebugStepsPaneModel>().SetRecordingEnabled(true);
+
 		var window = AppComposition.Current.GetExport<MainWindow>();
 		window.Show();
 		var vm = (MainWindowViewModel)window.DataContext!;
@@ -152,7 +159,7 @@ public class DebugStepsTests
 			.Select(transform => transform.GetType().Name)
 			.ToArray();
 
-		// The AST transforms close the tree; with the pane closed, nothing precedes them.
+		// The AST transforms close the tree, after the member groups of the IL half.
 		debugStepsVm.Steps!
 			.Select(step => StripStepNumber(step.Description))
 			.Should().EndWith(astTransformNames,
@@ -189,9 +196,10 @@ public class DebugStepsTests
 	[AvaloniaTest]
 	public async Task IL_Steps_Are_Recorded_Only_While_The_Pane_Asks_For_Them()
 	{
-		// Every retained IL step pins the ILAst it captured, which for one type runs to tens of
-		// thousands of nodes. A closed pane displays none of them, so the decompile must not collect
-		// them either - only the C# AST steps, which are few and cheap.
+		// Every retained step pins the ILAst it captured, which for one type runs to tens of thousands
+		// of nodes. A closed pane displays none of them, so the decompile must not collect any - not
+		// the IL transforms, and not the C# AST transforms either: a run that records half a pipeline
+		// would number its steps on a scale no displayed tree was recorded on.
 		var debugStepsVm = AppComposition.Current.GetExport<DebugStepsPaneModel>();
 		debugStepsVm.SetRecordingEnabled(false);
 
@@ -218,6 +226,7 @@ public class DebugStepsTests
 		AllDescriptions(csharp.Stepper.Steps).Should().NotContain(
 			description => ilTransformNames.Contains(StripStepNumber(description)),
 			"a closed pane leaves the IL transforms unrecorded");
+		csharp.Stepper.Steps.Should().BeEmpty("a closed pane leaves the whole pipeline unrecorded");
 
 		static System.Collections.Generic.IEnumerable<string> AllDescriptions(
 			System.Collections.Generic.IEnumerable<Stepper.Node> nodes)
@@ -477,9 +486,9 @@ public class DebugStepsTests
 	[AvaloniaTest]
 	public Task Writing_Option_CheckBoxes_Are_Bound_To_The_ILAst_Writing_Options()
 	{
-		// The checkboxes drive how a halted IL step's ILAst is rendered, through options that live in a
-		// static singleton: what the compiler cannot check is that the two-way path reaches that
-		// singleton rather than a copy, and a dead binding here is a dead feature.
+		// The checkboxes drive how a halted IL step's ILAst is rendered, through the options the pane
+		// owns: what the compiler cannot check is that the two-way path reaches the pane's own instance
+		// rather than a copy, and a dead binding here is a dead feature.
 		var vm = new DebugStepsPaneModel { IsAvailable = true };
 		var window = new Window { Width = 500, Height = 300, Content = new DebugSteps { DataContext = vm } };
 		window.Show();
@@ -488,18 +497,16 @@ public class DebugStepsTests
 		{
 			var fieldSugar = window.GetVisualDescendants().OfType<CheckBox>()
 				.Single(box => (box.Content as string) == "Field sugar");
-			fieldSugar.IsChecked.Should().Be(DebugStepsPaneModel.WritingOptions.UseFieldSugar,
+			fieldSugar.IsChecked.Should().Be(vm.WritingOptions.UseFieldSugar,
 				"the checkbox must show the current writing option");
 
 			fieldSugar.IsChecked = false;
 			Dispatcher.UIThread.RunJobs();
-			DebugStepsPaneModel.WritingOptions.UseFieldSugar.Should().BeFalse(
+			vm.WritingOptions.UseFieldSugar.Should().BeFalse(
 				"toggling the checkbox must reach the options the ILAst dump is written with");
 		}
 		finally
 		{
-			// Shared static state: leave it as the rest of the suite expects to find it.
-			DebugStepsPaneModel.WritingOptions.UseFieldSugar = true;
 			window.Close();
 		}
 		return Task.CompletedTask;

ILSpy/Languages/CSharpLanguage.DebugSteps.cs

@@ -23,6 +23,7 @@ using System;
 using ICSharpCode.Decompiler;
 using ICSharpCode.Decompiler.CSharp;
 using ICSharpCode.Decompiler.DebugSteps;
+using ICSharpCode.Decompiler.IL;
 
 using ICSharpCode.ILSpy.AppEnv;
 using ICSharpCode.ILSpy.Docking;
@@ -65,17 +66,24 @@ namespace ICSharpCode.ILSpy.Languages
 				avaloniaOutput.SyntaxExtensionOverride = ".il";
 			}
 			output.WriteLine();
-			function.WriteTo(output, DebugStepsPaneModel.WritingOptions);
+			function.WriteTo(output, DebugStepsPane?.WritingOptions ?? new ILAstWritingOptions());
 			handled = true;
 		}
 
 		/// <summary>
-		/// Points the decompiler's IL phase at the shared stepper, but only while the Debug Steps pane
-		/// is there to display what it records - see <see cref="DebugStepsPaneModel.IsRecording"/>.
+		/// The Debug Steps pane, or null before composition is up (design-time previews) and in the
+		/// window between startup and the pane being created. It is a [Shared] export, so a decompile
+		/// running on a background task resolves the same instance the view is bound to.
+		/// </summary>
+		static DebugStepsPaneModel? DebugStepsPane => AppComposition.TryGetExport<DebugStepsPaneModel>();
+
+		/// <summary>
+		/// Points the whole pipeline at the shared stepper, but only while the Debug Steps pane is
+		/// there to display what it records - see <see cref="DebugStepsPaneModel.IsRecording"/>.
 		/// </summary>
 		static partial void ConfigureStepRecording(CSharpDecompiler decompiler)
 		{
-			decompiler.RecordILTransformSteps = DebugStepsPaneModel.IsRecording;
+			decompiler.RecordSteps = DebugStepsPane?.IsRecording ?? false;
 		}
 
 		/// <summary>

ILSpy/ViewModels/DebugStepsPaneModel.cs

@@ -61,10 +61,10 @@ namespace ICSharpCode.ILSpy.ViewModels
 		/// <see cref="Stepper"/>. Every retained step pins the ILAst it captured, so recording a whole
 		/// type costs tens of thousands of nodes (System.Linq.Enumerable: 85k, ~35 MB) and a third
 		/// again as much decompilation time - worth it while the pane is on screen to show them,
-		/// wasted while it is closed. Static because the language is MEF-shared and decompiles on
-		/// background tasks that have no view-model reference.
+		/// wasted while it is closed. Reached from the C# language through composition: the pane is a
+		/// [Shared] export, so the background decompile resolves this very instance.
 		/// </summary>
-		public static bool IsRecording { get; private set; }
+		public bool IsRecording { get; private set; }
 
 		readonly LanguageService? languageService;
 
@@ -74,10 +74,10 @@ namespace ICSharpCode.ILSpy.ViewModels
 		/// <summary>
 		/// App-wide ILAst writing options shared between the C# language (which reads them while
 		/// emitting the ILAst a halted IL step stopped in) and the DebugSteps view (whose four
-		/// checkboxes toggle their values). Static singleton state because the language is
-		/// MEF-shared and decompiles on background tasks that have no view-model reference.
+		/// checkboxes toggle their values). Reached from the C# language through composition, the same
+		/// way <see cref="IsRecording"/> is.
 		/// </summary>
-		public static ILAstWritingOptions WritingOptions { get; } = new() {
+		public ILAstWritingOptions WritingOptions { get; } = new() {
 			UseFieldSugar = true,
 			UseLogicOperationSugar = true,
 		};