diff --git a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h --- a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h +++ b/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 ASTRoots, UsedSymbolCB CB); +void walkUsed(llvm::ArrayRef ASTRoots, const PragmaIncludes &PI, + UsedSymbolCB CB); } // namespace include_cleaner } // namespace clang diff --git a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h --- a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h +++ b/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 #include @@ -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 Storage; + std::variant Storage; }; } // namespace include_cleaner diff --git a/clang-tools-extra/include-cleaner/lib/Analysis.cpp b/clang-tools-extra/include-cleaner/lib/Analysis.cpp --- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp +++ b/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
-toHeader(llvm::ArrayRef Headers) { - llvm::SmallVector
Result; - llvm::for_each(Headers, [&](tooling::stdlib::Header H) { - Result.emplace_back(Header(H)); - }); - return Result; -} -} // namespace -void walkUsed(llvm::ArrayRef ASTRoots, UsedSymbolCB CB) { +void walkUsed(llvm::ArrayRef 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
findIncludeHeaders(const SymbolLocation &SLoc, + const SourceManager &SM, + const PragmaIncludes &PI) { + llvm::SmallVector
Results; + if (auto *Loc = std::get_if(&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(&SLoc)) { + for (const auto &H : Sym->headers()) + Results.push_back(H); + return Results; + } + llvm_unreachable("unhandled SymbolLocation kind!"); +} + } // namespace clang::include_cleaner diff --git a/clang-tools-extra/include-cleaner/lib/AnalysisInternal.h b/clang-tools-extra/include-cleaner/lib/AnalysisInternal.h --- a/clang-tools-extra/include-cleaner/lib/AnalysisInternal.h +++ b/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); +/// 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; + +/// Finds the headers that provide the symbol location. +// FIXME: expose signals +llvm::SmallVector
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 Roots, ASTContext &Ctx, 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 @@ -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(&PI); + }; + } + + llvm::DenseMap> walk(TestAST &AST) { + llvm::SmallVector TopLevelDecls; + for (Decl *D : AST.context().getTranslationUnitDecl()->decls()) + TopLevelDecls.emplace_back(D); + auto &SM = AST.sourceManager(); + + llvm::DenseMap> OffsetToProviders; + walkUsed( + TopLevelDecls, PI, + [&](SourceLocation RefLoc, Symbol S, llvm::ArrayRef
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 TopLevelDecls; - for (Decl *D : AST.context().getTranslationUnitDecl()->decls()) { - TopLevelDecls.emplace_back(D); - } - - auto &SM = AST.sourceManager(); - llvm::DenseMap> OffsetToProviders; - walkUsed(TopLevelDecls, [&](SourceLocation RefLoc, Symbol S, - llvm::ArrayRef
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("").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(&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