Skip to content

Commit 538ac25

Browse files
committed
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 5df8b04 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>
1 parent 13ce2c0 commit 538ac25

3 files changed

Lines changed: 98 additions & 11 deletions

File tree

‎internal/cbm/extract_defs.c‎

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2041,12 +2041,6 @@ static const char **extract_decorators(CBMArena *a, TSNode node, const char *sou
20412041
return result;
20422042
}
20432043

2044-
/* Rust: two same-named functions guarded by mutually-exclusive #[cfg(...)]
2045-
* attributes both parse as distinct function_item nodes and otherwise receive
2046-
* the SAME qualified_name, so the second graph upsert silently overwrites the
2047-
* first and one branch is lost (#495). Fold the cfg predicate into the QN so
2048-
* each cfg-gated twin gets a DISTINCT, predicate-encoding QN. Returns the
2049-
* (possibly suffixed) QN; the original QN when no cfg attribute is present. */
20502044
/* Rust: mark a function as a test when it carries a test attribute (#855).
20512045
* cbm's test detection is otherwise file-path-based (cbm_is_test_file:
20522046
* *_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) {
20762070
return false;
20772071
}
20782072

2073+
/* Rust: two same-named functions guarded by mutually-exclusive #[cfg(...)]
2074+
* attributes both parse as distinct function_item nodes and otherwise receive
2075+
* the SAME qualified_name, so the second graph upsert silently overwrites the
2076+
* first and one branch is lost (#495). Fold the cfg predicate into the QN so
2077+
* each cfg-gated twin gets a DISTINCT, predicate-encoding QN. Returns the
2078+
* (possibly suffixed) QN; the original QN when no cfg attribute is present. */
2079+
enum { ATTR_OPEN_CHARS = 2 }; /* the "#[" a bracketed attribute opens with */
2080+
20792081
static const char *rust_cfg_qualified_name(CBMArena *a, const char *base_qn,
20802082
const char *const *decorators) {
20812083
if (!decorators) {
20822084
return base_qn;
20832085
}
20842086
for (int i = 0; decorators[i]; i++) {
2085-
const char *cfg = strstr(decorators[i], "cfg(");
2087+
/* The attribute must BE `cfg`, not merely contain that text. `cfg_attr`
2088+
* carries its own nested `cfg(...)`, and in the common
2089+
* `#[cfg_attr(docsrs, doc(cfg(feature = "x")))]` that inner predicate
2090+
* gates DOCUMENTATION, not compilation: the function is compiled
2091+
* unconditionally and has no twin to be told apart from. Anchor on the
2092+
* bracketed path, which is the convention rust_def_is_test above already
2093+
* follows for the same reason. */
2094+
const char *cfg = strstr(decorators[i], "#[cfg(");
20862095
if (!cfg) {
20872096
continue;
20882097
}
2089-
/* Build a compact predicate suffix from the cfg(...) text, dropping
2090-
* whitespace and quotes so the QN stays readable and stable. */
2098+
cfg += ATTR_OPEN_CHARS;
2099+
/* Copy the BALANCED cfg(...) span, dropping whitespace and quotes so the
2100+
* QN stays readable and stable. Running to the end of the decorator text
2101+
* instead cost two things. It appended the attribute's own ']' to every
2102+
* QN. And on a multi-line attribute the copied text carried the newline,
2103+
* which cbm_defs_push cuts the QN at by design, leaving the suffix
2104+
* truncated mid-predicate: two twins whose predicates differ only past
2105+
* that newline then computed ONE qualified name and the second upsert
2106+
* overwrote the first, which is the loss this function exists to
2107+
* prevent (#495).
2108+
*
2109+
* A predicate longer than the buffer still truncates, as before. It
2110+
* stays distinct for the first 255 characters, which is what keeps the
2111+
* twins apart. */
20912112
char buf[CBM_SZ_256];
20922113
size_t bi = 0;
2114+
int depth = 0;
20932115
for (const char *p = cfg; *p && bi + 1 < sizeof(buf); p++) {
2094-
if (*p == ' ' || *p == '\t' || *p == '"' || *p == '\'') {
2116+
if (*p == ' ' || *p == '\t' || *p == '\n' || *p == '\r' || *p == '"' || *p == '\'') {
20952117
continue;
20962118
}
20972119
buf[bi++] = *p;
2120+
if (*p == '(') {
2121+
depth++;
2122+
} else if (*p == ')' && --depth == 0) {
2123+
break;
2124+
}
20982125
}
20992126
buf[bi] = '\0';
21002127
return cbm_arena_sprintf(a, "%s#%s", base_qn, buf);

‎src/store/store.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ typedef struct cbm_store cbm_store_t;
2323
#define CBM_STORE_OK 0
2424
#define CBM_STORE_ERR (-1)
2525
#define CBM_STORE_NOT_FOUND (-2)
26-
#define CBM_INDEX_FORMAT_VERSION 1
26+
#define CBM_INDEX_FORMAT_VERSION 2
2727
#define CBM_STORE_CANCELLED (-3)
2828
#define CBM_STORE_SCAN_LIMIT (-4)
2929
#define CBM_STORE_CALLBACK_ERR (-5)

‎tests/test_extraction.c‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -945,6 +945,63 @@ TEST(rust_function) {
945945
PASS();
946946
}
947947

948+
/* The cfg fence exists so two mutually-exclusive #[cfg(...)] twins get distinct
949+
* qualified names instead of one overwriting the other (#495). Three ways it
950+
* misread the attribute, all visible in the QN it mints. */
951+
TEST(rust_cfg_fence_is_the_balanced_predicate_only) {
952+
CBMFileResult *r = extract("#[cfg(test)]\n"
953+
"pub fn gated() {}\n"
954+
"\n"
955+
"#[cfg(target_os = \"linux\")]\n"
956+
"pub fn gated_os() {}\n",
957+
CBM_LANG_RUST, "t", "src/lib.rs");
958+
ASSERT_NOT_NULL(r);
959+
ASSERT_FALSE(r->has_error);
960+
/* The attribute's own closing bracket is not part of the predicate. */
961+
ASSERT_TRUE(has_def_qn(r, "t.src.lib.gated#cfg(test)"));
962+
ASSERT_TRUE(has_def_qn(r, "t.src.lib.gated_os#cfg(target_os=linux)"));
963+
cbm_free_result(r);
964+
PASS();
965+
}
966+
967+
/* `#[cfg_attr(docsrs, doc(cfg(...)))]` gates documentation, not compilation, so
968+
* the function is compiled unconditionally and has no twin. An unanchored search
969+
* for "cfg(" found the nested predicate and fenced it anyway. */
970+
TEST(rust_cfg_attr_doc_gate_mints_no_fence) {
971+
CBMFileResult *r = extract("#[cfg_attr(docsrs, doc(cfg(feature = \"rt\")))]\n"
972+
"pub async fn yield_now() {}\n",
973+
CBM_LANG_RUST, "t", "src/lib.rs");
974+
ASSERT_NOT_NULL(r);
975+
ASSERT_FALSE(r->has_error);
976+
ASSERT_TRUE(has_def_qn(r, "t.src.lib.yield_now"));
977+
cbm_free_result(r);
978+
PASS();
979+
}
980+
981+
/* A multi-line attribute carried its newline into the QN, and cbm_defs_push cuts
982+
* a qualified name at a line break by design, so both twins were left with the
983+
* same truncated `#cfg(all(` and the second upsert overwrote the first. */
984+
TEST(rust_multiline_cfg_twins_stay_distinct) {
985+
CBMFileResult *r = extract("#[cfg(all(\n"
986+
" unix,\n"
987+
" feature = \"alpha\"\n"
988+
"))]\n"
989+
"pub fn twin() -> u32 { 1 }\n"
990+
"\n"
991+
"#[cfg(all(\n"
992+
" unix,\n"
993+
" feature = \"beta\"\n"
994+
"))]\n"
995+
"pub fn twin() -> u32 { 2 }\n",
996+
CBM_LANG_RUST, "t", "src/lib.rs");
997+
ASSERT_NOT_NULL(r);
998+
ASSERT_FALSE(r->has_error);
999+
ASSERT_TRUE(has_def_qn(r, "t.src.lib.twin#cfg(all(unix,feature=alpha))"));
1000+
ASSERT_TRUE(has_def_qn(r, "t.src.lib.twin#cfg(all(unix,feature=beta))"));
1001+
cbm_free_result(r);
1002+
PASS();
1003+
}
1004+
9481005
TEST(rust_struct) {
9491006
CBMFileResult *r = extract("pub struct Point { pub x: f64, pub y: f64 }\nimpl Point { pub fn "
9501007
"new(x: f64, y: f64) -> Self { Point { x, y } } }\n",
@@ -8701,6 +8758,9 @@ SUITE(extraction) {
87018758

87028759
/* Systems */
87038760
RUN_TEST(rust_function);
8761+
RUN_TEST(rust_cfg_fence_is_the_balanced_predicate_only);
8762+
RUN_TEST(rust_cfg_attr_doc_gate_mints_no_fence);
8763+
RUN_TEST(rust_multiline_cfg_twins_stay_distinct);
87048764
RUN_TEST(rust_struct);
87058765
RUN_TEST(go_function);
87068766
RUN_TEST(go_struct);

0 commit comments

Comments
 (0)