sammccall added a comment. Thanks for tracking this down. Unfortunately I don't think removing the assertion is enough, the code doesn't handle this case correctly.
================ Comment at: clang/unittests/Tooling/Syntax/TokensTest.cpp:487 )"}, + // Baseline case, bugz: https://bugs.llvm.org/show_bug.cgi?id=45428 + {R"cpp( ---------------- This is the case that always worked, right? Not sure we need to add this test. ================ Comment at: clang/unittests/Tooling/Syntax/TokensTest.cpp:505 + // Causes mapping miss when invoking tryConsumeSpelledUntil + {R"cpp( + #define NUM 42 ---------------- From the linked bug, the varags isn't essential to this crash. The linked example can be simplified a little further by removing foo: ``` #define N 1 #define ID(X) X #define ID2 ID ID2(N) ``` This makes it clearer that the trigger is using a macro in a arg to an "indirect" function macro (i.e. the function macro's name is expanded from another macro). ================ Comment at: clang/unittests/Tooling/Syntax/TokensTest.cpp:519 + ['#'_0, 'void'_34) => ['void'_0, 'void'_0) + ['M0'_40, 'NUM'_44) => ['{'_6, ';'_10) + ['NUM'_44, ')'_45) => [';'_10, ';'_10) ---------------- The problem with just removing the assertion is that this behavior is wrong: it's mapping `M0(,` onto `{ 42 ; }` - i.e. the whole M0 macro expansion. Then all the other mappings are empty. Compare to the baseline case above... This should be `[M0_42, ;_46)] => [{_6, ;_10]` I think. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77507/new/ https://reviews.llvm.org/D77507 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
