[clang-tools-extra] c472378 - [clangd] IncludeCleaner: Don't warn on system headers

[Kirill Bobyrev via cfe-commits]

Author: Kirill Bobyrev
Date: 2021-10-27T11:51:08+02:00
New Revision: c4723785c1902d6a53f3808fa1dabb2a97c1cc6e

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

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

This is a temporary hack to disable diagnostics for system headers. As of right
now, IncludeCleaner does not handle the Standard Library correctly and will
report most system headers as unused because very few symbols are defined in
top-level system headers. This will eventually be fixed, but for now we are
aiming for the most conservative approach with as little false-positive
warnings as possible. After the initial prototype and core functionality is
polished, I will turn back to handling the Standard Library as it requires
custom logic.

Reviewed By: sammccall

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

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp
index 95b4b2419cbe..cc69fdaa5e41 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -196,6 +196,14 @@ void findReferencedMacros(ParsedAST &AST, ReferencedLocations &Result) {
   }
 }
 
+// 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 @@ std::vector<Diag> issueUnusedIncludesDiagnostics(ParsedAST &AST,
           ->getName()
           .str();
   for (const auto *Inc : computeUnusedIncludes(AST)) {
+    if (!mayConsiderUnused(Inc))
+      continue;
     Diag D;
     D.Message =
         llvm::formatv("included header {0} is not used",

diff  --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index c3c56bdbbe03..4d269684b65a 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1483,7 +1483,9 @@ TEST(Diagnostics, Tags) {
 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 @@ TEST(DiagnosticsTest, IncludeCleaner) {
   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;

diff  --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
index 38cd8da23066..aeb76f8dee22 100644
--- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -206,6 +206,7 @@ TEST(IncludeCleaner, GetUnusedHeaders) {
     #include "dir/c.h"
     #include "dir/unused.h"
     #include "unused.h"
+    #include <system_header.h>
     void foo() {
       a();
       b();
@@ -220,17 +221,17 @@ TEST(IncludeCleaner, GetUnusedHeaders) {
   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) {