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

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added a subscriber: arphaman.
Herald added a project: All.
kadircet requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

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

Depends on D143197 <https://reviews.llvm.org/D143197>


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143597

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


Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -170,6 +170,9 @@
   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 @@
   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);
Index: clang-tools-extra/clangd/Preamble.cpp
===================================================================
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -685,13 +685,16 @@
       // 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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D143597.495888.patch
Type: text/x-patch
Size: 2466 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230208/db61ef18/attachment-0001.bin>