[clang-tools-extra] 19659b5 - [clangd] Drop includes from disabled PP regions in preamble patch

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2023-02-13T09:49:13+01:00
New Revision: 19659b5f0dd1a1dcf745cf058d042ada2d4ff061

URL: https://github.com/llvm/llvm-project/commit/19659b5f0dd1a1dcf745cf058d042ada2d4ff061
DIFF: https://github.com/llvm/llvm-project/commit/19659b5f0dd1a1dcf745cf058d042ada2d4ff061.diff

LOG: [clangd] Drop includes from disabled PP regions in preamble patch

In rest of the clangd functionality we treat these includes as
non-existent. Do so under preamble patching.

Depends on D143197

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

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 cdb8db14722d..f99c4142a464 100644
--- a/clang-tools-extra/clangd/Preamble.cpp
+++ b/clang-tools-extra/clangd/Preamble.cpp
@@ -685,13 +685,16 @@ PreamblePatch PreamblePatch::create(llvm::StringRef FileName,
       // Include already present in the baseline preamble. Set resolved path and
       // put into preamble includes.
       if (It != ExistingIncludes.end()) {
-        auto &PatchedInc = PP.PreambleIncludes.emplace_back();
-        // Copy everything from existing include, apart from the location, when
-        // it's coming from baseline preamble.
-        if (It->second)
+        if (It->second) {
+          // If this header is included in an active region of the baseline
+          // preamble, preserve it.
+          auto &PatchedInc = PP.PreambleIncludes.emplace_back();
+          // Copy everything from existing include, apart from the location,
+          // when it's coming from baseline preamble.
           PatchedInc = *It->second;
-        PatchedInc.HashLine = Inc.HashLine;
-        PatchedInc.HashOffset = Inc.HashOffset;
+          PatchedInc.HashLine = Inc.HashLine;
+          PatchedInc.HashOffset = Inc.HashOffset;
+        }
         continue;
       }
       // Include is new in the modified preamble. Inject it into the patch and

diff  --git a/clang-tools-extra/clangd/unittests/PreambleTests.cpp b/clang-tools-extra/clangd/unittests/PreambleTests.cpp
index ac46bfd06a13..ae353f94ce06 100644
--- a/clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ b/clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -170,6 +170,9 @@ TEST(PreamblePatchTest, PatchesPreambleIncludes) {
   auto TU = TestTU::withCode(R"cpp(
     #include "a.h" // IWYU pragma: keep
     #include "c.h"
+    #ifdef FOO
+    #include "d.h"
+    #endif
   )cpp");
   TU.AdditionalFiles["a.h"] = "#include \"b.h\"";
   TU.AdditionalFiles["b.h"] = "";
@@ -178,10 +181,14 @@ TEST(PreamblePatchTest, PatchesPreambleIncludes) {
   auto BaselinePreamble = buildPreamble(
       TU.Filename, *buildCompilerInvocation(PI, Diags), PI, true, nullptr);
   // We drop c.h from modified and add a new header. Since the latter is patched
-  // we should only get a.h in preamble includes.
+  // we should only get a.h in preamble includes. d.h shouldn't be part of the
+  // preamble, as it's coming from a disabled region.
   TU.Code = R"cpp(
     #include "a.h"
     #include "b.h"
+    #ifdef FOO
+    #include "d.h"
+    #endif
   )cpp";
   auto PP = PreamblePatch::createFullPatch(testPath(TU.Filename), TU.inputs(FS),
                                            *BaselinePreamble);