Skip to content

Commit 896aa91

Browse files
authored
Merge pull request #844 from DeusData/distill/828-usage-ownership
fix(graph): stable USAGE-edge ownership for directory-module nodes
2 parents 04e71b3 + f6b9ac9 commit 896aa91

8 files changed

Lines changed: 593 additions & 22 deletions

File tree

Makefile.cbm

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -462,6 +462,7 @@ TEST_REPRO_SRCS = \
462462
tests/repro/repro_new_cypher_limit_zero.c \
463463
tests/repro/repro_issue363.c \
464464
tests/repro/repro_issue581.c \
465+
tests/repro/repro_issue787.c \
465466
tests/repro/repro_invariant_calls.c \
466467
tests/repro/repro_invariant_graph.c \
467468
tests/repro/repro_invariant_breadth.c \

src/graph_buffer/graph_buffer.c

Lines changed: 42 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -635,25 +635,30 @@ int64_t cbm_gbuf_upsert_node(cbm_gbuf_t *gb, const char *label, const char *name
635635
/* Check if node already exists */
636636
cbm_gbuf_node_t *existing = cbm_ht_get(gb->node_by_qn, qualified_name);
637637
if (existing) {
638-
/* Update in-place. name/properties are strdup'd BEFORE freeing old ones
639-
* (callers may pass existing->name as an argument). label/file_path are
640-
* interned: gb_intern returns a stable pool pointer (idempotent even when
641-
* label == existing->label), so the old value is replaced, never freed. */
642-
char *new_name = heap_strdup(name);
643-
char *new_props = properties_json ? heap_strdup(properties_json) : NULL;
644-
/* Don't let a per-file "Module" def downgrade a structural directory node
638+
/* Don't let a per-file "Module" def touch a structural directory node
645639
* ("Project" root or "Folder"). In a directory-based-module language
646640
* (Go/Java) a file's module_qn equals its directory QN: a root file →
647641
* the project name (== the "Project" node's QN); a file in pkg/ →
648642
* proj.pkg (== the "pkg/" Folder node's QN). Its always-emitted Module
649643
* def collides here; the directory node is the package/module container
650-
* and must keep its structural label. (Both the sequential upsert and the
651-
* parallel local-gbuf merge route through this function.) */
652-
if (!(existing->label && label && strcmp(label, "Module") == 0 &&
653-
(strcmp(existing->label, "Project") == 0 ||
654-
strcmp(existing->label, "Folder") == 0))) {
655-
existing->label = (char *)gb_intern(gb, label);
644+
* and must keep its structural label AND its own name/file_path/range.
645+
* Updating those in place set the shared node's file_path to whichever
646+
* same-package file happened to be processed LAST (worker-order
647+
* dependent) — the nondeterministic file attribution behind #787 — and
648+
* left the Folder node exposed to delete-nodes-by-file on incremental
649+
* reindex of that file. Skip the update entirely. (Both the sequential
650+
* upsert and the parallel local-gbuf merge route through this function.) */
651+
if (existing->label && label && strcmp(label, "Module") == 0 &&
652+
(strcmp(existing->label, "Project") == 0 || strcmp(existing->label, "Folder") == 0)) {
653+
return existing->id;
656654
}
655+
/* Update in-place. name/properties are strdup'd BEFORE freeing old ones
656+
* (callers may pass existing->name as an argument). label/file_path are
657+
* interned: gb_intern returns a stable pool pointer (idempotent even when
658+
* label == existing->label), so the old value is replaced, never freed. */
659+
char *new_name = heap_strdup(name);
660+
char *new_props = properties_json ? heap_strdup(properties_json) : NULL;
661+
existing->label = (char *)gb_intern(gb, label);
657662
free(existing->name);
658663
existing->name = new_name;
659664
existing->file_path = (char *)gb_intern(gb, file_path);
@@ -1112,15 +1117,30 @@ static void free_remap_entry(const char *key, void *val, void *ud) {
11121117
* label/file_path are re-interned into dst's pool (sn's pointers belong to src). */
11131118
static void merge_update_existing(cbm_gbuf_t *dst, cbm_gbuf_node_t *existing,
11141119
const cbm_gbuf_node_t *sn, CBMHashTable **remap) {
1115-
existing->label = (char *)gb_intern(dst, sn->label);
1116-
free(existing->name);
1117-
existing->name = heap_strdup(sn->name);
1118-
existing->file_path = (char *)gb_intern(dst, sn->file_path);
1119-
existing->start_line = sn->start_line;
1120-
existing->end_line = sn->end_line;
1121-
if (sn->properties_json) {
1122-
free(existing->properties_json);
1123-
existing->properties_json = heap_strdup(sn->properties_json);
1120+
/* Same guard as cbm_gbuf_upsert_node: a per-file "Module" def coming from a
1121+
* worker-local gbuf must not touch the structural directory node ("Project"
1122+
* root or "Folder") that shares its QN in a directory-based-module language
1123+
* (Java/Go). pass_structure seeds Folder/Project nodes on the MAIN gbuf
1124+
* before the parallel extract, so every worker's always-emitted Module def
1125+
* for that package collides here; unconditional "src wins" relabelled the
1126+
* directory node to Module and set its file_path to whichever worker merged
1127+
* LAST — the nondeterministic USAGE-source misattribution of #787. Keep the
1128+
* structural node intact; the ID remap below still redirects the worker's
1129+
* edges onto the canonical node. */
1130+
bool module_on_container =
1131+
existing->label && sn->label && strcmp(sn->label, "Module") == 0 &&
1132+
(strcmp(existing->label, "Project") == 0 || strcmp(existing->label, "Folder") == 0);
1133+
if (!module_on_container) {
1134+
existing->label = (char *)gb_intern(dst, sn->label);
1135+
free(existing->name);
1136+
existing->name = heap_strdup(sn->name);
1137+
existing->file_path = (char *)gb_intern(dst, sn->file_path);
1138+
existing->start_line = sn->start_line;
1139+
existing->end_line = sn->end_line;
1140+
if (sn->properties_json) {
1141+
free(existing->properties_json);
1142+
existing->properties_json = heap_strdup(sn->properties_json);
1143+
}
11241144
}
11251145

11261146
if (sn->id != existing->id) {

src/pipeline/pass_calls.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,12 @@ static const cbm_gbuf_node_t *calls_find_source(cbm_pipeline_ctx_t *ctx, const c
409409
const cbm_gbuf_node_t *src = NULL;
410410
if (enclosing_qn) {
411411
src = cbm_gbuf_find_by_qn(ctx->gbuf, enclosing_qn);
412+
/* A class-level call in a directory-module language carries the
413+
* DIRECTORY module QN, which hits the shared Folder/Project node —
414+
* attribute to this file's File node instead (#787). */
415+
if (cbm_pipeline_node_is_dir_container(src)) {
416+
src = NULL;
417+
}
412418
}
413419
if (!src) {
414420
char *fqn = cbm_pipeline_fqn_compute(ctx->project_name, rel, "__file__");

src/pipeline/pass_parallel.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1828,6 +1828,12 @@ static const cbm_gbuf_node_t *find_source_node(const cbm_gbuf_t *gbuf, const cha
18281828
const cbm_gbuf_node_t *src = NULL;
18291829
if (enclosing_qn) {
18301830
src = cbm_gbuf_find_by_qn(gbuf, enclosing_qn);
1831+
/* A class-level reference in a directory-module language carries the
1832+
* DIRECTORY module QN, which hits the shared Folder/Project node —
1833+
* attribute to this file's File node instead (#787). */
1834+
if (cbm_pipeline_node_is_dir_container(src)) {
1835+
src = NULL;
1836+
}
18311837
}
18321838
if (!src) {
18331839
char *file_qn = cbm_pipeline_fqn_compute(project, rel, "__file__");

src/pipeline/pass_usages.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,12 @@ static const cbm_gbuf_node_t *find_enclosing_node(cbm_pipeline_ctx_t *ctx, const
197197
const cbm_gbuf_node_t *node = NULL;
198198
if (func_qn && func_qn[0]) {
199199
node = cbm_gbuf_find_by_qn(ctx->gbuf, func_qn);
200+
/* A class-level reference in a directory-module language carries the
201+
* DIRECTORY module QN, which hits the shared Folder/Project node —
202+
* attribute to this file's File node instead (#787). */
203+
if (cbm_pipeline_node_is_dir_container(node)) {
204+
node = NULL;
205+
}
200206
}
201207
if (!node) {
202208
char *file_qn = cbm_pipeline_fqn_compute(ctx->project_name, rel_path, "__file__");

src/pipeline/pipeline_internal.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,19 @@
3434
* out_sz >= strlen(in) + 1 always suffices. Returns out. */
3535
const char *cbm_route_canon_path(const char *in, char *out, size_t out_sz);
3636

37+
/* True when a graph node is a structural directory container (Folder/Project)
38+
* rather than a code node. In a directory-based-module language (Java/Go, see
39+
* cbm_lang_module_is_dir) a file's module QN equals its directory QN, so an
40+
* enclosing-scope lookup for a CLASS-LEVEL usage/call (enclosing_func_qn ==
41+
* module_qn) resolves to the ONE Folder/Project node shared by every file in
42+
* that package. Sourcing an edge there conflates all same-package files into a
43+
* single source node with an arbitrary file_path (#787). Source-node finders
44+
* must treat such a hit as a miss and fall back to the per-file File node. */
45+
static inline bool cbm_pipeline_node_is_dir_container(const cbm_gbuf_node_t *node) {
46+
return node && node->label &&
47+
(strcmp(node->label, "Folder") == 0 || strcmp(node->label, "Project") == 0);
48+
}
49+
3750
/* Time unit conversions */
3851
#define CBM_NS_PER_SEC 1000000000LL
3952
#define CBM_US_PER_SEC 1000000LL

0 commit comments

Comments
 (0)