[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
This commit is contained in:
Sam McCall 2022-11-30 16:33:50 +01:00
parent 9961fa1653
commit d3714c2b27
9 changed files with 122 additions and 99 deletions

View File

@ -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

View File

@ -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

View File

@ -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,

View File

@ -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 @@ private:
} // 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,

View File

@ -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);
}

View File

@ -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

View File

@ -8,6 +8,7 @@ add_unittest(ClangIncludeCleanerUnitTests ClangIncludeCleanerTests
AnalysisTest.cpp
FindHeadersTest.cpp
RecordTest.cpp
TypesTest.cpp
WalkASTTest.cpp
)

View File

@ -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!

View File

@ -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