kbobyrev updated this revision to Diff 382955.
kbobyrev added a comment.
Update the docs.
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
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
@@ -9,6 +9,7 @@
#include "Annotations.h"
#include "IncludeCleaner.h"
#include "TestTU.h"
+#include "llvm/ADT/StringRef.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@@ -16,6 +17,10 @@
namespace clangd {
namespace {
+std::string guard(llvm::StringRef Code) {
+ return "#pragma once\n" + Code.str();
+}
+
using ::testing::ElementsAre;
using ::testing::UnorderedElementsAre;
@@ -216,13 +221,13 @@
// Build expected ast with symbols coming from headers.
TestTU TU;
TU.Filename = "foo.cpp";
- TU.AdditionalFiles["foo.h"] = "void foo();";
- TU.AdditionalFiles["a.h"] = "void a();";
- TU.AdditionalFiles["b.h"] = "void b();";
- TU.AdditionalFiles["dir/c.h"] = "void c();";
- TU.AdditionalFiles["unused.h"] = "void unused();";
- TU.AdditionalFiles["dir/unused.h"] = "void dirUnused();";
- TU.AdditionalFiles["system/system_header.h"] = "";
+ TU.AdditionalFiles["foo.h"] = guard("void foo();");
+ TU.AdditionalFiles["a.h"] = guard("void a();");
+ TU.AdditionalFiles["b.h"] = guard("void b();");
+ TU.AdditionalFiles["dir/c.h"] = guard("void c();");
+ TU.AdditionalFiles["unused.h"] = guard("void unused();");
+ TU.AdditionalFiles["dir/unused.h"] = guard("void dirUnused();");
+ TU.AdditionalFiles["system/system_header.h"] = guard("");
TU.ExtraArgs.push_back("-I" + testPath("dir"));
TU.ExtraArgs.push_back("-isystem" + testPath("system"));
TU.Code = MainFile.str();
@@ -252,6 +257,9 @@
// ScratchBuffer with `flags::FLAGS_FOO` that will have FileID but not
// FileEntry.
TU.AdditionalFiles["macros.h"] = R"cpp(
+ #ifndef MACROS_H
+ #define MACROS_H
+
#define DEFINE_FLAG(X) \
namespace flags { \
int FLAGS_##X; \
@@ -261,6 +269,8 @@
#define ab x
#define concat(x, y) x##y
+
+ #endif // MACROS_H
)cpp";
TU.ExtraArgs = {"-DCLI=42"};
ParsedAST AST = TU.build();
@@ -278,7 +288,9 @@
testPath("macros.h")));
// Should not crash due to FileIDs that are not headers.
- auto ReferencedHeaders = translateToHeaderIDs(ReferencedFiles, Includes, SM);
+ auto ReferencedHeaders =
+ translateToHeaderIDs(ReferencedFiles, Includes, SM,
+ AST.getPreprocessor().getHeaderSearchInfo());
std::vector<llvm::StringRef> ReferencedHeaderNames;
for (IncludeStructure::HeaderID HID : ReferencedHeaders)
ReferencedHeaderNames.push_back(Includes.getRealPath(HID));
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1494,9 +1494,11 @@
TestTU TU;
TU.Code = Test.code().str();
TU.AdditionalFiles["unused.h"] = R"cpp(
+ #pragma once
void unused() {}
)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
@@ -24,6 +24,7 @@
#include "Headers.h"
#include "ParsedAST.h"
#include "clang/Basic/SourceLocation.h"
+#include "clang/Lex/HeaderSearch.h"
#include "llvm/ADT/DenseSet.h"
#include <vector>
@@ -55,10 +56,12 @@
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);
+ const IncludeStructure &Includes, const SourceManager &SM,
+ HeaderSearch &HeaderSearch);
/// Retrieves headers that are referenced from the main file but not used.
std::vector<const Inclusion *>
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"
@@ -251,8 +253,8 @@
llvm::DenseSet<IncludeStructure::HeaderID>
translateToHeaderIDs(const llvm::DenseSet<FileID> &Files,
- const IncludeStructure &Includes,
- const SourceManager &SM) {
+ const IncludeStructure &Includes, const SourceManager &SM,
+ HeaderSearch &HeaderInfo) {
llvm::DenseSet<IncludeStructure::HeaderID> TranslatedHeaderIDs;
TranslatedHeaderIDs.reserve(Files.size());
for (FileID FID : Files) {
@@ -261,6 +263,13 @@
assert(isSpecialBuffer(FID, SM));
continue;
}
+ // Headers without include guards have side effects and are not
+ // self-contained, skip them.
+ if (!HeaderInfo.isFileMultipleIncludeGuarded(FE)) {
+ dlog("{0} doesn't have header guard and will not be considered unused",
+ FE->getName());
+ continue;
+ }
const auto File = Includes.getID(FE);
assert(File);
TranslatedHeaderIDs.insert(*File);
@@ -272,8 +281,9 @@
const auto &SM = AST.getSourceManager();
auto Refs = findReferencedLocations(AST);
- auto ReferencedFiles = translateToHeaderIDs(findReferencedFiles(Refs, SM),
- AST.getIncludeStructure(), SM);
+ auto ReferencedFiles = translateToHeaderIDs(
+ findReferencedFiles(Refs, SM), AST.getIncludeStructure(), SM,
+ AST.getPreprocessor().getHeaderSearchInfo());
return getUnused(AST.getIncludeStructure(), ReferencedFiles);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
