feat: interprocedural stage 2 — per-overload signature keys (sig) in the MOS#217
Merged
Conversation
Same-name overloads no longer have to share one conservative summary. A functions[] record and a call op may both carry an optional 'sig' — the canonical parameter-type list (FQ names, comma-separated, no spaces, generic arity via backtick, global:: stripped). When an overloaded name's records carry it, _build_skeletons emits one summary per overload keyed name(sig) BESIDE the name-merged fallback, and every consumer resolves through one helper (_mos_lookup: sig key -> exact name -> canonical name): the overload channel, kill sites, OWN051 collection, the fresh-result mint, branch hoisting and the method's own param seeding — so the untrack set, the advisories and the emitted effects can never disagree. The fallback rule is load-bearing (tz §6.1): a sig missing or unmatched on either side of an edge lands on the merged summary — degraded, never a wrong overload — and the first_party/overloaded suppressions stay keyed on the bare name (INV4). Additive per the IR evolution policy: no OWNIR_VERSION bump; a non-string sig on a record fails load, on a flow op it reads as absent. Extractor: sig is stamped from the same resolved IMethodSymbol on both sides of the edge (CanonicalSig/CanonicalTypeName; type arguments erased to backtick arity — a collision merely merges, never mis-resolves), on the main flow record and the first-party-factory call op. Also fixes a latent parity-dump nondeterminism the new matrix exposed: _merge_skeletons took the first-seen param name per index, which depended on functions[] input order; it now takes the lexicographic min. Recall restored (e2e anchor OverloadSigSample.cs + CI assertions): a sig'd call to the fresh overload of a mixed factory leaks its dropped result (OWN001); a sig'd forward to a consume overload yields OWN002 — both were silenced by the merge. Test matrix from tz §6.1 lands in tests/test_ownir.py (14 new checks); solver untouched (keys stay opaque). Tier B review (DoD): Process.Start / new StreamReader(stream) stay excluded — their ambiguity is BCL-side (static-vs-instance, adopt); a sig-keyed Tier B table is now possible and recorded as follow-up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LsWw4Ay8KLTHFom1HvRu3U
![github-advanced-security[bot]](https://cdn.github.com/in/57789?s=60&v=4){kind=small}
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughStage 2 interprocedural ownership analysis now supports canonical per-overload ChangesInterprocedural overload signatures
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CSharpExtractor
participant OwnIRFacts
participant OwnIRBridge
participant MOS
participant CI
CSharpExtractor->>OwnIRFacts: Emit functions[].sig and call.sig
OwnIRFacts->>OwnIRBridge: Load validated facts
OwnIRBridge->>MOS: Resolve name(sig)
MOS-->>OwnIRBridge: Return overload contract or merged fallback
OwnIRBridge->>CI: Produce ownership findings
CI-->>CI: Verify signatures and OWN001 for Drop
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![chatgpt-codex-connector[bot]](https://cdn.github.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 540e07dc60
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…n, extension-method sigs The wpf-extractor CI job runs WITHOUT --flow-locals, so functions[] is always empty there and the stage-2 assertions could never pass (first CI run caught it). Move OverloadSigSample.cs and its assertions into the dedicated flow-locals step (flow.json), rename its locals (made->opened, s->dropped) clear of the step's existing silent-case guard, and pin the exact leak wording plus the opened/probe silent cases. Codex P2 x2, both verified real: - call_key now canonicalizes the sig-less/unmatched fallback: a forward or return edge to a global::-qualified first-party callee lands on the bare-name merged summary exactly like _mos_lookup resolves a direct call, instead of going extern (unknown) and downgrading a resolvable must/fresh to OWN051/silence. Regression test: qualified forward to agreeing-consume overloads yields OWN002 (271/271). - CanonicalSig canonicalizes through (ReducedFrom ?? m).OriginalDefinition so a reduced extension-method call site (receiver dropped from Parameters) and a constructed generic (type args substituted) produce the same string the declared functions[] record stamps — the precise per-overload contract applies instead of the merged fallback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LsWw4Ay8KLTHFom1HvRu3U
|
|
||
| public static void Drop(string path) | ||
| { | ||
| var dropped = Open(path); // sig'd call -> the fresh overload's contract |
![coderabbitai[bot]](https://cdn.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 260-271: Update the e2e assertion around the `Drop` finding to
match the specific dropped-result/`Drop` location in `OverloadSigSample.cs`,
rather than accepting any `OWN001` from that file. Also validate the call
operation’s `sig` structurally, using the expected `System.String` signature, so
the test proves overload-specific resolution and rejects missing or unmatched
call-side signatures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b7a38f6a-5c0e-4d83-bde1-2d0ec608b1a8
📒 Files selected for processing (10)
.github/workflows/ci.ymldocs/notes/interprocedural-roadmap.mddocs/notes/interprocedural-tz.mdfrontend/roslyn/OwnSharp.Extractor/Program.csfrontend/roslyn/samples/OverloadSigSample.csownlang/ownership.pyownlang/ownir.pyspec/OwnIR.mdspec/ownir.schema.jsontests/test_ownir.py
Replace the two flow.json greps (which either side of the edge could satisfy alone) with a structural check: the two SigOverloads.Open records must carry exactly the (string) and (FileStream,bool) sigs, and Drop's call op must carry callee/sig/result for the (string) overload — so a missing or unmatched CALL-side sig fails on its own, not only via the downstream OWN001 assertion. Dry-run locally against the extractor fact shape, positive and negative. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LsWw4Ay8KLTHFom1HvRu3U
|
@coderabbitai review |
✅ Action performedReview finished.
|
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
3 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.
Что и зачем
Этап 2 межпроцедурной карты (roadmap §5 / tz §6.1): перегрузки больше не сливаются в одно консервативное summary. Опциональное поле
sig(канонический список типов параметров) наfunctions[]-записи иcall-опе даёт каждой перегрузке собственный ключname(sig)РЯДОМ со слитым по имени fallback'ом; все потребители резолвятся через единый_mos_lookup(sig-ключ → точное имя → каноническое). Возвращает recall, который merge терял: sig-вызов fresh-перегрузки смешанной фабрики → OWN001 на брошенном результате, sig-вызов/forward в consume-перегрузку → OWN002; borrow-перегрузка → проверенный lend без OWN051-шума. Fallback несущий: нет/не совпалsigс любой стороны ребра → слитое summary (сегодняшнее поведение), никогда «мимо»;first_party/overloadedостаются на голом имени (INV4). Аддитивно, без бампаOWNIR_VERSION; solver не тронут (ключи для него непрозрачны). Extractor штампуетsigиз одногоIMethodSymbolна обеих сторонах ребра (generic-аргументы стёрты до backtick-арности — коллизия просто сливает, никогда не мис-резолвит). Попутно починен скрытый недетерминизм паритетного дампа (имя параметра слитой перегрузки зависело от порядка входа).Тип изменения
Как проверено
python tests/run_tests.py— 270/270 bridge-проверок (14 новых по матрице ТЗ §6.1: раздельные контракты, fallback без/с несовпавшимsig,global::-qualifiedsig, смешанные продюсеры, fresh-регресс fallback'а, дубликат name+sig, байт-детерминизм sig-дампа)ruff check .иmypyfrontend/roslyn/samples/OverloadSigSample.cs+ ассерты вwpf-extractorjob (sig на обеих сторонах ребра в фактах; OWN001 вDrop) — extractor-половина проверяется CI (локально dotnet нет)Связанные issue
Closes #216. Refs: roadmap
docs/notes/interprocedural-roadmap.md§5, ТЗdocs/notes/interprocedural-tz.md§6.1.Чеклист
feat:,fix:,docs:…)🤖 Generated with Claude Code
https://claude.ai/code/session_01LsWw4Ay8KLTHFom1HvRu3U
Generated by Claude Code
Summary by CodeRabbit
sig) support to interprocedural stage 2 resolution, enabling contracts and call outcomes to match the correct overload.sigthrough flow calls and tracked factory/return/forwarding behavior for more precise ownership conclusions.sigformat, fallback rules, and validation behavior.