Skip to content

Commit

Permalink
[clangd] IncludeCleaner: Don't warn on system headers
Browse files Browse the repository at this point in the history
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
  • Loading branch information
kirillbobyrev committed Oct 27, 2021
1 parent 897402e commit c472378
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 10 deletions.
10 changes: 10 additions & 0 deletions clang-tools-extra/clangd/IncludeCleaner.cpp
Expand Up @@ -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) {
Expand Down Expand Up @@ -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",
Expand Down
6 changes: 5 additions & 1 deletion clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
Expand Up @@ -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();
Expand All @@ -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;
Expand Down
19 changes: 10 additions & 9 deletions clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
Expand Up @@ -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();
Expand All @@ -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) {
Expand Down

0 comments on commit c472378

Please sign in to comment.