kbobyrev created this revision.
kbobyrev added a reviewer: sammccall.
Herald added subscribers: usaxena95, arphaman.
kbobyrev requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
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.
Repository:
rG LLVM Github Monorepo
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
@@ -177,6 +177,7 @@
#include "dir/c.h"
#include "dir/unused.h"
#include "unused.h"
+ #include <system_header.h>
void foo() {
a();
b();
@@ -191,17 +192,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
@@ -255,6 +255,12 @@
->getName()
.str();
for (const auto *Inc : computeUnusedIncludes(AST)) {
+ // FIXME(kirillbobyrev): Standard library headers are not handled correctly
+ // now because the symbols are typically not directly defined in the
+ // available headers. Suppress the warnings from any headers enclosed in
+ // angle brackets until we fully support the Standard Library features.
+ if (Inc->Written.front() == '<')
+ continue;
Diag D;
D.Message =
llvm::formatv("included header {0} is not used",
Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -177,6 +177,7 @@
#include "dir/c.h"
#include "dir/unused.h"
#include "unused.h"
+ #include <system_header.h>
void foo() {
a();
b();
@@ -191,17 +192,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
@@ -255,6 +255,12 @@
->getName()
.str();
for (const auto *Inc : computeUnusedIncludes(AST)) {
+ // FIXME(kirillbobyrev): Standard library headers are not handled correctly
+ // now because the symbols are typically not directly defined in the
+ // available headers. Suppress the warnings from any headers enclosed in
+ // angle brackets until we fully support the Standard Library features.
+ if (Inc->Written.front() == '<')
+ continue;
Diag D;
D.Message =
llvm::formatv("included header {0} is not used",
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
