hokein updated this revision to Diff 473185.
hokein marked 3 inline comments as done.
hokein added a comment.
address comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137320/new/
https://reviews.llvm.org/D137320
Files:
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
clang-tools-extra/include-cleaner/lib/Analysis.cpp
clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
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
@@ -7,11 +7,14 @@
//===----------------------------------------------------------------------===//
#include "clang-include-cleaner/Analysis.h"
+#include "AnalysisInternal.h"
+#include "clang-include-cleaner/Record.h"
#include "clang-include-cleaner/Types.h"
#include "clang/AST/ASTContext.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
+#include "clang/Frontend/FrontendActions.h"
#include "clang/Testing/TestAST.h"
#include "clang/Tooling/Inclusions/StandardLibrary.h"
#include "llvm/ADT/ArrayRef.h"
@@ -26,47 +29,137 @@
using testing::Pair;
using testing::UnorderedElementsAre;
-TEST(WalkUsed, Basic) {
- // FIXME: Have a fixture for setting up tests.
- llvm::Annotations HeaderCode(R"cpp(
- void foo();
- namespace std { class vector {}; })cpp");
+class WalkUsedTest : public ::testing::Test {
+protected:
+ TestInputs Inputs;
+ PragmaIncludes PI;
+
+ WalkUsedTest() {
+ Inputs.MakeAction = [this] {
+ struct Hook : public SyntaxOnlyAction {
+ 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);
+ };
+ }
+
+ llvm::DenseMap</*offset*/ size_t, std::vector<Header>> walk(TestAST &AST) {
+ llvm::SmallVector<Decl *> TopLevelDecls;
+ for (Decl *D : AST.context().getTranslationUnitDecl()->decls())
+ TopLevelDecls.emplace_back(D);
+ auto &SM = AST.sourceManager();
+
+ llvm::DenseMap<size_t, std::vector<Header>> OffsetToProviders;
+ walkUsed(
+ TopLevelDecls, PI,
+ [&](SourceLocation RefLoc, Symbol S, llvm::ArrayRef<Header> Providers) {
+ auto [FID, Offset] = SM.getDecomposedLoc(RefLoc);
+ EXPECT_EQ(FID, SM.getMainFileID());
+ OffsetToProviders.try_emplace(Offset, Providers.vec());
+ });
+ return OffsetToProviders;
+ }
+};
+
+TEST_F(WalkUsedTest, Basic) {
llvm::Annotations Code(R"cpp(
- void $bar^bar() {
+ #include "header.h"
+ #include "private.h"
+ void $bar^bar($private^Private) {
$foo^foo();
std::$vector^vector $vconstructor^v;
}
)cpp");
- TestInputs Inputs(Code.code());
- Inputs.ExtraFiles["header.h"] = HeaderCode.code().str();
- Inputs.ExtraArgs.push_back("-include");
- Inputs.ExtraArgs.push_back("header.h");
+ Inputs.Code = Code.code();
+ Inputs.ExtraFiles["header.h"] = R"cpp(
+ void foo();
+ namespace std { class vector {}; }
+ )cpp";
+ Inputs.ExtraFiles["private.h"] = R"cpp(
+ // IWYU pragma: private, include "path/public.h"
+ class Private {};
+ )cpp";
TestAST AST(Inputs);
- llvm::SmallVector<Decl *> TopLevelDecls;
- for (Decl *D : AST.context().getTranslationUnitDecl()->decls()) {
- TopLevelDecls.emplace_back(D);
- }
-
- auto &SM = AST.sourceManager();
- llvm::DenseMap<size_t, std::vector<Header>> OffsetToProviders;
- walkUsed(TopLevelDecls, [&](SourceLocation RefLoc, Symbol S,
- llvm::ArrayRef<Header> Providers) {
- auto [FID, Offset] = SM.getDecomposedLoc(RefLoc);
- EXPECT_EQ(FID, SM.getMainFileID());
- OffsetToProviders.try_emplace(Offset, Providers.vec());
- });
auto HeaderFile = Header(AST.fileManager().getFile("header.h").get());
- auto MainFile = Header(SM.getFileEntryForID(SM.getMainFileID()));
+ auto MainFile = Header(AST.sourceManager().getFileEntryForID(
+ AST.sourceManager().getMainFileID()));
auto VectorSTL = Header(tooling::stdlib::Header::named("<vector>").value());
EXPECT_THAT(
- OffsetToProviders,
+ walk(AST),
UnorderedElementsAre(
Pair(Code.point("bar"), UnorderedElementsAre(MainFile)),
+ Pair(Code.point("private"),
+ UnorderedElementsAre(Header("\"path/public.h\""))),
Pair(Code.point("foo"), UnorderedElementsAre(HeaderFile)),
Pair(Code.point("vector"), UnorderedElementsAre(VectorSTL)),
Pair(Code.point("vconstructor"), UnorderedElementsAre(VectorSTL))));
}
+TEST(FindIncludeHeaders, IWYU) {
+ 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;
+ };
+ return std::make_unique<Hook>(&PI);
+ };
+
+ Inputs.Code = R"cpp(
+ #include "header1.h"
+ #include "header2.h"
+ )cpp";
+ Inputs.ExtraFiles["header1.h"] = R"cpp(
+ // IWYU pragma: private, include "path/public.h"
+ )cpp";
+ Inputs.ExtraFiles["header2.h"] = R"cpp(
+ #include "detail1.h" // IWYU pragma: export
+
+ // IWYU pragma: begin_exports
+ #include "detail2.h"
+ // IWYU pragma: end_exports
+
+ #include "normal.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);
+ };
+
+ EXPECT_THAT(findIncludeHeaders(SourceLocFromFile("header1.h"), SM, PI),
+ UnorderedElementsAre(Header("\"path/public.h\"")));
+
+ EXPECT_THAT(findIncludeHeaders(SourceLocFromFile("detail1.h"), SM, PI),
+ UnorderedElementsAre(Header(FM.getFile("header2.h").get()),
+ Header(FM.getFile("detail1.h").get())));
+ EXPECT_THAT(findIncludeHeaders(SourceLocFromFile("detail2.h"), SM, PI),
+ UnorderedElementsAre(Header(FM.getFile("header2.h").get()),
+ Header(FM.getFile("detail2.h").get())));
+
+ EXPECT_THAT(findIncludeHeaders(SourceLocFromFile("normal.h"), SM, PI),
+ UnorderedElementsAre(Header(FM.getFile("normal.h").get())));
+}
+
} // namespace
} // namespace clang::include_cleaner
Index: clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
===================================================================
--- clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
+++ clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
@@ -21,6 +21,8 @@
#ifndef CLANG_INCLUDE_CLEANER_ANALYSISINTERNAL_H
#define CLANG_INCLUDE_CLEANER_ANALYSISINTERNAL_H
+#include "clang-include-cleaner/Record.h"
+#include "clang-include-cleaner/Types.h"
#include "clang/Basic/SourceLocation.h"
#include "llvm/ADT/STLFunctionalExtras.h"
@@ -42,6 +44,18 @@
/// being analyzed, in order to find all references within it.
void walkAST(Decl &Root, llvm::function_ref<void(SourceLocation, NamedDecl &)>);
+/// A location where a symbol can be provided.
+/// It is either a physical file of the TU (SourceLocation) or a logical
+/// location in the standard library (stdlib::Symbol).
+// FIXME: use a real Class!
+using SymbolLocation = std::variant<SourceLocation, tooling::stdlib::Symbol>;
+
+/// Finds the headers that provide the symbol location.
+// FIXME: expose signals
+llvm::SmallVector<Header> findIncludeHeaders(const SymbolLocation &Loc,
+ const SourceManager &SM,
+ const PragmaIncludes &PI);
+
/// Write an HTML summary of the analysis to the given stream.
/// FIXME: Once analysis has a public API, this should be public too.
void writeHTMLReport(FileID File, llvm::ArrayRef<Decl *> Roots, ASTContext &Ctx,
Index: clang-tools-extra/include-cleaner/lib/Analysis.cpp
===================================================================
--- clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -7,28 +7,19 @@
//===----------------------------------------------------------------------===//
#include "clang-include-cleaner/Analysis.h"
-#include "clang-include-cleaner/Types.h"
#include "AnalysisInternal.h"
+#include "clang-include-cleaner/Record.h"
+#include "clang-include-cleaner/Types.h"
#include "clang/AST/ASTContext.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Tooling/Inclusions/StandardLibrary.h"
#include "llvm/ADT/ArrayRef.h"
-#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallVector.h"
namespace clang::include_cleaner {
-namespace {
-llvm::SmallVector<Header>
-toHeader(llvm::ArrayRef<tooling::stdlib::Header> Headers) {
- llvm::SmallVector<Header> Result;
- llvm::for_each(Headers, [&](tooling::stdlib::Header H) {
- Result.emplace_back(Header(H));
- });
- return Result;
-}
-} // namespace
-void walkUsed(llvm::ArrayRef<Decl *> ASTRoots, UsedSymbolCB CB) {
+void walkUsed(llvm::ArrayRef<Decl *> ASTRoots, const PragmaIncludes &PI,
+ UsedSymbolCB CB) {
tooling::stdlib::Recognizer Recognizer;
for (auto *Root : ASTRoots) {
auto &SM = Root->getASTContext().getSourceManager();
@@ -36,16 +27,48 @@
if (auto SS = Recognizer(&ND)) {
// FIXME: Also report forward decls from main-file, so that the caller
// can decide to insert/ignore a header.
- return CB(Loc, Symbol(*SS), toHeader(SS->headers()));
+ return CB(Loc, Symbol(*SS), findIncludeHeaders({*SS}, SM, PI));
}
// FIXME: Extract locations from redecls.
- // FIXME: Handle IWYU pragmas, non self-contained files.
// FIXME: Handle macro locations.
- if (auto *FE = SM.getFileEntryForID(SM.getFileID(ND.getLocation())))
- return CB(Loc, Symbol(ND), {Header(FE)});
+ return CB(Loc, Symbol(ND),
+ findIncludeHeaders({ND.getLocation()}, SM, PI));
});
}
// FIXME: Handle references of macros.
}
+llvm::SmallVector<Header> findIncludeHeaders(const SymbolLocation &SLoc,
+ const SourceManager &SM,
+ const PragmaIncludes &PI) {
+ llvm::SmallVector<Header> Results;
+ if (auto *Loc = std::get_if<SourceLocation>(&SLoc)) {
+ // FIXME: Handle non self-contained files.
+ FileID FID = SM.getFileID(*Loc);
+ const auto *FE = SM.getFileEntryForID(FID);
+ if (!FE)
+ return {};
+
+ // 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 {{VerbatimSpelling}};
+
+ Results = {{FE}};
+ // FIXME: compute transitive exporter headers.
+ for (const auto *Export : PI.getExporters(FE, SM.getFileManager()))
+ Results.push_back(Export);
+ return Results;
+ }
+ if (auto *Sym = std::get_if<tooling::stdlib::Symbol>(&SLoc)) {
+ for (const auto &H : Sym->headers())
+ Results.push_back(H);
+ return Results;
+ }
+ llvm_unreachable("unhandled SymbolLocation kind!");
+}
+
} // namespace clang::include_cleaner
Index: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
===================================================================
--- clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
+++ clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
@@ -22,7 +22,9 @@
#ifndef CLANG_INCLUDE_CLEANER_TYPES_H
#define CLANG_INCLUDE_CLEANER_TYPES_H
+#include "clang/Basic/FileEntry.h"
#include "clang/Tooling/Inclusions/StandardLibrary.h"
+#include "llvm/ADT/StringRef.h"
#include <memory>
#include <vector>
@@ -48,12 +50,14 @@
Header(const FileEntry *FE) : Storage(FE) {}
/// A logical file representing a stdlib header.
Header(tooling::stdlib::Header H) : Storage(H) {}
+ /// A verbatim header spelling, a string quoted with <> or "" that can be
+ /// #included directly.
+ Header(StringRef VerbatimSpelling) : Storage(VerbatimSpelling) {}
bool operator==(const Header &RHS) const { return Storage == RHS.Storage; }
private:
- // FIXME: Handle verbatim spellings.
- std::variant<const FileEntry *, tooling::stdlib::Header> Storage;
+ std::variant<const FileEntry *, tooling::stdlib::Header, StringRef> Storage;
};
} // namespace include_cleaner
Index: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
===================================================================
--- clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
+++ clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
@@ -11,6 +11,7 @@
#ifndef CLANG_INCLUDE_CLEANER_ANALYSIS_H
#define CLANG_INCLUDE_CLEANER_ANALYSIS_H
+#include "clang-include-cleaner/Record.h"
#include "clang-include-cleaner/Types.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/STLFunctionalExtras.h"
@@ -42,9 +43,8 @@
/// headers which don't match any #include in the main file
/// - to diagnose unused includes: an #include in the main file does not match
/// the headers for any referenced symbol
-/// FIXME: Take in an include structure to improve location to header mappings
-/// (e.g. IWYU pragmas).
-void walkUsed(llvm::ArrayRef<Decl *> ASTRoots, UsedSymbolCB CB);
+void walkUsed(llvm::ArrayRef<Decl *> ASTRoots, const PragmaIncludes &PI,
+ UsedSymbolCB CB);
} // namespace include_cleaner
} // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
