hokein updated this revision to Diff 476372.
hokein marked 4 inline comments as done.
hokein added a comment.
rebase and update based on the offline discussion
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137698/new/
https://reviews.llvm.org/D137698
Files:
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
Index: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
===================================================================
--- clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
@@ -17,6 +17,10 @@
namespace {
using testing::UnorderedElementsAre;
+std::string guard(llvm::StringRef Code) {
+ return "#pragma once\n" + Code.str();
+}
+
TEST(FindIncludeHeaders, IWYU) {
TestInputs Inputs;
PragmaIncludes PI;
@@ -38,20 +42,34 @@
#include "header1.h"
#include "header2.h"
)cpp";
- Inputs.ExtraFiles["header1.h"] = R"cpp(
+ Inputs.ExtraFiles["header1.h"] = guard(R"cpp(
// IWYU pragma: private, include "path/public.h"
- )cpp";
- Inputs.ExtraFiles["header2.h"] = R"cpp(
+ )cpp");
+ Inputs.ExtraFiles["header2.h"] = guard(R"cpp(
#include "detail1.h" // IWYU pragma: export
// IWYU pragma: begin_exports
#include "detail2.h"
// IWYU pragma: end_exports
+ #include "detail3.h" // IWYU pragma: export
+ #include "imp.inc"
+ #include "imp_private.inc"
+
#include "normal.h"
- )cpp";
+
+ )cpp");
Inputs.ExtraFiles["normal.h"] = Inputs.ExtraFiles["detail1.h"] =
- Inputs.ExtraFiles["detail2.h"] = "";
+ Inputs.ExtraFiles["detail2.h"] = guard("");
+ Inputs.ExtraFiles["detail3.h"] = guard(R"cpp(
+ #include "imp3.inc"
+ )cpp");
+ Inputs.ExtraFiles["imp3.inc"] = "";
+ Inputs.ExtraFiles["imp.inc"] = "";
+ Inputs.ExtraFiles["imp_private.inc"] = R"cpp(
+ // IWYU pragma: private, include "public2.h"
+ )cpp";
+
TestAST AST(Inputs);
const auto &SM = AST.sourceManager();
auto &FM = SM.getFileManager();
@@ -60,20 +78,36 @@
return SM.translateFileLineCol(FM.getFile(FileName).get(),
/*Line=*/1, /*Col=*/1);
};
+ auto PhysicalHeader = [&](llvm::StringRef FileName) {
+ return FM.getFile(FileName).get();
+ };
EXPECT_THAT(findHeaders(SourceLocFromFile("header1.h"), SM, &PI),
- UnorderedElementsAre(Header("\"path/public.h\"")));
+ UnorderedElementsAre(Header("\"path/public.h\""),
+ PhysicalHeader("header1.h")));
EXPECT_THAT(findHeaders(SourceLocFromFile("detail1.h"), SM, &PI),
- UnorderedElementsAre(Header(FM.getFile("header2.h").get()),
- Header(FM.getFile("detail1.h").get())));
+ UnorderedElementsAre(PhysicalHeader("header2.h"),
+ PhysicalHeader("detail1.h")));
EXPECT_THAT(findHeaders(SourceLocFromFile("detail2.h"), SM, &PI),
- UnorderedElementsAre(Header(FM.getFile("header2.h").get()),
- Header(FM.getFile("detail2.h").get())));
-
+ UnorderedElementsAre(PhysicalHeader("header2.h"),
+ PhysicalHeader("detail2.h")));
+ // Verify that we emit the exporter for the details3.h.
+ EXPECT_THAT(findHeaders(SourceLocFromFile("imp3.inc"), SM, &PI),
+ UnorderedElementsAre(PhysicalHeader("imp3.inc"),
+ PhysicalHeader("detail3.h"),
+ PhysicalHeader("header2.h")));
+ EXPECT_THAT(findHeaders(SourceLocFromFile("imp.inc"), SM, &PI),
+ UnorderedElementsAre(PhysicalHeader("header2.h"),
+ PhysicalHeader("imp.inc")));
+ // There is a IWYU priviate mapping in the non self-contained header, verify
+ // that we don't emit its includer.
+ EXPECT_THAT(findHeaders(SourceLocFromFile("imp_private.inc"), SM, &PI),
+ UnorderedElementsAre(PhysicalHeader("imp_private.inc"),
+ Header("\"public2.h\"")));
EXPECT_THAT(findHeaders(SourceLocFromFile("normal.h"), SM, &PI),
- UnorderedElementsAre(Header(FM.getFile("normal.h").get())));
+ UnorderedElementsAre(PhysicalHeader("normal.h")));
}
} // namespace
} // namespace clang::include_cleaner
\ No newline at end of file
Index: clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
===================================================================
--- clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -28,6 +28,10 @@
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 @@
}
)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 @@
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 @@
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))));
Index: clang-tools-extra/include-cleaner/lib/Record.cpp
===================================================================
--- clang-tools-extra/include-cleaner/lib/Record.cpp
+++ 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 @@
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 @@
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 @@
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;
Index: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
===================================================================
--- clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
+++ clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
@@ -19,27 +19,29 @@
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 {};
-
- 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)};
+ const FileEntry *FE = SM.getFileEntryForID(FID);
+ if (!PI && FE)
+ return {Header(FE)};
+ while (FE) {
+ Results.push_back(Header(FE));
// FIXME: compute transitive exporter headers.
for (const auto *Export : PI->getExporters(FE, SM.getFileManager()))
Results.push_back(Export);
- }
+ llvm::StringRef VerbatimSpelling = PI->getPublic(FE);
+ if (!VerbatimSpelling.empty()) {
+ Results.push_back(VerbatimSpelling);
+ break;
+ }
+ if (PI->isSelfContained(FE) || FID == SM.getMainFileID())
+ break;
+
+ // Walkup the include stack for non self-contained headers.
+ FID = SM.getDecomposedIncludedLoc(FID).first;
+ FE = SM.getFileEntryForID(FID);
+ }
return Results;
}
case SymbolLocation::Standard: {
Index: clang-tools-extra/include-cleaner/lib/CMakeLists.txt
===================================================================
--- clang-tools-extra/include-cleaner/lib/CMakeLists.txt
+++ clang-tools-extra/include-cleaner/lib/CMakeLists.txt
@@ -13,6 +13,7 @@
clangAST
clangBasic
clangLex
+ clangToolingInclusions
clangToolingInclusionsStdlib
)
Index: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
===================================================================
--- clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
+++ clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
@@ -70,6 +70,9 @@
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 @@
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.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
