Uh oh!
There was an error while loading. Please reload this page.
fix(pycg): canonicalize the builtins module spelling on PyCG edges - #134
Merged
Conversation
) PyCG spells the builtins module `<builtin>`; Jedi spells it `builtins`. Nothing normalized the two, so `_home_external_symbols` minted a separate `can://.../@external/<module>/<name>` home per spelling and one builtin ended up with two identities. Measured on the `requests` fixture at -a 2: 29 ids under `<builtin>/` against 14 under `builtins/`, with 12 names present under both (len, isinstance, getattr, sorted, ...). Two things follow from that. A consumer asking "who calls len" gets two disjoint answers, neither complete. And provenance can never merge for a builtin: `merge_edges` coalesces on (src, dst), so differing dst ids keep the two backends' edges apart -- in the same run 198 non-builtin edges do carry `prov: ["jedi", "pycg"]`, while builtins are structurally excluded from it. Canonicalization happens at `build_call_graph_edges`' single exit, so every shard strategy is covered, and before `merge_edges` runs in core.py -- doing it at id-minting time would leave two already-merged edges with identical endpoints and split provenance, moving the symptom rather than removing it. Endpoints that collide once rewritten are coalesced with summed weight and unioned provenance, matching merge_edges' semantics. That coalescing is done locally rather than through `_coalesce_edges`, which raises on its duplicate branch (#133). PyCG only ever emits the bare `<builtin>` module, so an exact-match alias suffices; the dotted forms (`builtins.str`, `builtins.dict`) are Jedi's and are already canonical.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Closes#132.
Problem
PyCG spells the builtins module
<builtin>. Jedi spells itbuiltins. Same builtin, twocan://ids.requestsfixture at-a 2, before the fix:@external/<builtin>/— allprov=['pycg']@external/builtins/— allprov=['jedi']len,isinstance,getattr,sorted,all,any,chr,hasattr,iter,max,print,setattrTwo consequences.
First. Asking "who calls
len" returns two disjoint answers. Neither is complete.Second.
merge_edgescoalesces on(src, dst). Differentdstmeans the two backends never meet, so a builtin can never reachprov: ["jedi", "pycg"]. In the same run, 198 non-builtin edges do carry both — the merge works. Builtins are structurally locked out.Root cause:
core.py:_home_external_symbolssplits the signature on its last dot, mints@external/<module>/<name>, and never canonicalizesmodule.Fix
Rewrite
<builtin>.xtobuiltins.xat one place — thePyCG.build_call_graph_edgesexit. Every shard strategy returns through it.Placement matters:
merge_edges(core.py:598), not at id-minting (core.py:633). A mint-time fix leaves two already-merged edges with identical endpoints and split provenance — the symptom moves rather than disappears.merge_edges._coalesce_edges. That one raises on its duplicate branch (_coalesce_edges raises on any duplicate edge: constructs PyCallEdge with source=/target= instead of src=/dst= #133), and this fix is what makes the branch reachable.PyCG only ever emits the bare
<builtin>module (69 occurrences acrossrequestsandflask). The dotted forms (builtins.str,builtins.dict) come from Jedi and are already canonical, so an exact-match alias suffices — no prefix rewriting.Verification
Both fixtures re-analyzed at
-a 2on this branch:<builtin>ids left<builtin>edges leftbuiltins/namesprov: ["jedi","pycg"]31 = 29 + 14 − 12. The merge is exact: no id lost, none invented.
Tests
test/test_pycg_builtin_canonicalization.py, five tests:<builtin>.xis rewrittenbuiltins.str.format, first-party signatures, bare names, bare<builtin>)Caveats
Suite is green on a quiet machine:
198 passed, 2 skipped, 14 deselected in 6:30— the 193 frommainplus the 5 added here. An earlier run hung while a 3.5-hourxarrayanalysis held the machine; that was contention, not this diff. The suite is load-sensitive (PyCG shard timeouts under load produce spurious L2 failures), so it needs a quiet machine to mean anything.Edge weights change where two spellings collapse into one. That is the correct new value, not a regression.