From 4903b78aeecb51fb285288255351d12d2a0a65fa Mon Sep 17 00:00:00 2001 From: Rik van Riel Date: Mon, 28 Sep 2026 20:21:02 -0700 Subject: [PATCH] treesitter: say which files gave up their swallowed directives Reading the macros an unreadable construct swallows is silent, in both directions. A file that gains 35 of them looks exactly like a file that gains none, and a file recovery refuses to read looks like a file with nothing to read -- which is the same silence that let the defect stand: one logging header held 41 directives and contributed one row, and nothing anywhere said so. Say it, once per file, and only where there is something to say. - A file whose swallowed definitions were read names them by count. - A file where the blanked parse no longer reads a definition the file's own parse does is refused whole, and says that it was: whatever else that parse found is not trustworthy, so the swallowed macros stay missing and the file is worth a look rather than a shrug. - A file where every definition the blanked parse found sits where the file writes a comment or a string says that too. Refusing to invent a macro is a different fact about a file than having none to recover, and a count of what was refused rides along on the line for a file that recovered something as well. Silence otherwise, because the population that enters recovery is an order of magnitude larger than the population it helps: on Linux 28a2bc7211da, 1,639 files are parsed a second time and 139 of them gain anything. Logging the attempt would bury the result. Counting what was refused needed one fix behind it. The rows a second reading finds were deduplicated to one per name before the file was asked which of them are comments, and the longer body wins that contest: a `#define` inside a comment can therefore take the name of a real swallowed definition below it, which both loses a macro the file defines and reports the loss as a refusal. Ask the file first, then deduplicate, and a refused name is exactly a name no row of the file declares. Test Plan: Three tests read what the analyzer says through a thread-local subscriber, so they hold the log as a contract rather than describing it: the recovered count appears with the file, a refusal appears with its count, and a file the parser reads whole says nothing at all. cargo test -- 294 tests, twenty-one in this module, none ignored. The phantom-takes-a-name test was run against a build that deduplicates before asking the file, and fails there. cargo clippy --all-targets -- -D warnings is clean. Volume over Linux 28a2bc7211da, reading every `*.c` and `*.h` with `SEMCODE_DEBUG=info` -- these go to stderr, and the default filter is `error`, so a plain run still shows none of them: 154 lines. 139 report what they recovered and their counts sum to 681, the tree-wide gain; 15 report a refusal, among them the AMD display header whose second reading drops 24 definitions the first one reads, `include/linux/bpf.h` and `drivers/char/random.c`. Not one file on this tree reports a refused definition, either alone or beside a gain: those two lines have fixtures and no instance here. That the recovery lines are invisible by default is worth its own decision later. A count of what a whole run recovered and refused belongs in the indexer's summary rather than behind a debug filter, and that needs the analyzer to return what it did instead of only saying it. --- src/treesitter_analyzer.rs | 181 ++++++++++++++++++++++++++++++++++--- 1 file changed, 170 insertions(+), 11 deletions(-) diff --git a/src/treesitter_analyzer.rs b/src/treesitter_analyzer.rs index 4547fd9..5c507fb 100644 --- a/src/treesitter_analyzer.rs +++ b/src/treesitter_analyzer.rs @@ -1253,25 +1253,59 @@ impl TreeSitterAnalyzer { .iter() .any(|entry| !readable.contains(&(entry.name.as_str(), entry.line_start))) { + tracing::info!( + file = %file_path.display(), + "refused to read the macros an unreadable construct swallowed here: \ + the second reading of this file that would recover them no longer \ + sees a definition the first reading does, so nothing it found can \ + be trusted. The swallowed macros stay missing from the index." + ); return Ok(GraftedMacros::keeping(macros)); } } - let healed_macros = self.deduplicate_macros_within_file(raw_healed); - let known: HashSet<&str> = macros.iter().map(|entry| entry.name.as_str()).collect(); + // Keep only the rows the file states are `#define`s before one row + // per name survives: a row that looked like a definition once the + // construct around it was blanked can carry a longer body than the + // real definition of the same name, and would take that name's place. + let (on_define_rows, elsewhere): (Vec, Vec) = raw_healed + .into_iter() + .partition(|entry| declared.contains(&entry.line_start)); - let gained: Vec = healed_macros - .iter() - .filter(|entry| { - // A name the original already read keeps the row the original - // read: the index holds one row per name per file, and on a - // tie the tree that needed no blanking wins. - !known.contains(entry.name.as_str()) && declared.contains(&entry.line_start) - }) - .cloned() + let known: HashSet<&str> = macros.iter().map(|entry| entry.name.as_str()).collect(); + let gained: Vec = self + .deduplicate_macros_within_file(on_define_rows) + .into_iter() + // A name the original already read keeps the row the original + // read: the index holds one row per name per file, and on a tie + // the tree that needed no blanking wins. + .filter(|entry| !known.contains(entry.name.as_str())) .collect(); + // The names a blanked parse found where the file writes a comment or + // a string. Naming them matters: it is the difference between a file + // this cannot help and a file it refused to invent a macro for. + let refused: HashSet<&str> = { + let taken: HashSet<&str> = gained.iter().map(|entry| entry.name.as_str()).collect(); + elsewhere + .iter() + .map(|entry| entry.name.as_str()) + .filter(|name| !known.contains(name) && !taken.contains(name)) + .collect() + }; + let invented = refused.len(); + if gained.is_empty() { + if invented > 0 { + tracing::info!( + file = %file_path.display(), + refused = invented, + "read no macros an unreadable construct swallowed here: every \ + definition found inside the swallowed part of this file sits where \ + the file writes a comment or a string, so none of them is a macro \ + the file defines" + ); + } return Ok(GraftedMacros::keeping(macros)); } @@ -1280,6 +1314,28 @@ impl TreeSitterAnalyzer { // `dev_printk`, which is the same silence one hop along. Offsets // survive blanking, so a site read from the healed tree is at the // place the file puts it. + // A file that indexes without the macros it defines says nothing + // today, which is why the defect stood for as long as it did: 41 + // directives in one logging header, one row, no warning. Say it. + tracing::info!( + file = %file_path.display(), + recovered = gained.len(), + refused = invented, + "read {} function-like macro{} an unreadable construct had swallowed in this \ + file{}", + gained.len(), + if gained.len() == 1 { "" } else { "s" }, + match invented { + 0 => String::new(), + 1 => ", and refused one more that sits where the file writes a comment or a \ + string" + .to_string(), + n => format!( + ", and refused {n} more that sit where the file writes a comment or a string" + ), + } + ); + let recovered: HashSet<&str> = gained.iter().map(|entry| entry.name.as_str()).collect(); let grafted = GraftedMacros { dispatch_sites: sites @@ -8919,6 +8975,47 @@ mod preproc_error_recovery_tests { found } + /// Somewhere for a test to read what the analyzer said. + #[derive(Clone, Default)] + struct Recorded(std::sync::Arc>>); + + impl std::io::Write for Recorded { + fn write(&mut self, buf: &[u8]) -> std::io::Result { + self.0.lock().unwrap().extend_from_slice(buf); + Ok(buf.len()) + } + + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } + } + + impl<'a> tracing_subscriber::fmt::MakeWriter<'a> for Recorded { + type Writer = Self; + + fn make_writer(&'a self) -> Self::Writer { + self.clone() + } + } + + /// What the analyzer logs while reading one file. The subscriber is + /// thread-local, so tests running beside each other do not read each + /// other's lines. + fn log_of(source: &str) -> String { + let recorded = Recorded::default(); + let subscriber = tracing_subscriber::fmt() + .with_writer(recorded.clone()) + .with_max_level(tracing::Level::INFO) + .without_time() + .with_ansi(false) + .finish(); + tracing::subscriber::with_default(subscriber, || { + let _ = macros_in(source); + }); + let bytes = recorded.0.lock().unwrap().clone(); + String::from_utf8(bytes).unwrap() + } + fn macro_names(source: &str) -> Vec { let mut names: Vec = macros_in(source) .into_iter() @@ -9080,6 +9177,68 @@ mod preproc_error_recovery_tests { */\n\ #define real(x) x\n"; + /// A row inside a spliced comment that defines the same name as a real + /// swallowed definition below it, with a longer body. One row per name + /// survives, and the longer body wins, so choosing the survivor before + /// the file has been asked which rows are comments hands the name to + /// the row that was never a definition. + const PHANTOM_TAKES_A_NAME: &str = "#define kept(x) x\n\ + static inline __printf(3, 4)\n\ + int broken(const char *fmt, ...);\n\ + // this comment continues over the row below \\\n\ + #define shared(x) a_much_longer_expansion_of(x, x, x, x)\n\ + #define shared(x) x\n"; + + #[test] + fn a_phantom_cannot_take_the_name_of_a_real_definition() { + // The real definition is on row 6 and the comment's row 5 carries + // the longer body. Recovering row 5 would be inventing a macro; not + // recovering row 6 because row 5 outbid it loses a real one. + let found = macros_in(PHANTOM_TAKES_A_NAME); + assert!( + found.contains(&("shared".to_string(), 6)), + "the real definition was not recovered: {found:?}" + ); + assert!( + !found + .iter() + .any(|(name, line)| name == "shared" && *line == 5), + "a row the file writes as a comment was read as a definition: {found:?}" + ); + } + + #[test] + fn a_file_whose_macros_were_recovered_says_which_and_how_many() { + // The defect stood as long as it did because a file that indexes + // without the macros it defines indexes "successfully": 41 + // directives in one logging header, one row, nothing logged. + let said = log_of(LOGGING_HEADER); + assert!( + said.contains("read 3 function-like macros"), + "the count of what was recovered is not in the log: {said}" + ); + assert!(said.contains("fixture.h"), "{said}"); + } + + #[test] + fn a_file_where_a_definition_was_refused_says_that_too() { + // Recovery declining is as much a fact about the file as recovery + // working. Here one row looked like a definition once the construct + // around it was blanked, and the file states it is the continuation + // of a comment. + let said = log_of(SPLICED_COMMENT); + assert!(said.contains("refused=1"), "{said}"); + } + + #[test] + fn a_file_the_parser_reads_whole_says_nothing() { + // Silence is the answer for the file with nothing to recover, or the + // log is noise: 1,639 files on one kernel tree enter recovery and + // 139 of them gain anything. + let said = log_of("#define fine(x) x\n\nvoid plain(void)\n{}\n"); + assert!(said.is_empty(), "{said}"); + } + #[test] fn a_recovered_macro_brings_what_its_body_does() { // A row for the definition is half of it. The body of a recovered