[clang-tools-extra] 0cf8885 - [include-cleaner] Add self-contained file support for PragmaIncludes.
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Fri Nov 18 04:52:39 PST 2022
Author: Haojian Wu
Date: 2022-11-18T13:52:30+01:00
New Revision: 0cf888514454350cd97ab79cdb4a73e7f189eea0
URL: https://github.com/llvm/llvm-project/commit/0cf888514454350cd97ab79cdb4a73e7f189eea0
DIFF: https://github.com/llvm/llvm-project/commit/0cf888514454350cd97ab79cdb4a73e7f189eea0.diff
LOG: [include-cleaner] Add self-contained file support for PragmaIncludes.
And use it findHeaders. findHeaders now finds all header candidates
given a symbol location (these headers will be attached with proper
signals, in a followup patch).
Differential Revision: https://reviews.llvm.org/D137698
Added:
Modified:
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
clang-tools-extra/include-cleaner/lib/CMakeLists.txt
clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
clang-tools-extra/include-cleaner/lib/Record.cpp
clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
index 6a6e339a7f43f..d6a6a6aa417c5 100644
--- a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
+++ b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
@@ -70,6 +70,9 @@ class PragmaIncludes {
llvm::SmallVector<const FileEntry *> getExporters(const FileEntry *File,
FileManager &FM) const;
+ /// Returns true if the given file is a self-contained file.
+ bool isSelfContained(const FileEntry *File) const;
+
private:
class RecordPragma;
/// 1-based Line numbers for the #include directives of the main file that
@@ -94,11 +97,13 @@ class PragmaIncludes {
llvm::SmallVector</*RealPathNames*/ llvm::StringRef>>
IWYUExportBy;
+ /// Contains all non self-contained files detected during the parsing.
+ llvm::DenseSet<llvm::sys::fs::UniqueID> NonSelfContainedFiles;
+
/// Owns the strings.
llvm::BumpPtrAllocator Arena;
// FIXME: add support for clang use_instead pragma
- // FIXME: add selfcontained file.
};
/// Recorded main-file parser events relevant to include-cleaner.
diff --git a/clang-tools-extra/include-cleaner/lib/CMakeLists.txt b/clang-tools-extra/include-cleaner/lib/CMakeLists.txt
index a71276f54b643..99f73836a5ad3 100644
--- a/clang-tools-extra/include-cleaner/lib/CMakeLists.txt
+++ b/clang-tools-extra/include-cleaner/lib/CMakeLists.txt
@@ -15,6 +15,7 @@ clang_target_link_libraries(clangIncludeCleaner
clangAST
clangBasic
clangLex
+ clangToolingInclusions
clangToolingInclusionsStdlib
)
diff --git a/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp b/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
index c782ff89755cb..321d692d7a8a8 100644
--- a/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
+++ b/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
@@ -19,27 +19,30 @@ llvm::SmallVector<Header> findHeaders(const SymbolLocation &Loc,
switch (Loc.kind()) {
case SymbolLocation::Physical: {
// FIXME: Handle macro locations.
- // FIXME: Handle non self-contained files.
FileID FID = SM.getFileID(Loc.physical());
- const auto *FE = SM.getFileEntryForID(FID);
- if (!FE)
- return {};
+ const FileEntry *FE = SM.getFileEntryForID(FID);
+ if (!PI) {
+ return FE ? llvm::SmallVector<Header>{Header(FE)}
+ : llvm::SmallVector<Header>();
+ }
+ while (FE) {
+ Results.push_back(Header(FE));
+ // FIXME: compute transitive exporter headers.
+ for (const auto *Export : PI->getExporters(FE, SM.getFileManager()))
+ Results.push_back(Header(Export));
- Results = {Header(FE)};
- if (PI) {
- // We treat the spelling header in the IWYU pragma as the final public
- // header.
- // FIXME: look for exporters if the public header is exported by another
- // header.
llvm::StringRef VerbatimSpelling = PI->getPublic(FE);
- if (!VerbatimSpelling.empty())
- return {Header(VerbatimSpelling)};
+ if (!VerbatimSpelling.empty()) {
+ Results.push_back(VerbatimSpelling);
+ break;
+ }
+ if (PI->isSelfContained(FE) || FID == SM.getMainFileID())
+ break;
- // FIXME: compute transitive exporter headers.
- for (const auto *Export : PI->getExporters(FE, SM.getFileManager()))
- Results.push_back(Export);
+ // Walkup the include stack for non self-contained headers.
+ FID = SM.getDecomposedIncludedLoc(FID).first;
+ FE = SM.getFileEntryForID(FID);
}
-
return Results;
}
case SymbolLocation::Standard: {
diff --git a/clang-tools-extra/include-cleaner/lib/Record.cpp b/clang-tools-extra/include-cleaner/lib/Record.cpp
index 72db299114da8..16956b45198b6 100644
--- a/clang-tools-extra/include-cleaner/lib/Record.cpp
+++ b/clang-tools-extra/include-cleaner/lib/Record.cpp
@@ -16,6 +16,7 @@
#include "clang/Lex/MacroInfo.h"
#include "clang/Lex/PPCallbacks.h"
#include "clang/Lex/Preprocessor.h"
+#include "clang/Tooling/Inclusions/HeaderAnalysis.h"
namespace clang::include_cleaner {
namespace {
@@ -118,12 +119,25 @@ static llvm::Optional<StringRef> parseIWYUPragma(const char *Text) {
class PragmaIncludes::RecordPragma : public PPCallbacks, public CommentHandler {
public:
RecordPragma(const CompilerInstance &CI, PragmaIncludes *Out)
- : SM(CI.getSourceManager()), Out(Out), UniqueStrings(Arena) {}
+ : SM(CI.getSourceManager()),
+ HeaderInfo(CI.getPreprocessor().getHeaderSearchInfo()), Out(Out),
+ UniqueStrings(Arena) {}
void FileChanged(SourceLocation Loc, FileChangeReason Reason,
SrcMgr::CharacteristicKind FileType,
FileID PrevFID) override {
InMainFile = SM.isWrittenInMainFile(Loc);
+
+ if (Reason == PPCallbacks::ExitFile) {
+ // At file exit time HeaderSearchInfo is valid and can be used to
+ // determine whether the file was a self-contained header or not.
+ if (const FileEntry *FE = SM.getFileEntryForID(PrevFID)) {
+ if (tooling::isSelfContainedHeader(FE, SM, HeaderInfo))
+ Out->NonSelfContainedFiles.erase(FE->getUniqueID());
+ else
+ Out->NonSelfContainedFiles.insert(FE->getUniqueID());
+ }
+ }
}
void EndOfMainFile() override {
@@ -238,6 +252,7 @@ class PragmaIncludes::RecordPragma : public PPCallbacks, public CommentHandler {
bool InMainFile = false;
const SourceManager &SM;
+ HeaderSearch &HeaderInfo;
PragmaIncludes *Out;
llvm::BumpPtrAllocator Arena;
/// Intern table for strings. Contents are on the arena.
@@ -287,6 +302,10 @@ PragmaIncludes::getExporters(const FileEntry *File, FileManager &FM) const {
return Results;
}
+bool PragmaIncludes::isSelfContained(const FileEntry *FE) const {
+ return !NonSelfContainedFiles.contains(FE->getUniqueID());
+}
+
std::unique_ptr<ASTConsumer> RecordedAST::record() {
class Recorder : public ASTConsumer {
RecordedAST *Out;
diff --git a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
index 5e3866469862c..2149800c966d1 100644
--- a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -28,6 +28,10 @@ namespace {
using testing::Pair;
using testing::UnorderedElementsAre;
+std::string guard(llvm::StringRef Code) {
+ return "#pragma once\n" + Code.str();
+}
+
TEST(WalkUsed, Basic) {
// FIXME: Have a fixture for setting up tests.
llvm::Annotations Code(R"cpp(
@@ -40,14 +44,14 @@ TEST(WalkUsed, Basic) {
}
)cpp");
TestInputs Inputs(Code.code());
- Inputs.ExtraFiles["header.h"] = R"cpp(
+ Inputs.ExtraFiles["header.h"] = guard(R"cpp(
void foo();
namespace std { class vector {}; }
- )cpp";
- Inputs.ExtraFiles["private.h"] = R"cpp(
+ )cpp");
+ Inputs.ExtraFiles["private.h"] = guard(R"cpp(
// IWYU pragma: private, include "path/public.h"
class Private {};
- )cpp";
+ )cpp");
PragmaIncludes PI;
Inputs.MakeAction = [&PI] {
@@ -78,7 +82,8 @@ TEST(WalkUsed, Basic) {
EXPECT_EQ(FID, SM.getMainFileID());
OffsetToProviders.try_emplace(Offset, Providers.vec());
});
- auto HeaderFile = Header(AST.fileManager().getFile("header.h").get());
+ auto &FM = AST.fileManager();
+ auto HeaderFile = Header(FM.getFile("header.h").get());
auto MainFile = Header(SM.getFileEntryForID(SM.getMainFileID()));
auto VectorSTL = Header(tooling::stdlib::Header::named("<vector>").value());
EXPECT_THAT(
@@ -86,7 +91,8 @@ TEST(WalkUsed, Basic) {
UnorderedElementsAre(
Pair(Code.point("bar"), UnorderedElementsAre(MainFile)),
Pair(Code.point("private"),
- UnorderedElementsAre(Header("\"path/public.h\""))),
+ UnorderedElementsAre(Header("\"path/public.h\""),
+ Header(FM.getFile("private.h").get()))),
Pair(Code.point("foo"), UnorderedElementsAre(HeaderFile)),
Pair(Code.point("vector"), UnorderedElementsAre(VectorSTL)),
Pair(Code.point("vconstructor"), UnorderedElementsAre(VectorSTL))));
diff --git a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
index c0da609907408..d176efaf5497d 100644
--- a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
@@ -8,71 +8,149 @@
#include "AnalysisInternal.h"
#include "clang-include-cleaner/Record.h"
+#include "clang/Basic/FileEntry.h"
+#include "clang/Basic/FileManager.h"
#include "clang/Frontend/FrontendActions.h"
#include "clang/Testing/TestAST.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
+#include <memory>
namespace clang::include_cleaner {
namespace {
using testing::UnorderedElementsAre;
-TEST(FindIncludeHeaders, IWYU) {
+std::string guard(llvm::StringRef Code) {
+ return "#pragma once\n" + Code.str();
+}
+
+class FindHeadersTest : public testing::Test {
+protected:
TestInputs Inputs;
PragmaIncludes PI;
- Inputs.MakeAction = [&PI] {
- struct Hook : public PreprocessOnlyAction {
- public:
- Hook(PragmaIncludes *Out) : Out(Out) {}
- bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
- Out->record(CI);
- return true;
- }
-
- PragmaIncludes *Out;
+ std::unique_ptr<TestAST> AST;
+ FindHeadersTest() {
+ Inputs.MakeAction = [this] {
+ struct Hook : public PreprocessOnlyAction {
+ public:
+ Hook(PragmaIncludes *Out) : Out(Out) {}
+ bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
+ Out->record(CI);
+ return true;
+ }
+
+ PragmaIncludes *Out;
+ };
+ return std::make_unique<Hook>(&PI);
};
- return std::make_unique<Hook>(&PI);
+ }
+ void buildAST() { AST = std::make_unique<TestAST>(Inputs); }
+
+ llvm::SmallVector<Header> findHeaders(llvm::StringRef FileName) {
+ return include_cleaner::findHeaders(
+ AST->sourceManager().translateFileLineCol(
+ AST->fileManager().getFile(FileName).get(),
+ /*Line=*/1, /*Col=*/1),
+ AST->sourceManager(), &PI);
+ }
+ const FileEntry *physicalHeader(llvm::StringRef FileName) {
+ return AST->fileManager().getFile(FileName).get();
};
+};
+TEST_F(FindHeadersTest, IWYUPrivateToPublic) {
Inputs.Code = R"cpp(
- #include "header1.h"
- #include "header2.h"
+ #include "private.h"
)cpp";
- Inputs.ExtraFiles["header1.h"] = R"cpp(
+ Inputs.ExtraFiles["private.h"] = guard(R"cpp(
// IWYU pragma: private, include "path/public.h"
+ )cpp");
+ buildAST();
+ EXPECT_THAT(findHeaders("private.h"),
+ UnorderedElementsAre(physicalHeader("private.h"),
+ Header("\"path/public.h\"")));
+}
+
+TEST_F(FindHeadersTest, IWYUExport) {
+ Inputs.Code = R"cpp(
+ #include "exporter.h"
)cpp";
- Inputs.ExtraFiles["header2.h"] = R"cpp(
- #include "detail1.h" // IWYU pragma: export
+ Inputs.ExtraFiles["exporter.h"] = guard(R"cpp(
+ #include "exported1.h" // IWYU pragma: export
// IWYU pragma: begin_exports
- #include "detail2.h"
+ #include "exported2.h"
// IWYU pragma: end_exports
#include "normal.h"
+ )cpp");
+ Inputs.ExtraFiles["exported1.h"] = guard("");
+ Inputs.ExtraFiles["exported2.h"] = guard("");
+ Inputs.ExtraFiles["normal.h"] = guard("");
+
+ buildAST();
+ EXPECT_THAT(findHeaders("exported1.h"),
+ UnorderedElementsAre(physicalHeader("exported1.h"),
+ physicalHeader("exporter.h")));
+ EXPECT_THAT(findHeaders("exported2.h"),
+ UnorderedElementsAre(physicalHeader("exported2.h"),
+ physicalHeader("exporter.h")));
+ EXPECT_THAT(findHeaders("normal.h"),
+ UnorderedElementsAre(physicalHeader("normal.h")));
+ EXPECT_THAT(findHeaders("exporter.h"),
+ UnorderedElementsAre(physicalHeader("exporter.h")));
+}
+
+TEST_F(FindHeadersTest, SelfContained) {
+ Inputs.Code = R"cpp(
+ #include "header.h"
)cpp";
- Inputs.ExtraFiles["normal.h"] = Inputs.ExtraFiles["detail1.h"] =
- Inputs.ExtraFiles["detail2.h"] = "";
- TestAST AST(Inputs);
- const auto &SM = AST.sourceManager();
- auto &FM = SM.getFileManager();
- // Returns the source location for the start of the file.
- auto SourceLocFromFile = [&](llvm::StringRef FileName) {
- return SM.translateFileLineCol(FM.getFile(FileName).get(),
- /*Line=*/1, /*Col=*/1);
- };
+ Inputs.ExtraFiles["header.h"] = guard(R"cpp(
+ #include "fragment.inc"
+ )cpp");
+ Inputs.ExtraFiles["fragment.inc"] = "";
+ buildAST();
+ EXPECT_THAT(findHeaders("fragment.inc"),
+ UnorderedElementsAre(physicalHeader("fragment.inc"),
+ physicalHeader("header.h")));
+}
- EXPECT_THAT(findHeaders(SourceLocFromFile("header1.h"), SM, &PI),
- UnorderedElementsAre(Header("\"path/public.h\"")));
+TEST_F(FindHeadersTest, NonSelfContainedTraversePrivate) {
+ Inputs.Code = R"cpp(
+ #include "header.h"
+ )cpp";
+ Inputs.ExtraFiles["header.h"] = guard(R"cpp(
+ #include "fragment.inc"
+ )cpp");
+ Inputs.ExtraFiles["fragment.inc"] = R"cpp(
+ // IWYU pragma: private, include "public.h"
+ )cpp";
- EXPECT_THAT(findHeaders(SourceLocFromFile("detail1.h"), SM, &PI),
- UnorderedElementsAre(Header(FM.getFile("header2.h").get()),
- Header(FM.getFile("detail1.h").get())));
- EXPECT_THAT(findHeaders(SourceLocFromFile("detail2.h"), SM, &PI),
- UnorderedElementsAre(Header(FM.getFile("header2.h").get()),
- Header(FM.getFile("detail2.h").get())));
+ buildAST();
+ // There is a IWYU private mapping in the non self-contained header, verify
+ // that we don't emit its includer.
+ EXPECT_THAT(findHeaders("fragment.inc"),
+ UnorderedElementsAre(physicalHeader("fragment.inc"),
+ Header("\"public.h\"")));
+}
- EXPECT_THAT(findHeaders(SourceLocFromFile("normal.h"), SM, &PI),
- UnorderedElementsAre(Header(FM.getFile("normal.h").get())));
+TEST_F(FindHeadersTest, NonSelfContainedTraverseExporter) {
+ Inputs.Code = R"cpp(
+ #include "exporter.h"
+ )cpp";
+ Inputs.ExtraFiles["exporter.h"] = guard(R"cpp(
+ #include "exported.h" // IWYU pragma: export
+ )cpp");
+ Inputs.ExtraFiles["exported.h"] = guard(R"cpp(
+ #include "fragment.inc"
+ )cpp");
+ Inputs.ExtraFiles["fragment.inc"] = "";
+ buildAST();
+ // Verify that we emit exporters for each header on the path.
+ EXPECT_THAT(findHeaders("fragment.inc"),
+ UnorderedElementsAre(physicalHeader("fragment.inc"),
+ physicalHeader("exported.h"),
+ physicalHeader("exporter.h")));
}
} // namespace
diff --git a/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp b/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
index 7b8d1c9ab2c46..37eabb9dbfb28 100644
--- a/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
@@ -390,5 +390,21 @@ TEST_F(PragmaIncludeTest, IWYUExportBlock) {
EXPECT_TRUE(PI.getExporters(FM.getFile("bar.h").get(), FM).empty());
}
+TEST_F(PragmaIncludeTest, SelfContained) {
+ Inputs.Code = R"cpp(
+ #include "guarded.h"
+
+ #include "unguarded.h"
+ )cpp";
+ Inputs.ExtraFiles["guarded.h"] = R"cpp(
+ #pragma once
+ )cpp";
+ Inputs.ExtraFiles["unguarded.h"] = "";
+ TestAST Processed = build();
+ auto &FM = Processed.fileManager();
+ EXPECT_TRUE(PI.isSelfContained(FM.getFile("guarded.h").get()));
+ EXPECT_FALSE(PI.isSelfContained(FM.getFile("unguarded.h").get()));
+}
+
} // namespace
} // namespace clang::include_cleaner
More information about the cfe-commits
mailing list