sammccall created this revision. sammccall added a reviewer: kadircet. Herald added a project: All. sammccall requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits.
Requiring everything that wants to match Includes to depend on Record is weird. This isn't lightweight enough that it feels perfect in Types, could be its own header instead. But pragmatically it doesn't add bad deps, and is widely used. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D139014 Files: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h clang-tools-extra/include-cleaner/lib/AnalysisInternal.h clang-tools-extra/include-cleaner/lib/HTMLReport.cpp clang-tools-extra/include-cleaner/lib/Record.cpp clang-tools-extra/include-cleaner/lib/Types.cpp clang-tools-extra/include-cleaner/unittests/CMakeLists.txt clang-tools-extra/include-cleaner/unittests/RecordTest.cpp clang-tools-extra/include-cleaner/unittests/TypesTest.cpp
Index: clang-tools-extra/include-cleaner/unittests/TypesTest.cpp =================================================================== --- /dev/null +++ clang-tools-extra/include-cleaner/unittests/TypesTest.cpp @@ -0,0 +1,47 @@ +//===-- RecordTest.cpp ----------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "clang-include-cleaner/Types.h" +#include "clang/Basic/FileManager.h" +#include "clang/Tooling/Inclusions/StandardLibrary.h" +#include "llvm/ADT/IntrusiveRefCntPtr.h" +#include "llvm/Support/VirtualFileSystem.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace clang::include_cleaner { +namespace { +using testing::ElementsAre; + +// Matches an Include* on the specified line; +MATCHER_P(line, N, "") { return arg->Line == (unsigned)N; } + +TEST(RecordedIncludesTest, Match) { + // We're using synthetic data, but need a FileManager to obtain FileEntry*s. + // Ensure it doesn't do any actual IO. + auto FS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>(); + FileManager FM(FileSystemOptions{}); + const FileEntry *A = FM.getVirtualFile("/path/a", /*Size=*/0, time_t{}); + const FileEntry *B = FM.getVirtualFile("/path/b", /*Size=*/0, time_t{}); + + Includes Inc; + Inc.add(Include{"a", A, SourceLocation(), 1}); + Inc.add(Include{"a2", A, SourceLocation(), 2}); + Inc.add(Include{"b", B, SourceLocation(), 3}); + Inc.add(Include{"vector", B, SourceLocation(), 4}); + Inc.add(Include{"vector", B, SourceLocation(), 5}); + Inc.add(Include{"missing", nullptr, SourceLocation(), 6}); + + EXPECT_THAT(Inc.match(A), ElementsAre(line(1), line(2))); + EXPECT_THAT(Inc.match(B), ElementsAre(line(3), line(4), line(5))); + EXPECT_THAT(Inc.match(*tooling::stdlib::Header::named("<vector>")), + ElementsAre(line(4), line(5))); +} + +} // namespace +} // namespace clang::include_cleaner 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 @@ -11,9 +11,7 @@ #include "clang/Frontend/FrontendAction.h" #include "clang/Frontend/FrontendActions.h" #include "clang/Testing/TestAST.h" -#include "clang/Tooling/Inclusions/StandardLibrary.h" #include "llvm/ADT/ArrayRef.h" -#include "llvm/Support/VirtualFileSystem.h" #include "llvm/Support/raw_ostream.h" #include "llvm/Testing/Support/Annotations.h" #include "gmock/gmock.h" @@ -21,7 +19,6 @@ namespace clang::include_cleaner { namespace { -using testing::ElementsAre; using testing::ElementsAreArray; using testing::IsEmpty; @@ -256,31 +253,6 @@ EXPECT_THAT(RefOffsets, ElementsAreArray(MainFile.points())); } -// Matches an Include* on the specified line; -MATCHER_P(line, N, "") { return arg->Line == (unsigned)N; } - -TEST(RecordedIncludesTest, Match) { - // We're using synthetic data, but need a FileManager to obtain FileEntry*s. - // Ensure it doesn't do any actual IO. - auto FS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>(); - FileManager FM(FileSystemOptions{}); - const FileEntry *A = FM.getVirtualFile("/path/a", /*Size=*/0, time_t{}); - const FileEntry *B = FM.getVirtualFile("/path/b", /*Size=*/0, time_t{}); - - RecordedPP::RecordedIncludes Includes; - Includes.add(Include{"a", A, SourceLocation(), 1}); - Includes.add(Include{"a2", A, SourceLocation(), 2}); - Includes.add(Include{"b", B, SourceLocation(), 3}); - Includes.add(Include{"vector", B, SourceLocation(), 4}); - Includes.add(Include{"vector", B, SourceLocation(), 5}); - Includes.add(Include{"missing", nullptr, SourceLocation(), 6}); - - EXPECT_THAT(Includes.match(A), ElementsAre(line(1), line(2))); - EXPECT_THAT(Includes.match(B), ElementsAre(line(3), line(4), line(5))); - EXPECT_THAT(Includes.match(*tooling::stdlib::Header::named("<vector>")), - ElementsAre(line(4), line(5))); -} - class PragmaIncludeTest : public ::testing::Test { protected: // We don't build an AST, we just run a preprocessor action! Index: clang-tools-extra/include-cleaner/unittests/CMakeLists.txt =================================================================== --- clang-tools-extra/include-cleaner/unittests/CMakeLists.txt +++ clang-tools-extra/include-cleaner/unittests/CMakeLists.txt @@ -8,6 +8,7 @@ AnalysisTest.cpp FindHeadersTest.cpp RecordTest.cpp + TypesTest.cpp WalkASTTest.cpp ) Index: clang-tools-extra/include-cleaner/lib/Types.cpp =================================================================== --- clang-tools-extra/include-cleaner/lib/Types.cpp +++ clang-tools-extra/include-cleaner/lib/Types.cpp @@ -64,4 +64,40 @@ llvm_unreachable("Unexpected RefType"); } +void Includes::add(const Include &I) { + unsigned Index = All.size(); + All.push_back(I); + auto BySpellingIt = BySpelling.try_emplace(I.Spelled).first; + All.back().Spelled = BySpellingIt->first(); // Now we own the backing string. + + BySpellingIt->second.push_back(Index); + if (I.Resolved) + ByFile[I.Resolved].push_back(Index); + ByLine[I.Line] = Index; +} + +const Include *Includes::atLine(unsigned OneBasedIndex) const { + auto It = ByLine.find(OneBasedIndex); + return (It == ByLine.end()) ? nullptr : &All[It->second]; +} + +llvm::SmallVector<const Include *> Includes::match(Header H) const { + llvm::SmallVector<const Include *> Result; + switch (H.kind()) { + case Header::Physical: + for (unsigned I : ByFile.lookup(H.physical())) + Result.push_back(&All[I]); + break; + case Header::Standard: + for (unsigned I : BySpelling.lookup(H.standard().name().trim("<>"))) + Result.push_back(&All[I]); + break; + case Header::Verbatim: + for (unsigned I : BySpelling.lookup(H.verbatim().trim("\"<>"))) + Result.push_back(&All[I]); + break; + } + return Result; +} + } // namespace clang::include_cleaner 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 @@ -373,44 +373,6 @@ return std::make_unique<Recorder>(this); } -void RecordedPP::RecordedIncludes::add(const Include &I) { - unsigned Index = All.size(); - All.push_back(I); - auto BySpellingIt = BySpelling.try_emplace(I.Spelled).first; - All.back().Spelled = BySpellingIt->first(); // Now we own the backing string. - - BySpellingIt->second.push_back(Index); - if (I.Resolved) - ByFile[I.Resolved].push_back(Index); - ByLine[I.Line] = Index; -} - -const Include * -RecordedPP::RecordedIncludes::atLine(unsigned OneBasedIndex) const { - auto It = ByLine.find(OneBasedIndex); - return (It == ByLine.end()) ? nullptr : &All[It->second]; -} - -llvm::SmallVector<const Include *> -RecordedPP::RecordedIncludes::match(Header H) const { - llvm::SmallVector<const Include *> Result; - switch (H.kind()) { - case Header::Physical: - for (unsigned I : ByFile.lookup(H.physical())) - Result.push_back(&All[I]); - break; - case Header::Standard: - for (unsigned I : BySpelling.lookup(H.standard().name().trim("<>"))) - Result.push_back(&All[I]); - break; - case Header::Verbatim: - for (unsigned I : BySpelling.lookup(H.verbatim().trim("\"<>"))) - Result.push_back(&All[I]); - break; - } - return Result; -} - std::unique_ptr<PPCallbacks> RecordedPP::record(const Preprocessor &PP) { return std::make_unique<PPRecorder>(*this, PP); } Index: clang-tools-extra/include-cleaner/lib/HTMLReport.cpp =================================================================== --- clang-tools-extra/include-cleaner/lib/HTMLReport.cpp +++ clang-tools-extra/include-cleaner/lib/HTMLReport.cpp @@ -135,7 +135,7 @@ const ASTContext &Ctx; const SourceManager &SM; HeaderSearch &HS; - const RecordedPP::RecordedIncludes &Includes; + const include_cleaner::Includes &Includes; const PragmaIncludes *PI; FileID MainFile; const FileEntry *MainFE; @@ -220,7 +220,7 @@ public: Reporter(llvm::raw_ostream &OS, ASTContext &Ctx, HeaderSearch &HS, - const RecordedPP::RecordedIncludes &Includes, + const include_cleaner::Includes &Includes, const PragmaIncludes *PI, FileID MainFile) : OS(OS), Ctx(Ctx), SM(Ctx.getSourceManager()), HS(HS), Includes(Includes), PI(PI), MainFile(MainFile), @@ -521,7 +521,7 @@ } // namespace -void writeHTMLReport(FileID File, const RecordedPP::RecordedIncludes &Includes, +void writeHTMLReport(FileID File, const include_cleaner::Includes &Includes, llvm::ArrayRef<Decl *> Roots, llvm::ArrayRef<SymbolReference> MacroRefs, ASTContext &Ctx, HeaderSearch &HS, PragmaIncludes *PI, 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 @@ -85,7 +85,7 @@ const PragmaIncludes *PI); /// Write an HTML summary of the analysis to the given stream. -void writeHTMLReport(FileID File, const RecordedPP::RecordedIncludes &Includes, +void writeHTMLReport(FileID File, const Includes &, llvm::ArrayRef<Decl *> Roots, llvm::ArrayRef<SymbolReference> MacroRefs, ASTContext &Ctx, HeaderSearch &HS, PragmaIncludes *PI, 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 @@ -24,6 +24,10 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Tooling/Inclusions/StandardLibrary.h" +#include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringMap.h" #include <memory> #include <vector> @@ -136,6 +140,33 @@ }; llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Include &); +/// A container for all includes present in a file. +/// Supports efficiently hit-testing Headers against Includes. +class Includes { +public: + void add(const Include &); + + /// All #includes seen, in the order they appear. + llvm::ArrayRef<Include> all() const { return All; } + + /// Determine #includes that match a header (that provides a used symbol). + /// + /// Matching is based on the type of Header specified: + /// - for a physical file like /path/to/foo.h, we check Resolved + /// - for a logical file like <vector>, we check Spelled + llvm::SmallVector<const Include *> match(Header H) const; + + /// Finds the include written on the specified line. + const Include *atLine(unsigned OneBasedIndex) const; + +private: + std::vector<Include> All; + // Lookup structures for match(), values are index into All. + llvm::StringMap<llvm::SmallVector<unsigned>> BySpelling; + llvm::DenseMap<const FileEntry *, llvm::SmallVector<unsigned>> ByFile; + llvm::DenseMap<unsigned, unsigned> ByLine; +}; + } // namespace include_cleaner } // namespace clang 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 @@ -18,11 +18,9 @@ #define CLANG_INCLUDE_CLEANER_RECORD_H #include "clang-include-cleaner/Types.h" -#include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DenseSet.h" #include "llvm/ADT/SmallVector.h" -#include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Allocator.h" #include "llvm/Support/FileSystem/UniqueID.h" @@ -133,33 +131,8 @@ /// Describes where macros were used in the main file. std::vector<SymbolReference> MacroReferences; - /// A container for all includes present in the main file. - /// Supports efficiently hit-testing Headers against Includes. - /// FIXME: is there a more natural header for this class? - class RecordedIncludes { - public: - void add(const Include &); - - /// All #includes seen, in the order they appear. - llvm::ArrayRef<Include> all() const { return All; } - - /// Determine #includes that match a header (that provides a used symbol). - /// - /// Matching is based on the type of Header specified: - /// - for a physical file like /path/to/foo.h, we check Resolved - /// - for a logical file like <vector>, we check Spelled - llvm::SmallVector<const Include *> match(Header H) const; - - /// Finds the include written on the specified line. - const Include *atLine(unsigned OneBasedIndex) const; - - private: - std::vector<Include> All; - // Lookup structures for match(), values are index into All. - llvm::StringMap<llvm::SmallVector<unsigned>> BySpelling; - llvm::DenseMap<const FileEntry *, llvm::SmallVector<unsigned>> ByFile; - llvm::DenseMap<unsigned, unsigned> ByLine; - } Includes; + /// The include directives seen in the main file. + include_cleaner::Includes Includes; }; } // namespace include_cleaner
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
