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) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
