diff --git a/ICSharpCode.Decompiler.Tests/DebugStepRecordingTests.cs b/ICSharpCode.Decompiler.Tests/DebugStepRecordingTests.cs
index a4e873c64..a6b3912b2 100644
--- a/ICSharpCode.Decompiler.Tests/DebugStepRecordingTests.cs
+++ b/ICSharpCode.Decompiler.Tests/DebugStepRecordingTests.cs
@@ -146,6 +146,45 @@ namespace ICSharpCode.Decompiler.Tests
}
}
+ ///
+ /// 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.
+ ///
+ [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");
+ }
+ }
+
+ ///
+ /// 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.
+ ///
+ [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");
+ }
+
///
/// 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;
}
diff --git a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs
index 4e7d729d4..6eab6a2c0 100644
--- a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs
+++ b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs
@@ -236,20 +236,20 @@ namespace ICSharpCode.Decompiler.CSharp
public Stepper Stepper { get; set; } = new Stepper();
///
- /// Gets or sets whether the IL transform phase records its steps into ,
- /// 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 , 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 retention: 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 retained, 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 .
///
- public bool RecordILTransformSteps { get; set; }
+ public bool RecordSteps { get; set; }
///
/// The whose IL transforms were halted by ,
@@ -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)
diff --git a/ILSpy.Tests/Views/DebugStepsTests.cs b/ILSpy.Tests/Views/DebugStepsTests.cs
index 83f783d81..90df8ec81 100644
--- a/ILSpy.Tests/Views/DebugStepsTests.cs
+++ b/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().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().SetRecordingEnabled(true);
var window = AppComposition.Current.GetExport();
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().SetRecordingEnabled(true);
+
var window = AppComposition.Current.GetExport();
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();
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 AllDescriptions(
System.Collections.Generic.IEnumerable 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()
.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;
diff --git a/ILSpy/Languages/CSharpLanguage.DebugSteps.cs b/ILSpy/Languages/CSharpLanguage.DebugSteps.cs
index dc9bc6546..5e729e4cb 100644
--- a/ILSpy/Languages/CSharpLanguage.DebugSteps.cs
+++ b/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;
}
///
- /// 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 .
+ /// 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.
+ ///
+ static DebugStepsPaneModel? DebugStepsPane => AppComposition.TryGetExport();
+
+ ///
+ /// Points the whole pipeline at the shared stepper, but only while the Debug Steps pane is
+ /// there to display what it records - see .
///
static partial void ConfigureStepRecording(CSharpDecompiler decompiler)
{
- decompiler.RecordILTransformSteps = DebugStepsPaneModel.IsRecording;
+ decompiler.RecordSteps = DebugStepsPane?.IsRecording ?? false;
}
///
diff --git a/ILSpy/ViewModels/DebugStepsPaneModel.cs b/ILSpy/ViewModels/DebugStepsPaneModel.cs
index ec2f5d3c0..d43e9a5a6 100644
--- a/ILSpy/ViewModels/DebugStepsPaneModel.cs
+++ b/ILSpy/ViewModels/DebugStepsPaneModel.cs
@@ -61,10 +61,10 @@ namespace ICSharpCode.ILSpy.ViewModels
/// . 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.
///
- public static bool IsRecording { get; private set; }
+ public bool IsRecording { get; private set; }
readonly LanguageService? languageService;
@@ -74,10 +74,10 @@ namespace ICSharpCode.ILSpy.ViewModels
///
/// 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 is.
///
- public static ILAstWritingOptions WritingOptions { get; } = new() {
+ public ILAstWritingOptions WritingOptions { get; } = new() {
UseFieldSugar = true,
UseLogicOperationSugar = true,
};