From b87d067637d94222cb88723196c365c05f3d190a Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Tue, 13 Oct 2026 17:50:15 +0200 Subject: [PATCH] fix(cpp): one spelling per template specialization in the preprocessed pass The preprocessed second pass rebuilt the expanded source with simplecpp's TokenList::stringify(), which puts one space between every pair of tokens: `template<> struct bitonic` became `bitonic < int64_t , AVX2 >`. Every QN builder takes the class segment from node text, so a definition the #961 rescue re-adopted from that buffer (vxsort's INLINE/NOINLINE macros break the raw parse on every method line) got a second, token-spaced QN, and the QN-equality dedup kept both. CALLS split across the two spellings (dotnet/runtime: 902 compact + 996 spaced into the bitonic sorters). The expanded buffer now keeps the source spacing: tokens that touched in the source (same file and line, no macro expansion between them) stay touching, every other gap is still one space, and the line structure and #line directives are unchanged, so every line remap holds. One fix point instead of a normaliser at each QN builder; no vendored code changes. Proof (macOS arm64, dotnet/runtime 60108ba6, two identical runs each): - duplicate definition groups 706 -> 217 (v0.11.0: 545); nodes with a token-spaced QN 495 -> 0; CALLS into bitonic 902 + 996 -> 998, one spelling. The remaining groups are coreclr gc.cpp's SVR/WKS builds (184) and 33 scope disagreements between the two parses. - Side effect: rapidjson document.h's GenericValue members now exist once, with the raw parse's scope (the token-spaced copy that carried the nested scope is gone); `operator[]` / `operator()` in the expanded pass now parse as single tokens. - New test (RED without the change): extraction extract_cpp_template_specialization_preprocessed_qn_single_spelling. Three c_lsp collision fixtures recalibrated to the new byte offsets (comment padding and offsets only, assertions unchanged). - extraction, c_lsp, pipeline, edge_types_probe, parse_coverage, conditional_variants, lsp_resolution_probe: 1852 passed. Signed-off-by: Martin Vogel --- internal/cbm/preprocessor.cpp | 52 ++++++++++++++++++++++++++- tests/test_c_lsp.c | 27 +++++++------- tests/test_extraction.c | 66 +++++++++++++++++++++++++++++++++++ 3 files changed, 132 insertions(+), 13 deletions(-) diff --git a/internal/cbm/preprocessor.cpp b/internal/cbm/preprocessor.cpp index 9be6298f3..43ab183d0 100644 --- a/internal/cbm/preprocessor.cpp +++ b/internal/cbm/preprocessor.cpp @@ -381,6 +381,56 @@ static bool parse_line_directive(const char *line, size_t len, uint32_t *out_lin return true; } +// True when `prev` and `tok` touched in the source text: same file and line, +// `prev` ending exactly where `tok` starts (simplecpp's own adjacency test), +// and neither produced by a macro expansion (expansion tokens carry the call +// site's location, so their columns prove nothing). +static bool tokens_source_adjacent(const simplecpp::Token *prev, const simplecpp::Token *tok) { + if (!prev->macro.empty() || !tok->macro.empty() || !prev->location.sameline(tok->location)) { + return false; + } + const std::string &s = prev->str(); + if (s.find_first_of("\r\n") != std::string::npos) { + return false; + } + return prev->location.col + s.size() == tok->location.col; +} + +// The expanded buffer in the source's own spelling. Mirrors +// simplecpp::TokenList::stringify() line for line — the same newlines and +// #line directives, so build_line_map and every line-based remap are unchanged — +// except that tokens which touched in the source stay touching. stringify() +// joins every same-line pair with one space, so the second pass spelled a +// multi-token name differently from the raw pass: the explicit specialization +// `template<> struct bitonic` became `bitonic < int64_t , AVX2 >`, +// and every QN built from that text (defs adopted by the #961 rescue, call +// scopes, C-LSP class scopes) minted a second node for the same definition, the +// QN dedup (strcmp) missing it. A gap that held whitespace or a comment is still +// one space, as before. +static std::string stringify_source_spacing(const simplecpp::TokenList &tokens) { + std::ostringstream ret; + simplecpp::Location loc; + loc.line = 1; + for (const simplecpp::Token *tok = tokens.cfront(); tok; tok = tok->next) { + if (tok->location.line < loc.line || tok->location.fileIndex != loc.fileIndex) { + ret << "\n#line " << tok->location.line << " \"" << tokens.file(tok->location) + << "\"\n"; + loc = tok->location; + } + while (tok->location.line > loc.line) { + ret << '\n'; + loc.line++; + } + const simplecpp::Token *prev = tok->previous; + if (prev && prev->location.sameline(tok->location) && !tokens_source_adjacent(prev, tok)) { + ret << ' '; + } + ret << tok->str(); + loc.adjust(tok->str()); + } + return ret.str(); +} + static bool build_line_map(const std::string &expanded, const std::string &main_file, uint32_t *original_line_by_expanded_line, uint8_t *belongs_to_main_file) { @@ -476,7 +526,7 @@ CBMPreprocessedSource *cbm_preprocess_with_map(const char *source, int source_le simplecpp::preprocess(output, rawtokens, files, filedata, dui); - std::string result = output.stringify(); + std::string result = stringify_source_spacing(output); // Clean up loaded file data simplecpp::cleanup(filedata); diff --git a/tests/test_c_lsp.c b/tests/test_c_lsp.c index 0dc3abb80..a4e46c7bc 100644 --- a/tests/test_c_lsp.c +++ b/tests/test_c_lsp.c @@ -16278,21 +16278,23 @@ TEST(clsp_c_global_initializer_does_not_inherit_previous_function_caller) { } static int assert_cpp_family_preprocessed_collision_isolated(CBMLanguage language) { - /* simplecpp removes the 41-byte comment payload and normalizes tokens. As - * verified by the non-vacuity assertions below, raw alpha.render and the - * macro-hidden expanded bravo.render both land at numeric span 182:200, - * even though those offsets refer to different source texts. */ + /* The preprocessed buffer drops the 21-byte comment payload and the line + * indentation, keeps source spacing between source tokens and one space + * between macro-expansion tokens. As verified by the non-vacuity assertions + * below, raw alpha.render and the macro-hidden expanded bravo.render both + * land at numeric span 162:180, even though those offsets refer to + * different source texts. */ static const char source[] = "struct Alpha { int render(){return 1;} };\n" "struct Bravo { int render(){return 2;} };\n" "int occurrence_probe(){\n" " Alpha alpha; Bravo bravo;\n" - " /*xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx*/ alpha . render ( );\n" + " /*xxxxxxxxxxxxxxxxxxxxx*/ alpha . render ( );\n" "#define HIDDEN_RENDER(x) x.render()\n" " return HIDDEN_RENDER(bravo);\n" "}\n"; - const uint32_t collision_start = 182; - const uint32_t collision_end = 200; + const uint32_t collision_start = 162; + const uint32_t collision_end = 180; ASSERT_EQ((uint32_t)(strstr(source, "alpha . render ( )") - source), collision_start); CBMFileResult *result = extract_c_family(source, language); @@ -16354,19 +16356,20 @@ TEST(clsp_cuda_preprocessed_coordinate_collision_isolated) { } TEST(clsp_c_preprocessed_coordinate_collision_isolated) { - /* The raw fp call and macro-hidden expanded fp call both occupy 206:212, - * but data flow resolves them to different targets. */ + /* The raw fp call and macro-hidden expanded fp call both occupy 180:186 + * (comment payload sized against the preprocessed buffer's spacing), but + * data flow resolves them to different targets. */ static const char source[] = "static int alpha_target(void){return 1;}\n" "static int bravo_target(void){return 2;}\n" "int occurrence_probe(void){\n" " int (*fp)(void) = alpha_target;\n" - " /*xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx*/ fp ( );\n" + " /*xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx*/ fp ( );\n" "#define HIDDEN_FP_CALL() (fp = bravo_target, fp())\n" " return HIDDEN_FP_CALL();\n" "}\n"; - const uint32_t collision_start = 206; - const uint32_t collision_end = 212; + const uint32_t collision_start = 180; + const uint32_t collision_end = 186; const char *raw_probe = strstr(source, "occurrence_probe"); ASSERT_NOT_NULL(raw_probe); ASSERT_EQ((uint32_t)(strstr(raw_probe, "fp ( )") - source), collision_start); diff --git a/tests/test_extraction.c b/tests/test_extraction.c index f66b72e16..7382e147e 100644 --- a/tests/test_extraction.c +++ b/tests/test_extraction.c @@ -7424,6 +7424,71 @@ TEST(extract_c_clean_file_no_recovery_duplicates_issue961) { PASS(); } +/* True when `qn` carries the token-joined spelling of the `box` + * template-id that the preprocessed buffer used to produce. */ +static bool qn_has_token_spaced_box(const char *qn) { + return qn && (strstr(qn, "box <") || strstr(qn, "< int") || strstr(qn, "int ,")); +} + +/* dotnet/runtime vxsort shape (src/coreclr/gc/vxsort/smallsort/ + * bitonic_sort.*.generated.h): methods of an explicit specialization + * `template<> struct bitonic` carry an attribute macro defined in + * another header (INLINE / NOINLINE), so the raw parse has ERROR regions on the + * method lines and the #961 rescue re-extracts them from the preprocessed + * buffer. That buffer used to join every token with one space, spelling the + * scope `box < int , 2 >` where the raw pass spells `box`; the QN dedup + * (strcmp) missed, so every method got a second node and the CALLS split across + * both. One definition, one spelling, on every path. */ +TEST(extract_cpp_template_specialization_preprocessed_qn_single_spelling) { + CBMFileResult *r = extract("#ifndef BOX_H\n" + "#define BOX_H\n" + "namespace ns {\n" + "template struct box {\n" + " static void sort(T *p);\n" + "};\n" + "template<> struct box {\n" + " static INLINE void first(int *p) { p[0] = 0; }\n" + " static INLINE void second(int *p) { first(p); }\n" + "};\n" + "}\n" + "#endif\n", + CBM_LANG_CPP, "t", "box.h"); + ASSERT_NOT_NULL(r); + int first_count = 0; + int second_count = 0; + const char *first_qn = NULL; + for (int i = 0; i < r->defs.count; i++) { + const CBMDefinition *d = &r->defs.items[i]; + if (qn_has_token_spaced_box(d->qualified_name)) { + fprintf(stderr, " [tmplqn] FAIL token-spaced def QN: %s\n", d->qualified_name); + } + ASSERT_FALSE(qn_has_token_spaced_box(d->qualified_name)); + if (d->name && strcmp(d->name, "first") == 0) { + first_count++; + first_qn = d->qualified_name; + } + if (d->name && strcmp(d->name, "second") == 0) { + second_count++; + } + } + ASSERT_EQ(first_count, 1); + ASSERT_EQ(second_count, 1); + ASSERT_NOT_NULL(first_qn); + ASSERT_NOT_NULL(strstr(first_qn, "ns.box.first")); + /* Call scopes and LSP-resolved endpoints name the same single spelling, so + * no CALLS edge lands on (or starts from) a phantom second node. */ + for (int i = 0; i < r->calls.count; i++) { + ASSERT_FALSE(qn_has_token_spaced_box(r->calls.items[i].enclosing_func_qn)); + } + for (int i = 0; i < r->resolved_calls.count; i++) { + const CBMResolvedCall *rc = &r->resolved_calls.items[i]; + ASSERT_FALSE(qn_has_token_spaced_box(rc->caller_qn)); + ASSERT_FALSE(qn_has_token_spaced_box(rc->callee_qn)); + } + cbm_free_result(r); + PASS(); +} + /* #1989: build-system export macros (UE "_API", CMake generate_export_header * "_EXPORT") are empty on the real compile line but opaque to tree-sitter, * so `class MOD_API Foo` misparses. The preprocessed second pass predefines the @@ -12359,6 +12424,7 @@ SUITE(extraction) { RUN_TEST(extract_cpp_preproc_macro_generated_callable_skipped_issue949); RUN_TEST(extract_c_ifdef_split_brace_after_include_remapped_issue949); RUN_TEST(extract_c_clean_file_no_recovery_duplicates_issue961); + RUN_TEST(extract_cpp_template_specialization_preprocessed_qn_single_spelling); RUN_TEST(extract_cpp_export_macro_class_recovery_issue1989); RUN_TEST(extract_cpp_export_macro_struct_recovery_issue1989); RUN_TEST(extract_cpp_export_macro_enum_recovery_issue1989);