[PATCH] D112571: [clangd] IncludeCleaner: Don't warn on system headers

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 382576.
kbobyrev marked an inline comment as done.
kbobyrev added a comment.

Resolve review comment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112571/new/

https://reviews.llvm.org/D112571

Files:
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -206,6 +206,7 @@
     #include "dir/c.h"
     #include "dir/unused.h"
     #include "unused.h"
+    #include <system_header.h>
     void foo() {
       a();
       b();
@@ -220,17 +221,17 @@
   TU.AdditionalFiles["dir/c.h"] = "void c();";
   TU.AdditionalFiles["unused.h"] = "void unused();";
   TU.AdditionalFiles["dir/unused.h"] = "void dirUnused();";
-  TU.AdditionalFiles["not_included.h"] = "void notIncluded();";
-  TU.ExtraArgs = {"-I" + testPath("dir")};
+  TU.AdditionalFiles["system/system_header.h"] = "";
+  TU.ExtraArgs.push_back("-I" + testPath("dir"));
+  TU.ExtraArgs.push_back("-isystem" + testPath("system"));
   TU.Code = MainFile.str();
   ParsedAST AST = TU.build();
-  auto UnusedIncludes = computeUnusedIncludes(AST);
-  std::vector<std::string> UnusedHeaders;
-  UnusedHeaders.reserve(UnusedIncludes.size());
-  for (const auto &Include : UnusedIncludes)
-    UnusedHeaders.push_back(Include->Written);
-  EXPECT_THAT(UnusedHeaders,
-              UnorderedElementsAre("\"unused.h\"", "\"dir/unused.h\""));
+  std::vector<std::string> UnusedIncludes;
+  for (const auto &Include : computeUnusedIncludes(AST))
+    UnusedIncludes.push_back(Include->Written);
+  EXPECT_THAT(UnusedIncludes,
+              UnorderedElementsAre("\"unused.h\"", "\"dir/unused.h\"",
+                                   "<system_header.h>"));
 }
 
 TEST(IncludeCleaner, ScratchBuffer) {
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1483,7 +1483,9 @@
 TEST(DiagnosticsTest, IncludeCleaner) {
   Annotations Test(R"cpp(
 $fix[[  $diag[[#include "unused.h"]]
-]]  #include "used.h"
+]]#include "used.h"
+
+  #include <system_header.h>
 
   void foo() {
     used();
@@ -1497,6 +1499,8 @@
   TU.AdditionalFiles["used.h"] = R"cpp(
     void used() {}
   )cpp";
+  TU.AdditionalFiles["system/system_header.h"] = "";
+  TU.ExtraArgs = {"-isystem" + testPath("system")};
   // Off by default.
   EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty());
   Config Cfg;
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -196,6 +196,14 @@
   }
 }
 
+// FIXME(kirillbobyrev): We currently do not support the umbrella headers.
+// Standard Library headers are typically umbrella headers, and system headers
+// are likely to be the Standard Library headers. Until we have a good support
+// for umbrella headers and Standard Library headers, don't warn about them.
+bool mayConsiderUnused(const Inclusion *Inc) {
+  return Inc->Written.front() != '<';
+}
+
 } // namespace
 
 ReferencedLocations findReferencedLocations(ParsedAST &AST) {
@@ -283,6 +291,8 @@
           ->getName()
           .str();
   for (const auto *Inc : computeUnusedIncludes(AST)) {
+    if (!mayConsiderUnused(Inc))
+      continue;
     Diag D;
     D.Message =
         llvm::formatv("included header {0} is not used",


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D112571.382576.patch
Type: text/x-patch
Size: 3421 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20211027/1fc7a7f8/attachment.bin>