[clang-tools-extra] f3e8a11 - [include-cleaner] Add export IWYU pragma support.
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 10 06:43:08 PST 2022
Author: Haojian Wu
Date: 2022-11-10T15:39:30+01:00
New Revision: f3e8a117d2bc8d439434db5cb77b1c8a425a38c0
URL: https://github.com/llvm/llvm-project/commit/f3e8a117d2bc8d439434db5cb77b1c8a425a38c0
DIFF: https://github.com/llvm/llvm-project/commit/f3e8a117d2bc8d439434db5cb77b1c8a425a38c0.diff
LOG: [include-cleaner] Add export IWYU pragma support.
- add support to PragmaIncludes to handle IWYU export/begin_exports/end_exports
pragma;
- implement an API to retrieve the direct exporter headers;
Differential Revision: https://reviews.llvm.org/D137319
Added:
Modified:
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
clang-tools-extra/include-cleaner/lib/Record.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 f3fdeeec4faa7..6a6e339a7f43f 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
@@ -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 Decl;
class FileEntry;
class Preprocessor;
class PPCallbacks;
+class FileManager;
namespace include_cleaner {
@@ -61,6 +65,11 @@ class PragmaIncludes {
/// 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<const FileEntry *> 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 @@ class PragmaIncludes {
// !!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::sys::fs::UniqueID, std::string /*VerbatimSpelling*/>
+ llvm::DenseMap<llvm::sys::fs::UniqueID, llvm::StringRef /*VerbatimSpelling*/>
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<llvm::sys::fs::UniqueID,
+ llvm::SmallVector</*RealPathNames*/ llvm::StringRef>>
+ 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
index 752f0e3d8b739..948b14c3338f5 100644
--- a/clang-tools-extra/include-cleaner/lib/Record.cpp
+++ b/clang-tools-extra/include-cleaner/lib/Record.cpp
@@ -118,7 +118,7 @@ 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) {}
+ : SM(CI.getSourceManager()), Out(Out), UniqueStrings(Arena) {}
void FileChanged(SourceLocation Loc, FileChangeReason Reason,
SrcMgr::CharacteristicKind FileType,
@@ -126,6 +126,16 @@ class PragmaIncludes::RecordPragma : public PPCallbacks, public CommentHandler {
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 @@ class PragmaIncludes::RecordPragma : public PPCallbacks, public CommentHandler {
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 @@ class PragmaIncludes::RecordPragma : public PPCallbacks, public CommentHandler {
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 @@ class PragmaIncludes::RecordPragma : public PPCallbacks, public CommentHandler {
// 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<ExportPragma> ExportStack;
};
void PragmaIncludes::record(const CompilerInstance &CI) {
@@ -203,6 +272,21 @@ llvm::StringRef PragmaIncludes::getPublic(const FileEntry *F) const {
return It->getSecond();
}
+llvm::SmallVector<const FileEntry *>
+PragmaIncludes::getExporters(const FileEntry *File, FileManager &FM) const {
+ auto It = IWYUExportBy.find(File->getUniqueID());
+ if (It == IWYUExportBy.end())
+ return {};
+
+ llvm::SmallVector<const FileEntry *> 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<ASTConsumer> 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
index eafbef8017193..7b8d1c9ab2c46 100644
--- 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 @@ MATCHER_P(named, N, "") {
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 @@ class PragmaIncludeTest : public ::testing::Test {
}
TestAST build() { return TestAST(Inputs); }
+
+ void createEmptyFiles(llvm::ArrayRef<StringRef> 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 @@ TEST_F(PragmaIncludeTest, IWYUPrivate) {
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
More information about the cfe-commits
mailing list