diff --git a/Assets/Tests/Editor/HotReload/HotReloadPausePointContractTests.cs b/Assets/Tests/Editor/HotReload/HotReloadPausePointContractTests.cs
index aabeef07f6..53844a8c3e 100644
--- a/Assets/Tests/Editor/HotReload/HotReloadPausePointContractTests.cs
+++ b/Assets/Tests/Editor/HotReload/HotReloadPausePointContractTests.cs
@@ -801,6 +801,79 @@ public async Task Enable_OnHotReloadedLocalFunctionBody_CapturesInstanceAsThis()
FormatCaptured(status));
}
+ ///
+ /// What: a pause point inside a local function of a hot-reloaded method, where a lambda
+ /// capturing the loop variable turns the loop scope into a closure class, captures the
+ /// instance as "this" with its fields and the variables of every enclosing scope, although
+ /// the instance and the method parameter travel in a by-ref closure struct that the loop
+ /// closure class does not link to.
+ ///
+ [Test]
+ public async Task Enable_OnHotReloadedLocalFunctionOfLoopClosure_CapturesInstanceAndEnclosingScopes()
+ {
+ string onDisk = File.ReadAllText(ResolveFixtureAbsolutePath());
+ const string compiledBody =
+ "public int ComputeWithPrivate(int delta)\n {\n"
+ + " return _secret + delta;\n"
+ + " }";
+ Assert.That(onDisk, Does.Contain(compiledBody), "Precondition: ComputeWithPrivate body must exist.");
+ string editedSource = onDisk.Replace(
+ compiledBody,
+ "public int ComputeWithPrivate(int delta)\n {\n"
+ + " int total = 0;\n"
+ + " for (int j = 0; j < 1; j++)\n {\n"
+ + " Func read = () => j;\n"
+ + " int Local()\n {\n"
+ + " int loopSum = _secret + delta + j;\n"
+ + " return loopSum;\n"
+ + " }\n\n"
+ + " total += Local() + read();\n"
+ + " }\n\n"
+ + " return total;\n"
+ + " }",
+ StringComparison.Ordinal);
+ int enableLine = FindLineNumber(editedSource, "return loopSum;");
+ Assert.That(enableLine, Is.GreaterThan(0));
+
+ await HotReloadFromEditedSourceAsync(editedSource, "ContractLoopClosureLocalFunction.cs");
+
+ PausePointResponse enable = new PausePointUseCase().Enable(new EnablePausePointSchema
+ {
+ File = FixtureProjectRelativePath,
+ Line = enableLine,
+ TimeoutSeconds = 30,
+ Mode = UloopPausePointCaptureMode.Continuous
+ });
+ Assert.That(enable.Success, Is.True, enable.Message + " / " + enable.RecommendedNextAction);
+ Assert.That(enable.RetargetedToHotReloadPatch, Is.True);
+ Assert.That(enable.ResolvedLine, Is.GreaterThan(0));
+
+ HotReloadE2EFixture fixture = new HotReloadE2EFixture();
+ Assert.That(fixture.ComputeWithPrivate(5), Is.EqualTo(fixture.SecretForAssert + 5));
+
+ UloopPausePointSnapshot status = UloopPausePointRegistry.GetStatus(enable.Id);
+ Assert.That(status.IsHit, Is.True);
+ Assert.That(status.CapturedVariables.Any(v => v.Name == "this"), Is.True, FormatCaptured(status));
+ Assert.That(
+ status.CapturedVariables.Any(v => v.Name == "_secret" && v.Scope == UloopCapturedVariableScope.InstanceField),
+ Is.True,
+ FormatCaptured(status));
+ Assert.That(
+ status.CapturedVariables.Any(v => v.Name == "delta" && v.Value == "5"),
+ Is.True,
+ FormatCaptured(status));
+ Assert.That(
+ status.CapturedVariables.Any(v => v.Name == "j" && v.Value == "0"),
+ Is.True,
+ FormatCaptured(status));
+ Assert.That(
+ status.CapturedVariables.Any(v => v.Name == HotReloadShimMethodLookup.ShimReceiverParameterName),
+ Is.False,
+ FormatCaptured(status));
+ // Read from the registry: the enable response turns an empty list into null.
+ Assert.That(status.NotCapturableVariables, Is.Empty);
+ }
+
///
/// What: RevertAll after enable on a hot-reloaded async body restores instrumentation
/// onto the compiled MoveNext and hits with the original (non-edited) return value.
diff --git a/Assets/Tests/Editor/PausePointToolModeTests.cs b/Assets/Tests/Editor/PausePointToolModeTests.cs
index 88120f8cba..f89ffb2b54 100644
--- a/Assets/Tests/Editor/PausePointToolModeTests.cs
+++ b/Assets/Tests/Editor/PausePointToolModeTests.cs
@@ -289,7 +289,7 @@ public void Hit_WhenMarkerHasMaxPreviewElementsOverride_UsesOverrideForCollectio
List values = Enumerable.Range(0, maxPreviewElements + 5).ToList();
SourcePausePointCapture.Capture(
- "jump", null, Array.Empty
internal static class SourcePausePointCaptureEligibility
{
+ private const string CompilerGeneratedAttributeFullName =
+ "System.Runtime.CompilerServices.CompilerGeneratedAttribute";
+
internal static List CollectCapturableLocals(
MethodDefinition method,
int offset)
@@ -164,7 +167,9 @@ private static bool IsCaptureExcluded(TypeReference type)
// byref locals/parameters (ref, out, in) and pointers cannot be boxed; ref structs
// (Span, and any user-defined "ref struct") cannot be boxed either.
// Why derived from the reason: the excluded set and the reported set must stay the
- // same set, so a parameter is never both captured and named as not capturable.
+ // same set, so a parameter is never both captured and named as not capturable. The one
+ // exception is a by-ref closure struct, which is excluded but not reported because its
+ // variables are captured through it (see SourcePausePointClosureFrameArgument).
return DescribeNotCapturableReason(type).Length > 0;
}
@@ -221,6 +226,11 @@ internal static List CollectNotCapturableParameters(MethodDefinition met
List results = new List();
foreach (ParameterDefinition parameter in method.Parameters)
{
+ if (IsClosureFrameParameter(parameter.ParameterType))
+ {
+ continue;
+ }
+
string reason = DescribeNotCapturableReason(parameter.ParameterType);
if (reason.Length == 0)
{
@@ -302,6 +312,11 @@ internal static List CollectNotCapturableParametersFromReflection(
for (int index = startIndex; index < runtimeParameters.Length; index++)
{
System.Reflection.ParameterInfo parameter = runtimeParameters[index];
+ if (SourcePausePointClosureFrameArgument.FrameTypeOrNull(parameter.ParameterType) != null)
+ {
+ continue;
+ }
+
string reason = DescribeNotCapturableReasonFromReflection(parameter.ParameterType);
if (reason.Length == 0)
{
@@ -314,6 +329,34 @@ internal static List CollectNotCapturableParametersFromReflection(
return results;
}
+ // Cecil counterpart of SourcePausePointClosureFrameArgument.FrameTypeOrNull. A closure
+ // struct is always declared in the module being read, so its reference is already a
+ // TypeDefinition and no assembly has to be resolved.
+ private static bool IsClosureFrameParameter(TypeReference type)
+ {
+ if (!(type is ByReferenceType byReferenceType))
+ {
+ return false;
+ }
+
+ if (!(byReferenceType.ElementType is TypeDefinition elementType)
+ || !elementType.IsValueType
+ || !elementType.HasCustomAttributes)
+ {
+ return false;
+ }
+
+ foreach (CustomAttribute attribute in elementType.CustomAttributes)
+ {
+ if (attribute.AttributeType.FullName == CompilerGeneratedAttributeFullName)
+ {
+ return true;
+ }
+ }
+
+ return false;
+ }
+
///
/// The reason a Cecil type is excluded from capture, or empty when it is capturable.
///
diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointClosureFrameArgument.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointClosureFrameArgument.cs
index 4a14373d6e..32714a8f52 100644
--- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointClosureFrameArgument.cs
+++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointClosureFrameArgument.cs
@@ -1,54 +1,37 @@
using System;
+using System.Collections.Generic;
using System.Reflection;
using System.Runtime.CompilerServices;
-using io.github.hatayama.UnityCliLoop.ToolContracts;
-
namespace io.github.hatayama.UnityCliLoop.FirstPartyTools
{
///
- /// Finds the closure struct a static local function receives by reference, so a pause point in
- /// it can capture the variables the local function shares with its enclosing method.
+ /// Finds the closure structs a local function receives by reference, so a pause point in it
+ /// can capture the variables and the instance it shares with its enclosing scopes.
///
///
/// A local function that captures variables but is not converted to a delegate is compiled to a
- /// method taking its enclosing scopes as by-ref struct arguments. When the enclosing method is a
- /// hot-reload shim, the instance travels in that struct as the shim's receiver field, so the
- /// local function is static and has no `this` of its own.
+ /// method taking one by-ref struct argument per enclosing scope it uses, after its own declared
+ /// parameters. This happens whether the local function itself is static, an instance method of
+ /// the declaring class, or an instance method of a lambda's closure class. The instance often
+ /// travels inside one of those structs, as `<>4__this` or as a hot-reload shim's receiver field.
///
internal static class SourcePausePointClosureFrameArgument
{
- private const string OuterThisFieldName = "<>4__this";
-
- ///
- /// The GetParameters() index of the closure struct to capture, or -1 when there is none.
- /// The struct that holds the instance wins, because the instance is what the pause point
- /// most needs; otherwise the first one.
- ///
- internal static int FindIndexOrMinusOne(MethodBase method)
+ /// The GetParameters() indexes of every closure struct argument, in declaration order.
+ internal static List FindIndexes(MethodBase method)
{
System.Reflection.ParameterInfo[] parameters = method.GetParameters();
- int firstFrameIndex = -1;
+ List indexes = new List();
for (int index = 0; index < parameters.Length; index++)
{
- Type frameType = FrameTypeOrNull(parameters[index].ParameterType);
- if (frameType == null)
- {
- continue;
- }
-
- if (HoldsInstance(frameType))
+ if (FrameTypeOrNull(parameters[index].ParameterType) != null)
{
- return index;
- }
-
- if (firstFrameIndex < 0)
- {
- firstFrameIndex = index;
+ indexes.Add(index);
}
}
- return firstFrameIndex;
+ return indexes;
}
/// The struct type behind a by-ref closure argument, or null for any other parameter.
@@ -67,12 +50,5 @@ internal static Type FrameTypeOrNull(Type parameterType)
return Attribute.IsDefined(elementType, typeof(CompilerGeneratedAttribute)) ? elementType : null;
}
-
- private static bool HoldsInstance(Type frameType)
- {
- const BindingFlags instanceFields = BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic;
- return frameType.GetField(HotReloadShimMethodLookup.ShimReceiverParameterName, instanceFields) != null
- || frameType.GetField(OuterThisFieldName, instanceFields) != null;
- }
}
}
diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointInjectionEmitter.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointInjectionEmitter.cs
index 5b44ccc836..db41f4d3c6 100644
--- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointInjectionEmitter.cs
+++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointInjectionEmitter.cs
@@ -157,6 +157,7 @@ private static List BuildInjection(SourcePausePointPatchInjecti
};
AppendInstanceLoad(emitted, injection, method);
+ AppendClosureFramesLoad(emitted, method);
// InstanceFromFirstArgument stores absolute GetParameters() indexes (including the
// hole left by skipping __uloopInstance at 0), so argOffset stays 0. Ordinary
@@ -252,7 +253,7 @@ private static void AppendInstanceLoad(List emitted, SourcePaus
if (injection.IsStatic)
{
- AppendStaticInstanceLoad(emitted, method);
+ emitted.Add(new CodeInstruction(OpCodes.Ldnull));
return;
}
@@ -276,24 +277,34 @@ private static void AppendInstanceLoad(List emitted, SourcePaus
}
}
- // Why a static method may still pass an instance: a local function receives its enclosing
- // scopes as a by-ref closure struct, and the collector walks that struct to the variables
- // and the instance it holds, as it does for a lambda's closure class. The struct is copied
- // into the box, so the capture is a snapshot and cannot write back into the frame.
- private static void AppendStaticInstanceLoad(List emitted, MethodBase method)
+ // Why pass the by-ref closure structs separately from the instance: a local function
+ // receives the variables of its enclosing scopes, and often the instance, in those structs,
+ // and the collector walks them the way it walks a lambda's closure class. They cannot be
+ // captured as parameters because by-ref arguments cannot be boxed as they are; each struct
+ // is copied into a box, so the capture is a snapshot and cannot write back into the frame.
+ private static void AppendClosureFramesLoad(List emitted, MethodBase method)
{
- int frameIndex = SourcePausePointClosureFrameArgument.FindIndexOrMinusOne(method);
- if (frameIndex < 0)
+ List frameIndexes = SourcePausePointClosureFrameArgument.FindIndexes(method);
+ // Fully qualify: ToolContracts also defines ParameterInfo (tool schema DTO).
+ System.Reflection.ParameterInfo[] parameters = method.GetParameters();
+ // GetParameters() omits the hidden `this` that occupies argument slot 0 of an instance
+ // method. Decided by the patched method itself: a hot-reload shim taking the instance as
+ // its first declared parameter is static, so its indexes already match its slots.
+ int slotOffset = method.IsStatic ? 0 : 1;
+
+ emitted.Add(new CodeInstruction(OpCodes.Ldc_I4, frameIndexes.Count));
+ emitted.Add(new CodeInstruction(OpCodes.Newarr, typeof(object)));
+ for (int i = 0; i < frameIndexes.Count; i++)
{
- emitted.Add(new CodeInstruction(OpCodes.Ldnull));
- return;
+ int frameIndex = frameIndexes[i];
+ Type frameType = SourcePausePointClosureFrameArgument.FrameTypeOrNull(parameters[frameIndex].ParameterType);
+ emitted.Add(new CodeInstruction(OpCodes.Dup));
+ emitted.Add(new CodeInstruction(OpCodes.Ldc_I4, i));
+ emitted.Add(CodeInstruction.LoadArgument(frameIndex + slotOffset, false));
+ emitted.Add(new CodeInstruction(OpCodes.Ldobj, frameType));
+ emitted.Add(new CodeInstruction(OpCodes.Box, frameType));
+ emitted.Add(new CodeInstruction(OpCodes.Stelem_Ref));
}
-
- Type frameType = SourcePausePointClosureFrameArgument.FrameTypeOrNull(
- method.GetParameters()[frameIndex].ParameterType);
- emitted.Add(CodeInstruction.LoadArgument(frameIndex, false));
- emitted.Add(new CodeInstruction(OpCodes.Ldobj, frameType));
- emitted.Add(new CodeInstruction(OpCodes.Box, frameType));
}
private static void AppendNameValueArray(
diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointVariableCollector.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointVariableCollector.cs
index fa5e19c788..84f4c24ded 100644
--- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointVariableCollector.cs
+++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointVariableCollector.cs
@@ -24,9 +24,15 @@ internal static class SourcePausePointVariableCollector
// "this", so this never collides with a captured local, parameter, or field.
private const string ThisEntryName = "this";
+ ///
+ /// Lists locals, then parameters, then the variables the compiler-generated holders carry,
+ /// then the instance as "this" followed by its fields. The holders are the instance when it
+ /// is a closure or state machine, and every closure struct in closureFrames.
+ ///
public static UloopPausePointCapturedVariableFrame Collect(
- object instance, object[] parameterNamesAndValues, object[] localNamesAndValues)
+ object instance, object[] closureFrames, object[] parameterNamesAndValues, object[] localNamesAndValues)
{
+ Debug.Assert(closureFrames != null, "closureFrames must not be null");
Debug.Assert(parameterNamesAndValues != null, "parameterNamesAndValues must not be null");
Debug.Assert(localNamesAndValues != null, "localNamesAndValues must not be null");
Debug.Assert(parameterNamesAndValues.Length % 2 == 0, "parameterNamesAndValues must contain name/value pairs");
@@ -47,12 +53,9 @@ public static UloopPausePointCapturedVariableFrame Collect(
entries, capturedNames, truncatedVariableNames, ref truncatedVariableCount, ref truncated,
parameterNamesAndValues, UloopCapturedVariableScope.Parameter);
- if (instance != null)
- {
- CollectInstanceFieldVariables(
- instance, entries, capturedNames, truncatedVariableNames, ref truncatedVariableCount,
- ref truncated);
- }
+ CollectHolderAndInstanceVariables(
+ instance, closureFrames, entries, capturedNames, truncatedVariableNames, ref truncatedVariableCount,
+ ref truncated);
return new UloopPausePointCapturedVariableFrame(
entries, truncated, truncatedVariableNames, truncatedVariableCount);
@@ -77,39 +80,59 @@ private static void AppendPairs(
}
}
- private static void CollectInstanceFieldVariables(
+ private static void CollectHolderAndInstanceVariables(
object instance,
+ object[] closureFrames,
List entries,
HashSet capturedNames,
List truncatedVariableNames,
ref int truncatedVariableCount,
ref bool truncated)
{
- // Normal method: the paused instance itself is `this`, emitted before its fields so the
- // count cap keeps prioritizing locals and parameters over instance state.
- if (!IsCompilerGeneratedHolder(instance.GetType()))
+ // A normal instance is `this` itself. A compiler-generated instance (closure or state
+ // machine) is only a holder: the real `this` is found through its links, never the
+ // holder itself.
+ object receiver = instance != null && !IsCompilerGeneratedHolder(instance.GetType()) ? instance : null;
+ object outerThis = null;
+ if (instance != null && receiver == null)
{
- TryAppendEntry(
- entries, capturedNames, truncatedVariableNames, ref truncatedVariableCount, ref truncated,
- ThisEntryName, UloopCapturedVariableScope.This, instance);
+ object linkedOuterThis = CollectDirectFieldVariables(
+ instance, entries, capturedNames, truncatedVariableNames, ref truncatedVariableCount,
+ ref truncated, followOuterThis: true);
+ outerThis ??= linkedOuterThis;
+ }
+
+ // Closure structs hold variables of the enclosing scopes, so they are listed before the
+ // instance state to keep the count cap prioritizing variables over fields. Every struct
+ // is walked even after one of them links to the instance, because each carries the
+ // variables of its own scope.
+ foreach (object closureFrame in closureFrames)
+ {
+ if (closureFrame == null)
+ {
+ continue;
+ }
+
+ object linkedOuterThis = CollectDirectFieldVariables(
+ closureFrame, entries, capturedNames, truncatedVariableNames, ref truncatedVariableCount,
+ ref truncated, followOuterThis: true);
+ outerThis ??= linkedOuterThis;
}
- object outerThis = CollectDirectFieldVariables(
- instance, entries, capturedNames, truncatedVariableNames, ref truncatedVariableCount,
- ref truncated, followOuterThis: true);
- if (outerThis == null)
+ object thisValue = receiver ?? outerThis;
+ if (thisValue == null)
{
return;
}
- // State machine or closure: the real `this` is the outer instance found through the
- // compiler-generated holders, never a holder itself. Emit it before its fields.
TryAppendEntry(
entries, capturedNames, truncatedVariableNames, ref truncatedVariableCount, ref truncated,
- ThisEntryName, UloopCapturedVariableScope.This, outerThis);
+ ThisEntryName, UloopCapturedVariableScope.This, thisValue);
+ // Why no link following: only compiler-generated holders link outward, and `this` is
+ // never one of them.
CollectDirectFieldVariables(
- outerThis, entries, capturedNames, truncatedVariableNames, ref truncatedVariableCount,
+ thisValue, entries, capturedNames, truncatedVariableNames, ref truncatedVariableCount,
ref truncated, followOuterThis: false);
}
diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointVariableFormatter.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointVariableFormatter.cs
index a5c2353af9..96de8e6830 100644
--- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointVariableFormatter.cs
+++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointVariableFormatter.cs
@@ -24,7 +24,7 @@ public static (List Variables, bool Truncated) Format(
int maxCollectionPreviewElementCount = SourcePausePointConstants.MaxCollectionPreviewElementCount)
{
UloopPausePointCapturedVariableFrame frame = SourcePausePointVariableCollector.Collect(
- instance, parameterNamesAndValues, localNamesAndValues);
+ instance, Array.Empty