This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0cf888514454: [include-cleaner] Add self-contained file support for PragmaIncludes. (authored by hokein).
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 clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
Index: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp =================================================================== --- clang-tools-extra/include-cleaner/unittests/RecordTest.cpp +++ clang-tools-extra/include-cleaner/unittests/RecordTest.cpp @@ -390,5 +390,21 @@ 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 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 @@ -8,72 +8,150 @@ #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 } // 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,30 @@ 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: { 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 @@ -15,6 +15,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