[llvm-branch-commits] [clang] 0530e2a - [Syntax] Merge overlapping top-level macros in TokenBuffer

[Haojian Wu via llvm-branch-commits]

Author: Sam McCall
Date: 2020-06-10T14:21:44+02:00
New Revision: 0530e2a811b08f13e8137c29f047ad6bd11967fa

URL: https://github.com/llvm/llvm-project/commit/0530e2a811b08f13e8137c29f047ad6bd11967fa
DIFF: https://github.com/llvm/llvm-project/commit/0530e2a811b08f13e8137c29f047ad6bd11967fa.diff

LOG: [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 d66afd6dde542dc373f87e07fe764c071fe20d76)

Added: 
    

Modified: 
    clang/lib/Tooling/Syntax/Tokens.cpp
    clang/unittests/Tooling/Syntax/TokensTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Tooling/Syntax/Tokens.cpp b/clang/lib/Tooling/Syntax/Tokens.cpp
index 77cc9295757b..35a35f904069 100644
--- a/clang/lib/Tooling/Syntax/Tokens.cpp
+++ b/clang/lib/Tooling/Syntax/Tokens.cpp
@@ -335,14 +335,38 @@ class TokenCollector::CollectPPExpansions : public PPCallbacks {
                     SourceRange Range, const MacroArgs *Args) override {
     if (!Collector)
       return;
-    // Only record top-level expansions, not those where:
+    const auto &SM = Collector->PP.getSourceManager();
+    // Only record top-level expansions that directly produce expanded tokens.
+    // This excludes those where:
     //   - the macro use is inside a macro body,
     //   - the macro appears in an argument to another macro.
-    if (!MacroNameTok.getLocation().isFileID() ||
-        (LastExpansionEnd.isValid() &&
-         Collector->PP.getSourceManager().isBeforeInTranslationUnit(
-             Range.getBegin(), LastExpansionEnd)))
+    // However macro expansion isn't really a tree, it's token rewrite rules,
+    // so there are other cases, e.g.
+    //   #define B(X) X
+    //   #define A 1 + B
+    //   A(2)
+    // Both A and B produce expanded tokens, though the macro name 'B' comes
+    // from an expansion. The best we can do is merge the mappings for both.
+
+    // The *last* token of any top-level macro expansion must be in a file.
+    // (In the example above, see the closing paren of the expansion of B).
+    if (!Range.getEnd().isFileID())
       return;
+    // If there's a current expansion that encloses this one, this one can't be
+    // top-level.
+    if (LastExpansionEnd.isValid() &&
+        !SM.isBeforeInTranslationUnit(LastExpansionEnd, Range.getEnd()))
+      return;
+
+    // If the macro invocation (B) starts in a macro (A) but ends in a file,
+    // we'll create a merged mapping for A + B by overwriting the endpoint for
+    // A's startpoint.
+    if (!Range.getBegin().isFileID()) {
+      Range.setBegin(SM.getExpansionLoc(Range.getBegin()));
+      assert(Collector->Expansions.count(Range.getBegin().getRawEncoding()) &&
+             "Overlapping macros should have same expansion location");
+    }
+
     Collector->Expansions[Range.getBegin().getRawEncoding()] = Range.getEnd();
     LastExpansionEnd = Range.getEnd();
   }

diff  --git a/clang/unittests/Tooling/Syntax/TokensTest.cpp b/clang/unittests/Tooling/Syntax/TokensTest.cpp
index b2ad3859104a..f1e2dda963cb 100644
--- a/clang/unittests/Tooling/Syntax/TokensTest.cpp
+++ b/clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -470,11 +470,28 @@ file './input.cpp'
   mappings:
     ['#'_0, 'int'_7) => ['int'_0, 'int'_0)
     ['FOO'_10, '<eof>'_11) => ['10'_3, '<eof>'_7)
-)"}};
+)"},
+      {R"cpp(
+         #define NUM 42
+         #define ID(a) a
+         #define M 1 + ID
+         M(NUM)
+       )cpp",
+       R"(expanded tokens:
+  1 + 42
+file './input.cpp'
+  spelled tokens:
+    # define NUM 42 # define ID ( a ) a # define M 1 + ID M ( NUM )
+  mappings:
+    ['#'_0, 'M'_17) => ['1'_0, '1'_0)
+    ['M'_17, '<eof>'_21) => ['1'_0, '<eof>'_3)
+)"},
+  };
 
-  for (auto &Test : TestCases)
-    EXPECT_EQ(Test.second, collectAndDump(Test.first))
-        << collectAndDump(Test.first);
+  for (auto &Test : TestCases) {
+    std::string Dump = collectAndDump(Test.first);
+    EXPECT_EQ(Test.second, Dump) << Dump;
+  }
 }
 
 TEST_F(TokenCollectorTest, SpecialTokens) {