feat(package): Resolve source file and line to patch locations via portable PDBs#1681
Conversation
Adds a dedicated test assembly (UnityCLILoop.Tests.Editor.SourcePausePointResolver) isolated from the large shared Tests.Editor assembly, plus six frozen fixture scripts (normal statement, branching, try/finally, async, iterator/coroutine, local function) whose exact line numbers the upcoming Resolver tests assert against. Each fixture is marked frozen in a header comment; add a new fixture file instead of editing an existing one.
Resolves a project-relative file:line into a patchable method, its instruction index, and the locals/parameters visible there, by reading the compiled assembly's portable PDB with Mono.Cecil. Failures (script outside any assembly, missing compiled output, missing symbols, no sequence point on or after the requested line) are returned as data on a Result-style DTO instead of thrown, per this repo's Fail Fast + no-try-catch convention. Sequence point search covers nested compiler-generated types so async methods, iterators, and local functions resolve into their real MoveNext/local-function body rather than the outer declared method. Local variables are read from the PDB's lexical scope tree (so try/finally handlers correctly see locals declared in the method's root scope) and byref/pointer/ref-struct locals are excluded since they cannot be boxed for capture. Document-path matching normalizes separators so Windows-style absolute PDB paths still match a project-relative input path. All 9 scenarios (normal statement, branching, try/finally, async, iterator, local function, and the 3 failure paths) are covered against real compiled output from the frozen fixtures added in the previous commit.
|
Warning Review limit reached
Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a source pause-point resolver that maps project-relative file and line locations to compiled Unity methods, instructions, locals, and parameters using portable PDB data, with structured failures and NUnit coverage. ChangesSource pause-point resolution
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant TestCaller
participant SourcePausePointResolver
participant UnityCompiledAssembly
participant PortablePDB
participant ResolveResult
TestCaller->>SourcePausePointResolver: Resolve(file, line)
SourcePausePointResolver->>UnityCompiledAssembly: Locate owning DLL
SourcePausePointResolver->>PortablePDB: Read portable symbols
PortablePDB-->>SourcePausePointResolver: Return sequence points and scopes
SourcePausePointResolver->>ResolveResult: Return resolution or failure
ResolveResult-->>TestCaller: Expose result data
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Ref-struct locals (user-defined and Span<T>/ReadOnlySpan<T>) and ref/out/in parameters cannot be boxed, so leaving them uncaptured would let Harmony patch a pause point that crashes with InvalidProgramException at runtime (PR 4). Detect user-defined ref structs via the compiler-emitted IsByRefLikeAttribute, but only when the type reference already resolves to a TypeDefinition in the same module -- calling TypeReference.Resolve() on external references (e.g. corlib System.Int32) throws AssemblyResolutionException in the Unity Editor's Mono/.NET layout, so that path is intentionally avoided. Also renames the overloaded EnumerateMethods pair to EnumerateMethodsInModule/EnumerateMethodsInType (this project disallows overloads) and adds IsDeclaringTypeValueType to SourcePausePointResolution for the struct `this`-as-managed-pointer case PR 4 will need. Addresses Fable 5's PR #1681 review must-fix items.
A generic ref struct (e.g. MyRefStruct<int>) is a GenericInstanceType, not a TypeDefinition, so the prior TypeDefinition check let it slip through uncaptured-check unfiltered. Apply the same generic-instance unwrap already used for the Span<T>/ReadOnlySpan<T> fast path before the TypeDefinition check, and extend the fixture with a generic ref struct local to close the gap. Addresses Fable 5's conditional LGTM on PR #1681.
Refs: uloop Source Pause Point 実行計画 (Obsidian) — PR 2 / Phase 2
Summary
--file/--linepause point feature: given a project-relative script path and a line number, resolve which compiled method/instruction to patch and which locals/parameters are in scope there.User Impact
enable-pause-point --file/--lineis not wired up until PR 5.Changes
SourcePausePointResolver: reads the target script's compiled assembly (Library/ScriptAssemblies/*.dll+ portable.pdb) via Mono.Cecil and finds the closest sequence point on or after the requested line (rounding forward, skipping hidden sequence points), searching nested compiler-generated types so async methods / iterators / local functions resolve into their realMoveNext/local-function body instead of the outer declared method.SourcePausePointResolveResult/SourcePausePointResolution) instead of throwing; distinguishes "script not in any assembly", "compiled assembly not found", "symbols unavailable", and "no sequence point on or after line" failures.try/finally), excluding unnamed/compiler-generated locals and byref/pointer/ref-struct (Span<T>/ReadOnlySpan<T>) locals that cannot be boxed for capture.SourcePausePointPathNormalizercompares a PDB document URL against the project-relative input path tolerating separator and absolute-vs-relative differences (Windows Compatibility Guardrails).Verification
dist/darwin-arm64/uloop compile— 0 errors, 0 warningsdist/darwin-arm64/uloop run-tests --test-mode EditMode(full suite) — 1800 passed, 0 failed, 7 skipped (pre-existing)