[PATCH] D148143: [clangd] Treat preamble patch as main file for include-cleaner analysis

[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.

Since we redefine all macros in preamble-patch, and it's parsed after
consuming the preamble macros, we can get false missing-include diagnostics
while a fresh preamble is being rebuilt.

This patch makes sure preamble-patch is treated same as main file for
include-cleaner purposes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148143

Files:
  clang-tools-extra/clangd/IncludeCleaner.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
@@ -666,6 +666,7 @@
   Config Cfg;
   Cfg.Diagnostics.AllowStalePreamble = true;
   Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Strict;
+  Cfg.Diagnostics.MissingIncludes = Config::IncludesPolicy::Strict;
   WithContextValue WithCfg(Config::Key, std::move(Cfg));
 
   llvm::StringMap<std::string> AdditionalFiles;
@@ -699,6 +700,8 @@
   {
     Annotations Code("#define [[FOO]] 1\n");
     // Check ranges for notes.
+    // This also makes sure we don't generate missing-include diagnostics
+    // because macros are redefined in preamble-patch.
     Annotations NewCode(R"(#define BARXYZ 1
 #define $foo1[[FOO]] 1
 void foo();
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -11,6 +11,7 @@
 #include "Diagnostics.h"
 #include "Headers.h"
 #include "ParsedAST.h"
+#include "Preamble.h"
 #include "Protocol.h"
 #include "SourceCode.h"
 #include "URI.h"
@@ -112,12 +113,12 @@
       return false;
     // Check if main file is the public interface for a private header. If so we
     // shouldn't diagnose it as unused.
-    if(auto PHeader = PI->getPublic(*FE); !PHeader.empty()) {
+    if (auto PHeader = PI->getPublic(*FE); !PHeader.empty()) {
       PHeader = PHeader.trim("<>\"");
       // Since most private -> public mappings happen in a verbatim way, we
       // check textually here. This might go wrong in presence of symlinks or
       // header mappings. But that's not different than rest of the places.
-      if(AST.tuPath().endswith(PHeader))
+      if (AST.tuPath().endswith(PHeader))
         return false;
     }
   }
@@ -374,7 +375,8 @@
         bool Satisfied = false;
         for (const auto &H : Providers) {
           if (H.kind() == include_cleaner::Header::Physical &&
-              H.physical() == MainFile) {
+              (H.physical() == MainFile ||
+               H.physical()->getName().endswith(PreamblePatch::HeaderName))) {
             Satisfied = true;
             continue;
           }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D148143.512878.patch
Type: text/x-patch
Size: 2358 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230412/32977b3a/attachment.bin>