From 538ac25edda676ca7d8cb8a4c7c5fb3a3a36fda2 Mon Sep 17 00:00:00 2001 From: BobbieBarker <4307099+BobbieBarker@users.noreply.github.com> Date: Sun, 27 Sep 2026 16:40:47 -0700 Subject: [PATCH] fix(rust): fence a QN on the function's own cfg predicate, balanced rust_cfg_qualified_name folds a function's `#[cfg(...)]` predicate into its qualified name so two mutually-exclusive twins get distinct QNs instead of one overwriting the other (#495). It read the attribute three ways that the QN it minted shows are wrong. The suffix was copied from the `cfg(` it found to the END of the decorator text. extract_decorators stores the bracketed form, so that appended the attribute's own `]` to every QN: `#[cfg(test)]` minted `add#cfg(test)]`. Across ripgrep 14.1.1, tokio 1.40.0 and rust-analyzer 2024-09-30 that is 332 of 335 fenced QNs. On a multi-line attribute the copied text also carried the newline, and cbm_defs_push cuts a qualified name at a line break by design, pinned by defs_push_cuts_multiline_names_and_rejects_js_literal_names. What survived was everything before the newline, so the suffix stopped mid-predicate: `thread_rng_n#cfg(any(`. Two twins whose predicates differ only past that newline then computed ONE qualified name and the second upsert overwrote the first, which is the loss this function exists to prevent. 10 tokio QNs are truncated this way. None of them happens to collide, because the visible prefixes stay distinct there; the collision is in the test below, which drops from two nodes to one without this commit. `strstr(decorators[i], "cfg(")` was also unanchored, and `cfg_attr` carries its own nested `cfg(`. In `#[cfg_attr(docsrs, doc(cfg(feature = "rt")))]` that inner predicate gates DOCUMENTATION, not compilation: the function is compiled unconditionally and has no twin to be told apart from, and it was fenced anyway as `yield_now#cfg(feature=rt)))]`. 7 tokio functions are fenced solely by a cfg_attr. rust_def_is_test, twenty lines above, already anchors on the bracketed path and says why; this was the one Rust decorator inspector that did not. The fix anchors on `#[cfg(` and copies the balanced span, stopping at the paren that closes it and dropping newlines with the other whitespace. A predicate longer than the 256-byte buffer still truncates, as before, and stays distinct over its first 255 characters. Measured by indexing the three corpora above with a binary built from 5df8b044 and with this one: corpus QNs changed stop being fenced nodes added or lost ripgrep 131 0 0 tokio 176 7 0 rust-analyzer 28 0 0 The 7 are the cfg_attr-only functions, which now carry their plain QN. Node counts are identical in all three, and tokio's multi-line predicates are reconstructed in full: `main#cfg(all(tokio_unstable,tokio_taskdump,target_os=linux,any(target_arch=aarch64,target_arch=x86,target_arch=x86_64)))`. This changes node identity for cfg-fenced Rust functions, so CBM_INDEX_FORMAT_VERSION goes 1 -> 2 and the next index rebuilds from scratch. The deciding case is the incremental path rather than the mixed store: incr_capture_inbound_edge snapshots an inbound cross-file edge by its target's qualified_name STRING, and the re-link after the purge resolves that string again. A caller in an unchanged file would therefore lose its edge into a re-extracted fenced function, because the QN it was snapshotted under no longer exists, and nothing would report the loss. The #495 comment sat above rust_def_is_test while describing this function; it moves down with the code it documents. tests/test_extraction.c: three tests, one per misreading, and the only coverage this function has in the gating suite. tests/repro/repro_issue495.c exercises it today but Makefile.cbm keeps that file out of `make test`, and it asserts only that two QNs differ and that one contains `not(`, never the string minted. Each of the three fails without this commit. Signed-off-by: Chad <4307099+BobbieBarker@users.noreply.github.com> --- internal/cbm/extract_defs.c | 47 ++++++++++++++++++++++------- src/store/store.h | 2 +- tests/test_extraction.c | 60 +++++++++++++++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 11 deletions(-) diff --git a/internal/cbm/extract_defs.c b/internal/cbm/extract_defs.c index 49543e59a5..5d5e9847df 100644 --- a/internal/cbm/extract_defs.c +++ b/internal/cbm/extract_defs.c @@ -2041,12 +2041,6 @@ static const char **extract_decorators(CBMArena *a, TSNode node, const char *sou return result; } -/* Rust: two same-named functions guarded by mutually-exclusive #[cfg(...)] - * attributes both parse as distinct function_item nodes and otherwise receive - * the SAME qualified_name, so the second graph upsert silently overwrites the - * first and one branch is lost (#495). Fold the cfg predicate into the QN so - * each cfg-gated twin gets a DISTINCT, predicate-encoding QN. Returns the - * (possibly suffixed) QN; the original QN when no cfg attribute is present. */ /* Rust: mark a function as a test when it carries a test attribute (#855). * cbm's test detection is otherwise file-path-based (cbm_is_test_file: * *_test.rs / test_*), so inline #[test]/#[tokio::test] functions inside a @@ -2076,25 +2070,58 @@ static bool rust_def_is_test(const char *const *decorators) { return false; } +/* Rust: two same-named functions guarded by mutually-exclusive #[cfg(...)] + * attributes both parse as distinct function_item nodes and otherwise receive + * the SAME qualified_name, so the second graph upsert silently overwrites the + * first and one branch is lost (#495). Fold the cfg predicate into the QN so + * each cfg-gated twin gets a DISTINCT, predicate-encoding QN. Returns the + * (possibly suffixed) QN; the original QN when no cfg attribute is present. */ +enum { ATTR_OPEN_CHARS = 2 }; /* the "#[" a bracketed attribute opens with */ + static const char *rust_cfg_qualified_name(CBMArena *a, const char *base_qn, const char *const *decorators) { if (!decorators) { return base_qn; } for (int i = 0; decorators[i]; i++) { - const char *cfg = strstr(decorators[i], "cfg("); + /* The attribute must BE `cfg`, not merely contain that text. `cfg_attr` + * carries its own nested `cfg(...)`, and in the common + * `#[cfg_attr(docsrs, doc(cfg(feature = "x")))]` that inner predicate + * gates DOCUMENTATION, not compilation: the function is compiled + * unconditionally and has no twin to be told apart from. Anchor on the + * bracketed path, which is the convention rust_def_is_test above already + * follows for the same reason. */ + const char *cfg = strstr(decorators[i], "#[cfg("); if (!cfg) { continue; } - /* Build a compact predicate suffix from the cfg(...) text, dropping - * whitespace and quotes so the QN stays readable and stable. */ + cfg += ATTR_OPEN_CHARS; + /* Copy the BALANCED cfg(...) span, dropping whitespace and quotes so the + * QN stays readable and stable. Running to the end of the decorator text + * instead cost two things. It appended the attribute's own ']' to every + * QN. And on a multi-line attribute the copied text carried the newline, + * which cbm_defs_push cuts the QN at by design, leaving the suffix + * truncated mid-predicate: two twins whose predicates differ only past + * that newline then computed ONE qualified name and the second upsert + * overwrote the first, which is the loss this function exists to + * prevent (#495). + * + * A predicate longer than the buffer still truncates, as before. It + * stays distinct for the first 255 characters, which is what keeps the + * twins apart. */ char buf[CBM_SZ_256]; size_t bi = 0; + int depth = 0; for (const char *p = cfg; *p && bi + 1 < sizeof(buf); p++) { - if (*p == ' ' || *p == '\t' || *p == '"' || *p == '\'') { + if (*p == ' ' || *p == '\t' || *p == '\n' || *p == '\r' || *p == '"' || *p == '\'') { continue; } buf[bi++] = *p; + if (*p == '(') { + depth++; + } else if (*p == ')' && --depth == 0) { + break; + } } buf[bi] = '\0'; return cbm_arena_sprintf(a, "%s#%s", base_qn, buf); diff --git a/src/store/store.h b/src/store/store.h index b6ce844f75..e79f00a5ce 100644 --- a/src/store/store.h +++ b/src/store/store.h @@ -23,7 +23,7 @@ typedef struct cbm_store cbm_store_t; #define CBM_STORE_OK 0 #define CBM_STORE_ERR (-1) #define CBM_STORE_NOT_FOUND (-2) -#define CBM_INDEX_FORMAT_VERSION 1 +#define CBM_INDEX_FORMAT_VERSION 2 #define CBM_STORE_CANCELLED (-3) #define CBM_STORE_SCAN_LIMIT (-4) #define CBM_STORE_CALLBACK_ERR (-5) diff --git a/tests/test_extraction.c b/tests/test_extraction.c index acf5b2319c..5a7bca275d 100644 --- a/tests/test_extraction.c +++ b/tests/test_extraction.c @@ -945,6 +945,63 @@ TEST(rust_function) { PASS(); } +/* The cfg fence exists so two mutually-exclusive #[cfg(...)] twins get distinct + * qualified names instead of one overwriting the other (#495). Three ways it + * misread the attribute, all visible in the QN it mints. */ +TEST(rust_cfg_fence_is_the_balanced_predicate_only) { + CBMFileResult *r = extract("#[cfg(test)]\n" + "pub fn gated() {}\n" + "\n" + "#[cfg(target_os = \"linux\")]\n" + "pub fn gated_os() {}\n", + CBM_LANG_RUST, "t", "src/lib.rs"); + ASSERT_NOT_NULL(r); + ASSERT_FALSE(r->has_error); + /* The attribute's own closing bracket is not part of the predicate. */ + ASSERT_TRUE(has_def_qn(r, "t.src.lib.gated#cfg(test)")); + ASSERT_TRUE(has_def_qn(r, "t.src.lib.gated_os#cfg(target_os=linux)")); + cbm_free_result(r); + PASS(); +} + +/* `#[cfg_attr(docsrs, doc(cfg(...)))]` gates documentation, not compilation, so + * the function is compiled unconditionally and has no twin. An unanchored search + * for "cfg(" found the nested predicate and fenced it anyway. */ +TEST(rust_cfg_attr_doc_gate_mints_no_fence) { + CBMFileResult *r = extract("#[cfg_attr(docsrs, doc(cfg(feature = \"rt\")))]\n" + "pub async fn yield_now() {}\n", + CBM_LANG_RUST, "t", "src/lib.rs"); + ASSERT_NOT_NULL(r); + ASSERT_FALSE(r->has_error); + ASSERT_TRUE(has_def_qn(r, "t.src.lib.yield_now")); + cbm_free_result(r); + PASS(); +} + +/* A multi-line attribute carried its newline into the QN, and cbm_defs_push cuts + * a qualified name at a line break by design, so both twins were left with the + * same truncated `#cfg(all(` and the second upsert overwrote the first. */ +TEST(rust_multiline_cfg_twins_stay_distinct) { + CBMFileResult *r = extract("#[cfg(all(\n" + " unix,\n" + " feature = \"alpha\"\n" + "))]\n" + "pub fn twin() -> u32 { 1 }\n" + "\n" + "#[cfg(all(\n" + " unix,\n" + " feature = \"beta\"\n" + "))]\n" + "pub fn twin() -> u32 { 2 }\n", + CBM_LANG_RUST, "t", "src/lib.rs"); + ASSERT_NOT_NULL(r); + ASSERT_FALSE(r->has_error); + ASSERT_TRUE(has_def_qn(r, "t.src.lib.twin#cfg(all(unix,feature=alpha))")); + ASSERT_TRUE(has_def_qn(r, "t.src.lib.twin#cfg(all(unix,feature=beta))")); + cbm_free_result(r); + PASS(); +} + TEST(rust_struct) { CBMFileResult *r = extract("pub struct Point { pub x: f64, pub y: f64 }\nimpl Point { pub fn " "new(x: f64, y: f64) -> Self { Point { x, y } } }\n", @@ -8701,6 +8758,9 @@ SUITE(extraction) { /* Systems */ RUN_TEST(rust_function); + RUN_TEST(rust_cfg_fence_is_the_balanced_predicate_only); + RUN_TEST(rust_cfg_attr_doc_gate_mints_no_fence); + RUN_TEST(rust_multiline_cfg_twins_stay_distinct); RUN_TEST(rust_struct); RUN_TEST(go_function); RUN_TEST(go_struct);