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.
Headers without include guards might have side effects or can be the files we
don't want to consider (e.g. tablegen ".inc" files). Skip them when translating
headers to the HeaderIDs that we will consider as unused.
Repository:
rG LLVM Github Monorepo
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>
@@ -58,7 +59,8 @@
/// FileIDs that are not true files (<built-in> etc) 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
