fix(graph): key IMPORTS edge dedup on local_name, not just source/target#770
Open
apappas1129 wants to merge 2 commits into
Open
fix(graph): key IMPORTS edge dedup on local_name, not just source/target#770apappas1129 wants to merge 2 commits into
apappas1129 wants to merge 2 commits into
Conversation
Two named imports from the same specifier resolve to the same (source_file, target_file) pair, so cbm_gbuf_insert_edge's dedup key of (source_id, target_id, type) collapsed them into one edge - the second import's properties_json silently replaced the first's, dropping whichever symbol lost the write race. This breaks more than "who imports X" queries: pass_calls.c, pass_usages.c, pass_semantic.c, and pass_lsp_cross.c all build their local_name -> resolved module QN maps for cross-file call resolution by parsing exactly one local_name out of each file's IMPORTS edges, so the dropped symbol's calls also failed to resolve cross-file. make_edge_key now folds local_name into the key for IMPORTS edges specifically, so two different symbols from the same specifier become two distinct edges while a re-inserted identical import (idempotent re-index) still dedups to one. Other edge types keep their original (source, target, type) key unchanged. Closes DeusData#768. Signed-off-by: Alexandros Pappas <11921291+apappas1129@users.noreply.github.com>
Signed-off-by: Alexandros Pappas <11921291+apappas1129@users.noreply.github.com>
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
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 does this PR do?
Closes #768: a single
import { A, B } from './lib'statement produced exactlyone
IMPORTSedge instead of two — whichever symbol got written secondsilently overwrote the first.
cbm_gbuf_insert_edge()(src/graph_buffer/graph_buffer.c) dedups edges on(source_id, target_id, type)only. Two named imports from the same specifierresolve to the same
(source_file, target_file)pair, so the secondIMPORTSedge collides with the first on that key. On collision the functiondoesn't merge or reject — it replaces
properties_jsonoutright (theexisting
/* Merge properties (just replace for now) */comment suggeststhis was a known stub).
This has a bigger blast radius than "who imports X" queries:
pass_calls.c,pass_usages.c,pass_semantic.c, andpass_lsp_cross.call build theirlocal_name -> resolved module QNmaps for cross-file call resolution byparsing exactly one
local_nameout of eachIMPORTSedge's properties. Sothe dropped symbol's calls also failed to resolve cross-file, not just the
import edge itself.
make_edge_key()now foldslocal_nameinto the dedup key specifically forIMPORTSedges, so two different symbols imported from the same specifierbecome two distinct edges. A re-inserted identical import (e.g. an idempotent
re-index) still dedups to one edge. No changes were needed in any of the four
downstream consumers — they already iterate edges and parse one
local_nameper edge; they simply see both edges now that dedup stops collapsing them.
Other edge types (
CALLS,HTTP_CALLS,DEFINES, ...) keep their original(source, target, type)key, unchanged.Checklist
git commit -s)make -f Makefile.cbm test) — full suite greenexcept one pre-existing, unrelated RSS-ceiling test in
test_incremental.c(~2.6GB vs 2048MB limit) on this machinemake -f Makefile.cbm lint-ci) — could not verifylocally, cppcheck/clang-format are not installed in this
environment; relying on CI
gbuf_imports_multi_symbol_dedupin
tests/test_graph_buffer.c, asserting two same-target imports withdifferent
local_nameproduce distinct edges while a repeatedidentical import still dedups to one