kbobyrev updated this revision to Diff 382969.
kbobyrev added a comment.
Remove redundant changes.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112695/new/
https://reviews.llvm.org/D112695
Files:
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/IncludeCleaner.h
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
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 "no_guard.h"
+ #include "used.h"
#include <system_header.h>
@@ -1494,9 +1496,16 @@
TestTU TU;
TU.Code = Test.code().str();
TU.AdditionalFiles["unused.h"] = R"cpp(
+ #pragma once
void unused() {}
)cpp";
+ // This header is unused but doesn't have a header guard meaning it is not
+ // self-contained and can have side effects. There will be no warning for it.
+ TU.AdditionalFiles["no_guard.h"] = R"cpp(
+ int side_effects = 42;
+ )cpp";
TU.AdditionalFiles["used.h"] = R"cpp(
+ #pragma once
void used() {}
)cpp";
TU.AdditionalFiles["system/system_header.h"] = "";
Index: clang-tools-extra/clangd/IncludeCleaner.h
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.h
+++ clang-tools-extra/clangd/IncludeCleaner.h
@@ -55,7 +55,8 @@
const SourceManager &SM);
/// Maps FileIDs to the internal IncludeStructure representation (HeaderIDs).
-/// FileIDs that are not true files (<built-in> etc) are dropped.
+/// FileIDs that are not true files (<built-in> etc) and headers without
+/// include guards are dropped.
llvm::DenseSet<IncludeStructure::HeaderID>
translateToHeaderIDs(const llvm::DenseSet<FileID> &Files,
const IncludeStructure &Includes, const SourceManager &SM);
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -14,6 +14,8 @@
#include "support/Trace.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/Basic/SourceLocation.h"
+#include "clang/Lex/HeaderSearch.h"
+#include "clang/Lex/Preprocessor.h"
#include "clang/Tooling/Syntax/Tokens.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Path.h"
@@ -186,12 +188,27 @@
}
}
-// 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() != '<';
+bool mayConsiderUnused(const Inclusion *Inc, const IncludeStructure &Includes,
+ FileManager &FM, HeaderSearch &HeaderInfo) {
+ // 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.
+ if (Inc->Written.front() == '<')
+ return false;
+ // Headers without include guards have side effects and are not
+ // self-contained, skip them.
+ assert(Inc->HeaderID);
+ auto FE = FM.getFile(Includes.getRealPath(
+ static_cast<IncludeStructure::HeaderID>(*Inc->HeaderID)));
+ assert(FE);
+ if (!HeaderInfo.isFileMultipleIncludeGuarded(*FE)) {
+ dlog("{0} doesn't have header guard and will not be considered unused",
+ (*FE)->getName());
+ return false;
+ }
+ return true;
}
} // namespace
@@ -291,7 +308,9 @@
->getName()
.str();
for (const auto *Inc : computeUnusedIncludes(AST)) {
- if (!mayConsiderUnused(Inc))
+ if (!mayConsiderUnused(Inc, AST.getIncludeStructure(),
+ AST.getSourceManager().getFileManager(),
+ AST.getPreprocessor().getHeaderSearchInfo()))
continue;
Diag D;
D.Message =
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
