[clang-tools-extra] 75ae784 - [clangd] #undef macros inside preamble patch

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2023-02-22T15:54:15+01:00
New Revision: 75ae784e8f49cf2425f3bf702a1bbf6c581e721a

URL: https://github.com/llvm/llvm-project/commit/75ae784e8f49cf2425f3bf702a1bbf6c581e721a
DIFF: https://github.com/llvm/llvm-project/commit/75ae784e8f49cf2425f3bf702a1bbf6c581e721a.diff

LOG: [clangd] #undef macros inside preamble patch

That way we can stop generating false macro redefinition diagnostics.

Depends on D142890

Differential Revision: https://reviews.llvm.org/D143093

Added: 
    

Modified: 
    clang-tools-extra/clangd/Preamble.cpp
    clang-tools-extra/clangd/unittests/PreambleTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp
index f442c852c571..d01601e5a037 100644
--- a/clang-tools-extra/clangd/Preamble.cpp
+++ b/clang-tools-extra/clangd/Preamble.cpp
@@ -213,6 +213,9 @@ struct TextualPPDirective {
   // Full text that's representing the directive, including the `#`.
   std::string Text;
   unsigned Offset;
+  tok::PPKeywordKind Directive = tok::PPKeywordKind::pp_not_keyword;
+  // Name of the macro being defined in the case of a #define directive.
+  std::string MacroName;
 
   bool operator==(const TextualPPDirective &RHS) const {
     return std::tie(DirectiveLine, Offset, Text) ==
@@ -283,6 +286,8 @@ struct DirectiveCollector : public PPCallbacks {
       return;
     TextualDirectives.emplace_back();
     TextualPPDirective &TD = TextualDirectives.back();
+    TD.Directive = tok::pp_define;
+    TD.MacroName = MacroNameTok.getIdentifierInfo()->getName().str();
 
     const auto *MI = MD->getMacroInfo();
     TD.Text =
@@ -560,8 +565,8 @@ buildPreamble(PathRef FileName, CompilerInvocation CI,
 
   if (BuiltPreamble) {
     log("Built preamble of size {0} for file {1} version {2} in {3} seconds",
-         BuiltPreamble->getSize(), FileName, Inputs.Version,
-         PreambleTimer.getTime());
+        BuiltPreamble->getSize(), FileName, Inputs.Version,
+        PreambleTimer.getTime());
     std::vector<Diag> Diags = PreambleDiagnostics.take();
     auto Result = std::make_shared<PreambleData>(std::move(*BuiltPreamble));
     Result->Version = Inputs.Version;
@@ -724,6 +729,10 @@ PreamblePatch PreamblePatch::create(llvm::StringRef FileName,
     // reduce complexity. The former might cause problems because scanning is
     // imprecise and might pick directives from disabled regions.
     for (const auto &TD : ModifiedScan->TextualDirectives) {
+      // Introduce an #undef directive before #defines to suppress any
+      // re-definition warnings.
+      if (TD.Directive == tok::pp_define)
+        Patch << "#undef " << TD.MacroName << '\n';
       Patch << "#line " << TD.DirectiveLine << '\n';
       Patch << TD.Text << '\n';
     }

diff  --git a/clang-tools-extra/clangd/unittests/PreambleTests.cpp b/clang-tools-extra/clangd/unittests/PreambleTests.cpp
index 0bd5a9f80641..68de063f7168 100644
--- a/clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ b/clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -288,6 +288,7 @@ TEST(PreamblePatchTest, Define) {
         #define BAR
         [[BAR]])cpp",
           R"cpp(#line 0 ".*main.cpp"
+#undef BAR
 #line 2
 #define         BAR
 )cpp",
@@ -299,6 +300,7 @@ TEST(PreamblePatchTest, Define) {
 
         [[BAR]])cpp",
           R"cpp(#line 0 ".*main.cpp"
+#undef BAR
 #line 2
 #define         BAR
 )cpp",
@@ -310,6 +312,7 @@ TEST(PreamblePatchTest, Define) {
                 BAR
         [[BAR]])cpp",
           R"cpp(#line 0 ".*main.cpp"
+#undef BAR
 #line 3
 #define         BAR
 )cpp",
@@ -342,8 +345,10 @@ TEST(PreamblePatchTest, OrderingPreserved) {
   )cpp");
 
   llvm::StringLiteral ExpectedPatch(R"cpp(#line 0 ".*main.cpp"
+#undef BAR
 #line 2
 #define     BAR\(X, Y\) X Y
+#undef BAR
 #line 3
 #define     BAR\(X\) X
 )cpp");
@@ -693,23 +698,17 @@ TEST(PreamblePatch, DiagnosticsToPreamble) {
   {
     Annotations Code("#define [[FOO]] 1\n");
     // Check ranges for notes.
-    Annotations NewCode(R"(#define $barxyz[[BARXYZ]] 1
+    Annotations NewCode(R"(#define BARXYZ 1
 #define $foo1[[FOO]] 1
 void foo();
 #define $foo2[[FOO]] 2)");
     auto AST = createPatchedAST(Code.code(), NewCode.code(), AdditionalFiles);
     EXPECT_THAT(
         *AST->getDiagnostics(),
-        ElementsAre(
-            // FIXME: This diagnostics shouldn't exist. It's emitted from the
-            // preamble patch to the stale location inside preamble.
-            AllOf(Field(&Diag::Name, Eq("-Wmacro-redefined")),
-                  Field(&Diag::File, HasSubstr("_preamble_patch_")),
-                  withNote(Diag(NewCode.range("barxyz")))),
-            AllOf(
-                Diag(NewCode.range("foo2"), "-Wmacro-redefined"),
-                // FIXME: This should be translated into main file.
-                withNote(Field(&Note::File, HasSubstr("_preamble_patch_"))))));
+        ElementsAre(AllOf(
+            Diag(NewCode.range("foo2"), "-Wmacro-redefined"),
+            // FIXME: This should be translated into main file.
+            withNote(Field(&Note::File, HasSubstr("_preamble_patch_"))))));
   }
 }