front: resolve an ambiguous name path to None in GraphBodyProvider - #839
Conversation
source_for_name_path returned the first FunDecl whose name_path matched. Two extracted FunDecls can render the same name path, in which case the first match binds a body from a different funcobj. Scan the whole corpus and return None when more than one FunDecl carries the name. bookkeeper.py:361 getdesc keys a descriptor by the function object; a name path carries no such identity, and GraphBodySource is what the demand path resolves against. provider_reproduces_the_eagerly_lowered_body now also fails on a duplicate name path in the corpus fixture, as a lookup miss. Assisted-by: Claude
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Walkthrough
ChangesGraph body lookup
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
🤖 Codex parity reviewStatic analysis of this diff vs the local RPython/PyPy sources (commit 86ddf6e). Files in the reviewed diff1. Regressions to PyPy parity introduced by this patchNone. 2. Other mismatches introduced by this patchNone. 3. Pre-existing mismatches (already present before this patch)
4. Structural adaptations
|
Follow-up to #835, from its Codex parity review (section 2).
GraphBodyProvider::source_for_name_pathreturned the firstFunDeclwhosename_path()matched. Two extractedFunDecls can render the same name path,and in that case the first match binds a body that belongs to a different
funcobj — silently, because the caller sees a perfectly ordinary hit.
bookkeeper.py:361 getdesc(pyobj)keys a descriptor by the function objectitself, so upstream never conflates two functions that happen to share a name.
A Charon name path carries no such identity.
GraphBodySource— the(llbc_index, def_id)pair recorded at registration — is what does, and it isalready what the demand path resolves against; the name lookup is only the
registration-time/test seam.
So the helper now scans the whole corpus and returns
Nonewhen more than oneFunDeclcarries the name. A miss is visible where a wrong body is not.provider_reproduces_the_eagerly_lowered_bodygains a second job for free: itlooks every lowerable funcobj up by name, so a duplicate name path anywhere in
the corpus fixture now fails the test as a lookup miss.
No production behaviour changes — the provider still has no production call
site, as noted in #835.
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests