Skip to content

Commit 0530e2a

Browse files
sam-mccallhokein
authored andcommitted
[Syntax] Merge overlapping top-level macros in TokenBuffer
Summary: Our previous definition of "top-level" was too informal, and didn't allow for overlapping macros that each directly produce expanded tokens. See D77507 for previous discussion. Fixes http://bugs.llvm.org/show_bug.cgi?id=45428 Reviewers: kadircet, vabridgers Subscribers: cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D77615 (cherry picked from commit d66afd6)
1 parent 41c5efc commit 0530e2a

File tree

2 files changed

+50
-9
lines changed

2 files changed

+50
-9
lines changed

clang/lib/Tooling/Syntax/Tokens.cpp

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -335,14 +335,38 @@ class TokenCollector::CollectPPExpansions : public PPCallbacks {
335335
SourceRange Range, const MacroArgs *Args) override {
336336
if (!Collector)
337337
return;
338-
// Only record top-level expansions, not those where:
338+
const auto &SM = Collector->PP.getSourceManager();
339+
// Only record top-level expansions that directly produce expanded tokens.
340+
// This excludes those where:
339341
// - the macro use is inside a macro body,
340342
// - the macro appears in an argument to another macro.
341-
if (!MacroNameTok.getLocation().isFileID() ||
342-
(LastExpansionEnd.isValid() &&
343-
Collector->PP.getSourceManager().isBeforeInTranslationUnit(
344-
Range.getBegin(), LastExpansionEnd)))
343+
// However macro expansion isn't really a tree, it's token rewrite rules,
344+
// so there are other cases, e.g.
345+
// #define B(X) X
346+
// #define A 1 + B
347+
// A(2)
348+
// Both A and B produce expanded tokens, though the macro name 'B' comes
349+
// from an expansion. The best we can do is merge the mappings for both.
350+
351+
// The *last* token of any top-level macro expansion must be in a file.
352+
// (In the example above, see the closing paren of the expansion of B).
353+
if (!Range.getEnd().isFileID())
345354
return;
355+
// If there's a current expansion that encloses this one, this one can't be
356+
// top-level.
357+
if (LastExpansionEnd.isValid() &&
358+
!SM.isBeforeInTranslationUnit(LastExpansionEnd, Range.getEnd()))
359+
return;
360+
361+
// If the macro invocation (B) starts in a macro (A) but ends in a file,
362+
// we'll create a merged mapping for A + B by overwriting the endpoint for
363+
// A's startpoint.
364+
if (!Range.getBegin().isFileID()) {
365+
Range.setBegin(SM.getExpansionLoc(Range.getBegin()));
366+
assert(Collector->Expansions.count(Range.getBegin().getRawEncoding()) &&
367+
"Overlapping macros should have same expansion location");
368+
}
369+
346370
Collector->Expansions[Range.getBegin().getRawEncoding()] = Range.getEnd();
347371
LastExpansionEnd = Range.getEnd();
348372
}

clang/unittests/Tooling/Syntax/TokensTest.cpp

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -470,11 +470,28 @@ file './input.cpp'
470470
mappings:
471471
['#'_0, 'int'_7) => ['int'_0, 'int'_0)
472472
['FOO'_10, '<eof>'_11) => ['10'_3, '<eof>'_7)
473-
)"}};
473+
)"},
474+
{R"cpp(
475+
#define NUM 42
476+
#define ID(a) a
477+
#define M 1 + ID
478+
M(NUM)
479+
)cpp",
480+
R"(expanded tokens:
481+
1 + 42
482+
file './input.cpp'
483+
spelled tokens:
484+
# define NUM 42 # define ID ( a ) a # define M 1 + ID M ( NUM )
485+
mappings:
486+
['#'_0, 'M'_17) => ['1'_0, '1'_0)
487+
['M'_17, '<eof>'_21) => ['1'_0, '<eof>'_3)
488+
)"},
489+
};
474490

475-
for (auto &Test : TestCases)
476-
EXPECT_EQ(Test.second, collectAndDump(Test.first))
477-
<< collectAndDump(Test.first);
491+
for (auto &Test : TestCases) {
492+
std::string Dump = collectAndDump(Test.first);
493+
EXPECT_EQ(Test.second, Dump) << Dump;
494+
}
478495
}
479496

480497
TEST_F(TokenCollectorTest, SpecialTokens) {

0 commit comments

Comments
 (0)