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 --- 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 @@ llvm::SmallVector 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> IWYUExportBy; + /// Contains all non self-contained files detected during the parsing. + llvm::DenseSet 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 --- a/clang-tools-extra/include-cleaner/lib/CMakeLists.txt +++ b/clang-tools-extra/include-cleaner/lib/CMakeLists.txt @@ -13,6 +13,7 @@ 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 --- a/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp +++ b/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: { diff --git a/clang-tools-extra/include-cleaner/lib/Record.cpp b/clang-tools-extra/include-cleaner/lib/Record.cpp --- 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 @@ 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 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 --- a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp +++ b/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("").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)))); diff --git a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp --- a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp +++ b/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