[clang-tools-extra] d3714c2 - [include-cleaner] Move RecordedPP::RecordedIncludes -> Includes in Types.h. NFC
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Wed Nov 30 09:16:07 PST 2022
Author: Sam McCall
Date: 2022-11-30T18:14:55+01:00
New Revision: d3714c2b277ac649374ada6dacf20c7a1ba9120c
URL: https://github.com/llvm/llvm-project/commit/d3714c2b277ac649374ada6dacf20c7a1ba9120c
DIFF: https://github.com/llvm/llvm-project/commit/d3714c2b277ac649374ada6dacf20c7a1ba9120c.diff
LOG: [include-cleaner] Move RecordedPP::RecordedIncludes -> Includes in Types.h. NFC
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.
Differential Revision: https://reviews.llvm.org/D139014
Added:
clang-tools-extra/include-cleaner/unittests/TypesTest.cpp
Modified:
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
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 66fd0c7915fa..a077c26a04b8 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
@@ -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 @@ struct RecordedPP {
/// 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
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
index 03742f5efe67..102f5bc21a84 100644
--- a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
+++ b/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>
@@ -138,6 +142,33 @@ struct Include {
};
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
diff --git a/clang-tools-extra/include-cleaner/lib/AnalysisInternal.h b/clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
index 7bca824137e4..29dbd58886ab 100644
--- a/clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
+++ b/clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
@@ -85,7 +85,7 @@ llvm::SmallVector<Header> findHeaders(const SymbolLocation &Loc,
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,
diff --git a/clang-tools-extra/include-cleaner/lib/HTMLReport.cpp b/clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
index ac9dfc19b3e3..4d109d5f6d20 100644
--- a/clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
+++ b/clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
@@ -135,7 +135,7 @@ class Reporter {
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 @@ class Reporter {
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 @@ class Reporter {
} // 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,
diff --git a/clang-tools-extra/include-cleaner/lib/Record.cpp b/clang-tools-extra/include-cleaner/lib/Record.cpp
index 82cb2d646aa3..e16be1221e58 100644
--- a/clang-tools-extra/include-cleaner/lib/Record.cpp
+++ b/clang-tools-extra/include-cleaner/lib/Record.cpp
@@ -374,44 +374,6 @@ std::unique_ptr<ASTConsumer> RecordedAST::record() {
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);
}
diff --git a/clang-tools-extra/include-cleaner/lib/Types.cpp b/clang-tools-extra/include-cleaner/lib/Types.cpp
index 7ea6c6942983..68907917ea64 100644
--- a/clang-tools-extra/include-cleaner/lib/Types.cpp
+++ b/clang-tools-extra/include-cleaner/lib/Types.cpp
@@ -69,4 +69,41 @@ std::string Include::quote() const {
(Angled ? ">" : "\""))
.str();
}
+
+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
diff --git a/clang-tools-extra/include-cleaner/unittests/CMakeLists.txt b/clang-tools-extra/include-cleaner/unittests/CMakeLists.txt
index e5d8f8839aa1..94640c3c2fc1 100644
--- a/clang-tools-extra/include-cleaner/unittests/CMakeLists.txt
+++ b/clang-tools-extra/include-cleaner/unittests/CMakeLists.txt
@@ -8,6 +8,7 @@ add_unittest(ClangIncludeCleanerUnitTests ClangIncludeCleanerTests
AnalysisTest.cpp
FindHeadersTest.cpp
RecordTest.cpp
+ TypesTest.cpp
WalkASTTest.cpp
)
diff --git a/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp b/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
index a10b5f43f9d0..919d6bdc0d68 100644
--- a/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
+++ b/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;
@@ -258,31 +255,6 @@ TEST_F(RecordPPTest, CapturesConditionalMacroRefs) {
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!
diff --git a/clang-tools-extra/include-cleaner/unittests/TypesTest.cpp b/clang-tools-extra/include-cleaner/unittests/TypesTest.cpp
new file mode 100644
index 000000000000..554de211ac0e
--- /dev/null
+++ b/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
More information about the cfe-commits
mailing list