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 @@ -19,6 +19,9 @@ #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DenseSet.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/Allocator.h" #include "llvm/Support/FileSystem/UniqueID.h" #include "clang-include-cleaner/Types.h" #include "llvm/ADT/ArrayRef.h" @@ -35,6 +38,7 @@ class FileEntry; class Preprocessor; class PPCallbacks; +class FileManager; namespace include_cleaner { @@ -61,6 +65,11 @@ /// Returns "" if there is none. llvm::StringRef getPublic(const FileEntry *File) const; + /// Returns all direct exporter headers for the given header file. + /// Returns empty if there is none. + llvm::SmallVector getExporters(const FileEntry *File, + FileManager &FM) const; + private: class RecordPragma; /// 1-based Line numbers for the #include directives of the main file that @@ -73,10 +82,21 @@ // !!NOTE: instead of using a FileEntry* to identify the physical file, we // deliberately use the UniqueID to ensure the result is stable across // FileManagers (for clangd's preamble and main-file builds). - llvm::DenseMap + llvm::DenseMap IWYUPublic; - // FIXME: add other IWYU supports (export etc) + /// A reverse map from the underlying header to its exporter headers. + // + // There's no way to get a FileEntry from a UniqueID, especially when it + // hasn't been opened before. So store the full path and convert it to a + // FileEntry by opening the file again through a FileManager. + llvm::DenseMap> + IWYUExportBy; + + /// Owns the strings. + llvm::BumpPtrAllocator Arena; + // FIXME: add support for clang use_instead pragma // FIXME: add selfcontained file. }; 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 @@ -118,7 +118,7 @@ class PragmaIncludes::RecordPragma : public PPCallbacks, public CommentHandler { public: RecordPragma(const CompilerInstance &CI, PragmaIncludes *Out) - : SM(CI.getSourceManager()), Out(Out) {} + : SM(CI.getSourceManager()), Out(Out), UniqueStrings(Arena) {} void FileChanged(SourceLocation Loc, FileChangeReason Reason, SrcMgr::CharacteristicKind FileType, @@ -126,6 +126,16 @@ InMainFile = SM.isWrittenInMainFile(Loc); } + void EndOfMainFile() override { + for (auto &It : Out->IWYUExportBy) { + llvm::sort(It.getSecond()); + It.getSecond().erase( + std::unique(It.getSecond().begin(), It.getSecond().end()), + It.getSecond().end()); + } + Out->Arena = std::move(Arena); + } + void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok, llvm::StringRef FileName, bool IsAngled, CharSourceRange /*FilenameRange*/, @@ -134,14 +144,35 @@ llvm::StringRef /*RelativePath*/, const clang::Module * /*Imported*/, SrcMgr::CharacteristicKind FileKind) override { - if (!InMainFile) - return; - int HashLine = - SM.getLineNumber(SM.getMainFileID(), SM.getFileOffset(HashLoc)); - if (LastPragmaKeepInMainFileLine == HashLine) + FileID HashFID = SM.getFileID(HashLoc); + int HashLine = SM.getLineNumber(HashFID, SM.getFileOffset(HashLoc)); + checkForExport(HashFID, HashLine, File ? &File->getFileEntry() : nullptr); + + if (InMainFile && LastPragmaKeepInMainFileLine == HashLine) Out->ShouldKeep.insert(HashLine); } + void checkForExport(FileID IncludingFile, int HashLine, + const FileEntry *IncludedHeader) { + if (ExportStack.empty()) + return; + auto &Top = ExportStack.back(); + if (Top.SeenAtFile != IncludingFile) + return; + // Make sure current include is covered by the export pragma. + if ((Top.Block && HashLine > Top.SeenAtLine) || + Top.SeenAtLine == HashLine) { + if (IncludedHeader) + Out->IWYUExportBy[IncludedHeader->getUniqueID()].push_back( + Top.FullPath); + // main-file #include with export pragma should never be removed. + if (Top.SeenAtFile == SM.getMainFileID()) + Out->ShouldKeep.insert(HashLine); + } + if (!Top.Block) // Pop immediately for single-line export pragma. + ExportStack.pop_back(); + } + bool HandleComment(Preprocessor &PP, SourceRange Range) override { auto &SM = PP.getSourceManager(); auto Pragma = parseIWYUPragma(SM.getCharacterData(Range.getBegin())); @@ -153,11 +184,32 @@ if (auto *FE = SM.getFileEntryForID(SM.getFileID(Range.getBegin()))) Out->IWYUPublic.insert( {FE->getLastRef().getUniqueID(), - Pragma->startswith("<") || Pragma->startswith("\"") - ? Pragma->str() - : ("\"" + *Pragma + "\"").str()}); + save(Pragma->startswith("<") || Pragma->startswith("\"") + ? (*Pragma) + : ("\"" + *Pragma + "\"").str())}); return false; } + FileID CommentFID = SM.getFileID(Range.getBegin()); + int CommentLine = SM.getLineNumber(SM.getFileID(Range.getBegin()), + SM.getFileOffset(Range.getBegin())); + // Record export pragma. + if (Pragma->startswith("export")) { + ExportStack.push_back( + {CommentLine, CommentFID, + save(SM.getFileEntryForID(CommentFID)->tryGetRealPathName()), + false}); + } else if (Pragma->startswith("begin_exports")) { + ExportStack.push_back( + {CommentLine, CommentFID, + save(SM.getFileEntryForID(CommentFID)->tryGetRealPathName()), true}); + } else if (Pragma->startswith("end_exports")) { + // FIXME: be robust on unmatching cases. We should only pop the stack if + // the begin_exports and end_exports is in the same file. + if (!ExportStack.empty()) { + assert(ExportStack.back().Block); + ExportStack.pop_back(); + } + } if (InMainFile) { if (!Pragma->startswith("keep")) @@ -176,18 +228,35 @@ // This code stores the last location of "IWYU pragma: keep" (or export) // comment in the main file, so that when next InclusionDirective is // called, it will know that the next inclusion is behind the IWYU pragma. - LastPragmaKeepInMainFileLine = SM.getLineNumber( - SM.getMainFileID(), SM.getFileOffset(Range.getBegin())); + LastPragmaKeepInMainFileLine = CommentLine; } return false; } private: + StringRef save(llvm::StringRef S) { return UniqueStrings.save(S); } + bool InMainFile = false; const SourceManager &SM; PragmaIncludes *Out; + llvm::BumpPtrAllocator Arena; + /// Intern table for strings. Contents are on the arena. + llvm::StringSaver UniqueStrings; // Track the last line "IWYU pragma: keep" was seen in the main file, 1-based. int LastPragmaKeepInMainFileLine = -1; + struct ExportPragma { + // The line number where we saw the begin_exports or export pragma. + int SeenAtLine = 0; // 1-based line number. + // The file where we saw the pragma. + FileID SeenAtFile; + // FullPath of the file SeenAtFile. + StringRef FullPath; + // true if it is a block begin/end_exports pragma; false if it is a + // single-line export pragma. + bool Block = false; + }; + // A stack for tracking all open begin_exports or single-line export. + std::vector ExportStack; }; void PragmaIncludes::record(const CompilerInstance &CI) { @@ -203,6 +272,21 @@ return It->getSecond(); } +llvm::SmallVector +PragmaIncludes::getExporters(const FileEntry *File, FileManager &FM) const { + auto It = IWYUExportBy.find(File->getUniqueID()); + if (It == IWYUExportBy.end()) + return {}; + + llvm::SmallVector Results; + for (auto Export : It->getSecond()) { + // FIMXE: log the failing cases? + if (auto FE = expectedToOptional(FM.getFileRef(Export))) + Results.push_back(*FE); + } + return Results; +} + std::unique_ptr RecordedAST::record() { class Recorder : public ASTConsumer { RecordedAST *Out; diff --git a/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp b/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp --- a/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp +++ b/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp @@ -12,6 +12,7 @@ #include "clang/Testing/TestAST.h" #include "clang/Tooling/Inclusions/StandardLibrary.h" #include "llvm/Support/VirtualFileSystem.h" +#include "llvm/ADT/ArrayRef.h" #include "llvm/Support/raw_ostream.h" #include "llvm/Testing/Support/Annotations.h" #include "gmock/gmock.h" @@ -36,6 +37,13 @@ return false; } +MATCHER_P(FileNamed, N, "") { + if (arg->tryGetRealPathName() == N) + return true; + *result_listener << arg->tryGetRealPathName().str(); + return false; +} + class RecordASTTest : public ::testing::Test { protected: TestInputs Inputs; @@ -257,22 +265,43 @@ } TestAST build() { return TestAST(Inputs); } + + void createEmptyFiles(llvm::ArrayRef FileNames) { + for (llvm::StringRef File : FileNames) + Inputs.ExtraFiles[File] = ""; + } }; TEST_F(PragmaIncludeTest, IWYUKeep) { Inputs.Code = R"cpp(// Line 1 #include "keep1.h" // IWYU pragma: keep #include "keep2.h" /* IWYU pragma: keep */ - #include "normal.h" + + #include "export1.h" // IWYU pragma: export // line 5 + // IWYU pragma: begin_exports + #include "export2.h" // Line 7 + #include "export3.h" + // IWYU pragma: end_exports + + #include "normal.h" // Line 11 )cpp"; - Inputs.ExtraFiles["keep1.h"] = Inputs.ExtraFiles["keep2.h"] = - Inputs.ExtraFiles["normal.h"] = ""; + createEmptyFiles({"keep1.h", "keep2.h", "export1.h", "export2.h", "export3.h", + "normal.h"}); TestAST Processed = build(); EXPECT_FALSE(PI.shouldKeep(1)); + // Keep EXPECT_TRUE(PI.shouldKeep(2)); EXPECT_TRUE(PI.shouldKeep(3)); - EXPECT_FALSE(PI.shouldKeep(4)); + + // Exports + EXPECT_TRUE(PI.shouldKeep(5)); + EXPECT_TRUE(PI.shouldKeep(7)); + EXPECT_TRUE(PI.shouldKeep(8)); + EXPECT_FALSE(PI.shouldKeep(6)); // no # directive + EXPECT_FALSE(PI.shouldKeep(9)); // no # directive + + EXPECT_FALSE(PI.shouldKeep(11)); } TEST_F(PragmaIncludeTest, IWYUPrivate) { @@ -293,5 +322,73 @@ EXPECT_EQ(PI.getPublic(PublicFE.get()), ""); // no mapping. } +TEST_F(PragmaIncludeTest, IWYUExport) { + Inputs.Code = R"cpp(// Line 1 + #include "export1.h" + #include "export2.h" + )cpp"; + Inputs.ExtraFiles["export1.h"] = R"cpp( + #include "private.h" // IWYU pragma: export + )cpp"; + Inputs.ExtraFiles["export2.h"] = R"cpp( + #include "export3.h" + )cpp"; + Inputs.ExtraFiles["export3.h"] = R"cpp( + #include "private.h" // IWYU pragma: export + )cpp"; + Inputs.ExtraFiles["private.h"] = ""; + TestAST Processed = build(); + const auto &SM = Processed.sourceManager(); + auto &FM = Processed.fileManager(); + + EXPECT_THAT(PI.getExporters(FM.getFile("private.h").get(), FM), + testing::UnorderedElementsAre(FileNamed("export1.h"), + FileNamed("export3.h"))); + + EXPECT_TRUE(PI.getExporters(FM.getFile("export1.h").get(), FM).empty()); + EXPECT_TRUE(PI.getExporters(FM.getFile("export2.h").get(), FM).empty()); + EXPECT_TRUE(PI.getExporters(FM.getFile("export3.h").get(), FM).empty()); + EXPECT_TRUE( + PI.getExporters(SM.getFileEntryForID(SM.getMainFileID()), FM).empty()); +} + +TEST_F(PragmaIncludeTest, IWYUExportBlock) { + Inputs.Code = R"cpp(// Line 1 + #include "normal.h" + )cpp"; + Inputs.ExtraFiles["normal.h"] = R"cpp( + #include "foo.h" + + // IWYU pragma: begin_exports + #include "export1.h" + #include "private1.h" + // IWYU pragma: end_exports + )cpp"; + Inputs.ExtraFiles["export1.h"] = R"cpp( + // IWYU pragma: begin_exports + #include "private1.h" + #include "private2.h" + // IWYU pragma: end_exports + + #include "bar.h" + #include "private3.h" // IWYU pragma: export + )cpp"; + createEmptyFiles( + {"private1.h", "private2.h", "private3.h", "foo.h", "bar.h"}); + TestAST Processed = build(); + auto &FM = Processed.fileManager(); + + EXPECT_THAT(PI.getExporters(FM.getFile("private1.h").get(), FM), + testing::UnorderedElementsAre(FileNamed("export1.h"), + FileNamed("normal.h"))); + EXPECT_THAT(PI.getExporters(FM.getFile("private2.h").get(), FM), + testing::UnorderedElementsAre(FileNamed("export1.h"))); + EXPECT_THAT(PI.getExporters(FM.getFile("private3.h").get(), FM), + testing::UnorderedElementsAre(FileNamed("export1.h"))); + + EXPECT_TRUE(PI.getExporters(FM.getFile("foo.h").get(), FM).empty()); + EXPECT_TRUE(PI.getExporters(FM.getFile("bar.h").get(), FM).empty()); +} + } // namespace } // namespace clang::include_cleaner