feat(extractor): resolve pool Rent/Return via SemanticModel → aliased receivers (recall 6/9→7/10)#53
Merged
Merged
Conversation
…7/10)
Pool recognition matched the receiver's TEXT (Contains("Pool")), which is blind
to an aliased receiver (`ArrayPool<int> p = ArrayPool<int>.Shared; p.Rent(...);
p.Return(buf)` spells the receiver `p`, no "Pool") and can false-match an
unrelated `_connectionPool.Rent()`. Both failure modes go away by binding the
call through the Roslyn SemanticModel to System.Buffers.ArrayPool<T>:
- IsPoolRent / PoolReturnBuffer now resolve the invoked method symbol and check
its containing type is System.Buffers.ArrayPool<T> (new IsArrayPoolType); the
receiver may be spelled any way.
- The SemanticModel is threaded through LowerFlowBody -> LowerFlowStmt ->
EmitFlowExpr (and LowerSwitchSection); the candidates loop and POOL001 already
had it in scope.
- POOL001's Rent recognition now calls the same IsPoolRent, so the syntactic and
flow detectors share ONE definition of "is this a pool" (no divergence).
- ArrayPool-specific by design: MemoryPool<T>.Rent returns an IMemoryOwner<T>
released by Dispose (no Return to model) -> it rides the IDisposable path.
Proof + pin: new corpus case arraypool-aliased-receiver -- a double-return
reached through `var p = ArrayPool<int>.Shared` -- is a MISS under the old text
heuristic and a catch (OWN003) under the semantic one. Recall 6/9 -> 7/10;
specificity stays 10/10, 0 FP; CI floor raised to --min-recall 7. The
heuristic's failure mode was recall-leaning (a missed alias is a missed catch,
never a false alarm), so the upgrade only adds -- precision stays absolute.
case.own validated locally (test_corpus 6/6); the extractor change is validated
by the corpus-benchmark + wpf-extractor CI jobs (no local .NET SDK).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThe Roslyn extractor's ChangesSemantic ArrayPool Detection and Aliased-Receiver Fixture
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
This was referenced Jun 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Moves pool
Rent/Returnrecognition off the receiver-TEXT heuristic onto the Roslyn SemanticModel, so a pooled buffer is found no matter how its pool is spelled — corpus recall 6/9 → 7/10.The pool pass matched a
Rent/Returnby the text of the receiver —Contains("Pool"). That is brittle in both directions:ArrayPool<int> p = ArrayPool<int>.Shared; p.Rent(...); p.Return(buf);spells the receiverp, with no "Pool" in it, so the buffer is invisible and a double-return through it is a miss;_connectionPool.Rent().Both go away by binding the call through the SemanticModel to
System.Buffers.ArrayPool<T>:IsPoolRent/PoolReturnBuffernow resolve the invoked method symbol and check its containing type isSystem.Buffers.ArrayPool<T>(newIsArrayPoolType); the receiver may be spelled any way.SemanticModelis threaded throughLowerFlowBody → LowerFlowStmt → EmitFlowExpr(andLowerSwitchSection); the candidates loop andPOOL001already had it in scope.POOL001's Rent recognition now calls the sameIsPoolRent, so the syntactic and flow detectors can't disagree about what a pool is. (This is the unified upgrade I committed to on feat(extractor): pooled buffers through the flow engine → OWN003/OWN002 (recall 4/9→6/9) #52 rather than half-tightening one side.)MemoryPool<T>.Renthands back anIMemoryOwner<T>released byDispose— there is noReturnto model — so a pooledMemoryPoolowner rides the IDisposable path, not this one. (The exact distinction CodeRabbit recorded as a learning on feat(extractor): pooled buffers through the flow engine → OWN003/OWN002 (recall 4/9→6/9) #52.)The proof (benchmark, CI)
New corpus case
arraypool-aliased-receiver— a double-return reached throughvar p = ArrayPool<int>.Shared— is a miss under the old text heuristic and a catch (OWN003) under the semantic one. So the benchmark goes 6/9 → 7/10; specificity stays 10/10, 0 false positives; the CI floor rises to--min-recall 7.The heuristic's failure mode was recall-leaning — a missed alias is a missed catch, never a false alarm — so this upgrade only adds; precision stays absolute (and the
corpus-benchmarkgate pins 0 FP as the live guard).case.ownis validated locally (test_corpus6/6); the extractor change has no local .NET SDK, so it is validated by thecorpus-benchmark+wpf-extractorCI jobs. The existingArrayPool<int>.Sharedcases and thePooledBufferSamplePOOL001 assertion confirm no regression — those receivers resolve to the same symbol (ArrayPool<T>lives inSystem.Private.CoreLib, always in the compilation's trusted-platform references).Files
frontend/roslyn/OwnSharp.Extractor/Program.cs— semanticIsPoolRent/PoolReturnBuffer+IsArrayPoolType;SemanticModelthreaded through the flow-lowering chain;POOL001routed through the shared helper.corpus/real-world/arraypool-aliased-receiver/— the aliased-receiver double-return (before/after/case.own/expected/notes)..github/workflows/ci.yml— floor--min-recall 7.docs/—corpus-benchmark.md(the→ 7/10ratchet) + P-012 status.Follow-up still open (tracked from #52): the
Return-side name collection inPOOL001stays syntactic (it keys on already-confirmed pool rents, so it's precision-safe); a fuller pass could route that through the symbol model too.🤖 Generated with Claude Code
https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED
Generated by Claude Code
Summary by CodeRabbit
Release Notes
New Features
Tests
Documentation