Skip to content

Commit

Permalink
[clang][tools] Use FileEntryRef in include_cleaner::Header
Browse files Browse the repository at this point in the history
  • Loading branch information
jansvoboda11 committed Sep 9, 2023
1 parent 37b0889 commit 98e6deb
Show file tree
Hide file tree
Showing 15 changed files with 68 additions and 64 deletions.
4 changes: 2 additions & 2 deletions clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ bool IncludeCleanerCheck::shouldIgnore(const include_cleaner::Header &H) {
case include_cleaner::Header::Verbatim:
return R.match(H.verbatim());
case include_cleaner::Header::Physical:
return R.match(H.physical()->tryGetRealPathName());
return R.match(H.physical().getFileEntry().tryGetRealPathName());
}
llvm_unreachable("Unknown Header kind.");
});
Expand Down Expand Up @@ -145,7 +145,7 @@ void IncludeCleanerCheck::check(const MatchFinder::MatchResult &Result) {
for (const include_cleaner::Header &H : Providers) {
if (H.kind() == include_cleaner::Header::Physical &&
(H.physical() == MainFile ||
H.physical()->getDir() == ResourceDir)) {
H.physical().getDir() == ResourceDir)) {
Satisfied = true;
continue;
}
Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/clangd/IncludeCleaner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST) {
for (const auto &H : Providers) {
if (H.kind() == include_cleaner::Header::Physical &&
(H.physical() == MainFile || H.physical() == PreamblePatch ||
H.physical()->getLastRef().getDir() == ResourceDir)) {
H.physical().getDir() == ResourceDir)) {
Satisfied = true;
continue;
}
Expand Down
7 changes: 4 additions & 3 deletions clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,8 @@ TEST(IncludeCleaner, ComputeMissingHeaders) {
size_t End = llvm::cantFail(positionToOffset(MainFile.code(), Range.end));
syntax::FileRange BRange{SM.getMainFileID(), static_cast<unsigned int>(Start),
static_cast<unsigned int>(End)};
include_cleaner::Header Header{*SM.getFileManager().getFile("b.h")};
include_cleaner::Header Header{
*SM.getFileManager().getOptionalFileRef("b.h")};
MissingIncludeDiagInfo BInfo{B, BRange, {Header}};
EXPECT_THAT(Findings.MissingIncludes, ElementsAre(BInfo));
}
Expand Down Expand Up @@ -474,8 +475,8 @@ TEST(IncludeCleaner, IsPreferredProvider) {
auto &IncludeDef2 = AST.getIncludeStructure().MainFileIncludes[2];

auto &FM = AST.getSourceManager().getFileManager();
auto *DeclH = &FM.getOptionalFileRef("decl.h")->getFileEntry();
auto *DefH = &FM.getOptionalFileRef("def.h")->getFileEntry();
auto DeclH = *FM.getOptionalFileRef("decl.h");
auto DefH = *FM.getOptionalFileRef("def.h");

auto Includes = convertIncludes(AST);
std::vector<include_cleaner::Header> Providers = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,10 @@ class PragmaIncludes {

/// Returns all direct exporter headers for the given header file.
/// Returns empty if there is none.
llvm::SmallVector<const FileEntry *> getExporters(const FileEntry *File,
FileManager &FM) const;
llvm::SmallVector<const FileEntry *> getExporters(tooling::stdlib::Header,
FileManager &FM) const;
llvm::SmallVector<FileEntryRef> getExporters(const FileEntry *File,
FileManager &FM) const;
llvm::SmallVector<FileEntryRef> getExporters(tooling::stdlib::Header,
FileManager &FM) const;

/// Returns true if the given file is a self-contained file.
bool isSelfContained(const FileEntry *File) const;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,15 +122,15 @@ struct Header {
Verbatim,
};

Header(const FileEntry *FE) : Storage(FE) {}
Header(FileEntryRef FE) : Storage(FE) {}
Header(tooling::stdlib::Header H) : Storage(H) {}
Header(StringRef VerbatimSpelling) : Storage(VerbatimSpelling) {}

Kind kind() const { return static_cast<Kind>(Storage.index()); }
bool operator==(const Header &RHS) const { return Storage == RHS.Storage; }
bool operator<(const Header &RHS) const;

const FileEntry *physical() const { return std::get<Physical>(Storage); }
FileEntryRef physical() const { return std::get<Physical>(Storage); }
tooling::stdlib::Header standard() const {
return std::get<Standard>(Storage);
}
Expand All @@ -142,7 +142,7 @@ struct Header {

private:
// Order must match Kind enum!
std::variant<const FileEntry *, tooling::stdlib::Header, StringRef> Storage;
std::variant<FileEntryRef, tooling::stdlib::Header, StringRef> Storage;

// Disambiguation tag to make sure we can call the right constructor from
// DenseMapInfo methods.
Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/include-cleaner/lib/Analysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ analyze(llvm::ArrayRef<Decl *> ASTRoots,
for (const Header &H : Providers) {
if (H.kind() == Header::Physical &&
(H.physical() == MainFile ||
H.physical()->getDir() == ResourceDir)) {
H.physical().getDir() == ResourceDir)) {
Satisfied = true;
}
for (const Include *I : Inc.match(H)) {
Expand Down
26 changes: 13 additions & 13 deletions clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ llvm::StringRef basename(llvm::StringRef Header) {
bool nameMatch(llvm::StringRef DeclName, Header H) {
switch (H.kind()) {
case Header::Physical:
return basename(H.physical()->getName()).equals_insensitive(DeclName);
return basename(H.physical().getName()).equals_insensitive(DeclName);
case Header::Standard:
return basename(H.standard().name()).equals_insensitive(DeclName);
case Header::Verbatim:
Expand Down Expand Up @@ -101,7 +101,7 @@ hintedHeadersForStdHeaders(llvm::ArrayRef<tooling::stdlib::Header> Headers,
Results.emplace_back(H, Hints::PublicHeader | Hints::OriginHeader);
if (!PI)
continue;
for (const auto *Export : PI->getExporters(H, SM.getFileManager()))
for (FileEntryRef Export : PI->getExporters(H, SM.getFileManager()))
Results.emplace_back(Header(Export), isPublicHeader(Export, *PI));
}
// StandardLibrary returns headers in preference order, so only mark the
Expand Down Expand Up @@ -186,31 +186,31 @@ llvm::SmallVector<Hinted<Header>> findHeaders(const SymbolLocation &Loc,
switch (Loc.kind()) {
case SymbolLocation::Physical: {
FileID FID = SM.getFileID(SM.getExpansionLoc(Loc.physical()));
const FileEntry *FE = SM.getFileEntryForID(FID);
OptionalFileEntryRef FE = SM.getFileEntryRefForID(FID);
if (!FE)
return {};
if (!PI)
return {{FE, Hints::PublicHeader | Hints::OriginHeader}};
return {{*FE, Hints::PublicHeader | Hints::OriginHeader}};
bool IsOrigin = true;
std::queue<const FileEntry *> Exporters;
std::queue<FileEntryRef> Exporters;
while (FE) {
Results.emplace_back(FE,
isPublicHeader(FE, *PI) |
Results.emplace_back(*FE,
isPublicHeader(*FE, *PI) |
(IsOrigin ? Hints::OriginHeader : Hints::None));
for (const auto *Export : PI->getExporters(FE, SM.getFileManager()))
for (FileEntryRef Export : PI->getExporters(*FE, SM.getFileManager()))
Exporters.push(Export);

if (auto Verbatim = PI->getPublic(FE); !Verbatim.empty()) {
if (auto Verbatim = PI->getPublic(*FE); !Verbatim.empty()) {
Results.emplace_back(Verbatim,
Hints::PublicHeader | Hints::PreferredHeader);
break;
}
if (PI->isSelfContained(FE) || FID == SM.getMainFileID())
if (PI->isSelfContained(*FE) || FID == SM.getMainFileID())
break;

// Walkup the include stack for non self-contained headers.
FID = SM.getDecomposedIncludedLoc(FID).first;
FE = SM.getFileEntryForID(FID);
FE = SM.getFileEntryRefForID(FID);
IsOrigin = false;
}
// Now traverse provider trees rooted at exporters.
Expand All @@ -219,12 +219,12 @@ llvm::SmallVector<Hinted<Header>> findHeaders(const SymbolLocation &Loc,
// being exported in this header.
std::set<const FileEntry *> SeenExports;
while (!Exporters.empty()) {
auto *Export = Exporters.front();
FileEntryRef Export = Exporters.front();
Exporters.pop();
if (!SeenExports.insert(Export).second) // In case of cyclic exports
continue;
Results.emplace_back(Export, isPublicHeader(Export, *PI));
for (const auto *Export : PI->getExporters(Export, SM.getFileManager()))
for (FileEntryRef Export : PI->getExporters(Export, SM.getFileManager()))
Exporters.push(Export);
}
return Results;
Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ class Reporter {
OS << "<tr><th>Header</th><td>";
switch (H.kind()) {
case Header::Physical:
printFilename(H.physical()->getName());
printFilename(H.physical().getName());
break;
case Header::Standard:
OS << "stdlib " << H.standard().name();
Expand Down
14 changes: 7 additions & 7 deletions clang-tools-extra/include-cleaner/lib/Record.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ class PragmaIncludes::RecordPragma : public PPCallbacks, public CommentHandler {
IncludedHeader = *StandardHeader;
}
if (!IncludedHeader && File)
IncludedHeader = &File->getFileEntry();
IncludedHeader = *File;
checkForExport(HashFID, HashLine, std::move(IncludedHeader), File);
checkForKeep(HashLine, File);
}
Expand All @@ -243,7 +243,7 @@ class PragmaIncludes::RecordPragma : public PPCallbacks, public CommentHandler {
if (IncludedHeader) {
switch (IncludedHeader->kind()) {
case Header::Physical:
Out->IWYUExportBy[IncludedHeader->physical()->getUniqueID()]
Out->IWYUExportBy[IncludedHeader->physical().getUniqueID()]
.push_back(Top.Path);
break;
case Header::Standard:
Expand Down Expand Up @@ -393,26 +393,26 @@ llvm::StringRef PragmaIncludes::getPublic(const FileEntry *F) const {
return It->getSecond();
}

static llvm::SmallVector<const FileEntry *>
static llvm::SmallVector<FileEntryRef>
toFileEntries(llvm::ArrayRef<StringRef> FileNames, FileManager &FM) {
llvm::SmallVector<const FileEntry *> Results;
llvm::SmallVector<FileEntryRef> Results;

for (auto FName : FileNames) {
// FIMXE: log the failing cases?
if (auto FE = expectedToOptional(FM.getFileRef(FName)))
if (auto FE = FM.getOptionalFileRef(FName))
Results.push_back(*FE);
}
return Results;
}
llvm::SmallVector<const FileEntry *>
llvm::SmallVector<FileEntryRef>
PragmaIncludes::getExporters(const FileEntry *File, FileManager &FM) const {
auto It = IWYUExportBy.find(File->getUniqueID());
if (It == IWYUExportBy.end())
return {};

return toFileEntries(It->getSecond(), FM);
}
llvm::SmallVector<const FileEntry *>
llvm::SmallVector<FileEntryRef>
PragmaIncludes::getExporters(tooling::stdlib::Header StdHeader,
FileManager &FM) const {
auto It = StdIWYUExportBy.find(StdHeader);
Expand Down
6 changes: 3 additions & 3 deletions clang-tools-extra/include-cleaner/lib/Types.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Symbol &S) {
llvm::StringRef Header::resolvedPath() const {
switch (kind()) {
case include_cleaner::Header::Physical:
return physical()->tryGetRealPathName();
return physical().getFileEntry().tryGetRealPathName();
case include_cleaner::Header::Standard:
return standard().name().trim("<>\"");
case include_cleaner::Header::Verbatim:
Expand All @@ -60,7 +60,7 @@ llvm::StringRef Header::resolvedPath() const {
llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Header &H) {
switch (H.kind()) {
case Header::Physical:
return OS << H.physical()->getName();
return OS << H.physical().getName();
case Header::Standard:
return OS << H.standard().name();
case Header::Verbatim:
Expand Down Expand Up @@ -198,7 +198,7 @@ bool Header::operator<(const Header &RHS) const {
return kind() < RHS.kind();
switch (kind()) {
case Header::Physical:
return physical()->getName() < RHS.physical()->getName();
return physical().getName() < RHS.physical().getName();
case Header::Standard:
return standard().name() < RHS.standard().name();
case Header::Verbatim:
Expand Down
24 changes: 12 additions & 12 deletions clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,10 @@ TEST_F(WalkUsedTest, Basic) {

TestAST AST(Inputs);
auto &SM = AST.sourceManager();
auto HeaderFile = Header(AST.fileManager().getFile("header.h").get());
auto PrivateFile = Header(AST.fileManager().getFile("private.h").get());
auto HeaderFile = Header(*AST.fileManager().getOptionalFileRef("header.h"));
auto PrivateFile = Header(*AST.fileManager().getOptionalFileRef("private.h"));
auto PublicFile = Header("\"path/public.h\"");
auto MainFile = Header(SM.getFileEntryForID(SM.getMainFileID()));
auto MainFile = Header(*SM.getFileEntryRefForID(SM.getMainFileID()));
auto VectorSTL = Header(*tooling::stdlib::Header::named("<vector>"));
auto UtilitySTL = Header(*tooling::stdlib::Header::named("<utility>"));
EXPECT_THAT(
Expand Down Expand Up @@ -152,9 +152,9 @@ TEST_F(WalkUsedTest, MultipleProviders) {

TestAST AST(Inputs);
auto &SM = AST.sourceManager();
auto HeaderFile1 = Header(AST.fileManager().getFile("header1.h").get());
auto HeaderFile2 = Header(AST.fileManager().getFile("header2.h").get());
auto MainFile = Header(SM.getFileEntryForID(SM.getMainFileID()));
auto HeaderFile1 = Header(*AST.fileManager().getOptionalFileRef("header1.h"));
auto HeaderFile2 = Header(*AST.fileManager().getOptionalFileRef("header2.h"));
auto MainFile = Header(*SM.getFileEntryRefForID(SM.getMainFileID()));
EXPECT_THAT(
offsetToProviders(AST),
Contains(Pair(Code.point("foo"),
Expand All @@ -173,8 +173,8 @@ TEST_F(WalkUsedTest, MacroRefs) {
TestAST AST(Inputs);
auto &SM = AST.sourceManager();
auto &PP = AST.preprocessor();
const auto *HdrFile = SM.getFileManager().getFile("hdr.h").get();
auto MainFile = Header(SM.getFileEntryForID(SM.getMainFileID()));
auto HdrFile = *SM.getFileManager().getOptionalFileRef("hdr.h");
auto MainFile = Header(*SM.getFileEntryRefForID(SM.getMainFileID()));

auto HdrID = SM.translateFile(HdrFile);

Expand Down Expand Up @@ -490,9 +490,9 @@ TEST_F(WalkUsedTest, TemplateDecls) {
guard("template<typename T> struct Foo<T*> {};");
TestAST AST(Inputs);
auto &SM = AST.sourceManager();
const auto *Fwd = SM.getFileManager().getFile("fwd.h").get();
const auto *Def = SM.getFileManager().getFile("def.h").get();
const auto *Partial = SM.getFileManager().getFile("partial.h").get();
auto Fwd = *SM.getFileManager().getOptionalFileRef("fwd.h");
auto Def = *SM.getFileManager().getOptionalFileRef("def.h");
auto Partial = *SM.getFileManager().getOptionalFileRef("partial.h");

EXPECT_THAT(
offsetToProviders(AST),
Expand Down Expand Up @@ -524,7 +524,7 @@ TEST_F(WalkUsedTest, IgnoresIdentityMacros) {

TestAST AST(Inputs);
auto &SM = AST.sourceManager();
auto MainFile = Header(SM.getFileEntryForID(SM.getMainFileID()));
auto MainFile = Header(*SM.getFileEntryRefForID(SM.getMainFileID()));
EXPECT_THAT(offsetToProviders(AST),
UnorderedElementsAre(
// FIXME: we should have a reference from stdin to header.h
Expand Down
11 changes: 6 additions & 5 deletions clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ class FindHeadersTest : public testing::Test {
/*Line=*/1, /*Col=*/1),
AST->sourceManager(), &PI);
}
const FileEntry *physicalHeader(llvm::StringRef FileName) {
return AST->fileManager().getFile(FileName).get();
FileEntryRef physicalHeader(llvm::StringRef FileName) {
return *AST->fileManager().getOptionalFileRef(FileName);
};
};

Expand Down Expand Up @@ -409,9 +409,10 @@ TEST_F(HeadersForSymbolTest, MainFile) {
buildAST();
auto &SM = AST->sourceManager();
// FIXME: Symbols provided by main file should be treated specially.
EXPECT_THAT(headersForFoo(),
ElementsAre(physicalHeader("public_complete.h"),
Header(SM.getFileEntryForID(SM.getMainFileID()))));
EXPECT_THAT(
headersForFoo(),
ElementsAre(physicalHeader("public_complete.h"),
Header(*SM.getFileEntryRefForID(SM.getMainFileID()))));
}

TEST_F(HeadersForSymbolTest, PreferExporterOfPrivate) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ class DummyIncludeSpeller : public IncludeSpeller {
return "<bits/stdc++.h>";
if (Input.H.kind() != Header::Physical)
return "";
llvm::StringRef AbsolutePath = Input.H.physical()->tryGetRealPathName();
llvm::StringRef AbsolutePath =
Input.H.physical().getFileEntry().tryGetRealPathName();
std::string RootWithSeparator{testRoot()};
RootWithSeparator += llvm::sys::path::get_separator();
if (!AbsolutePath.consume_front(llvm::StringRef{RootWithSeparator}))
Expand All @@ -70,10 +71,12 @@ TEST(IncludeSpeller, IsRelativeToTestRoot) {
const auto *MainFile = AST.sourceManager().getFileEntryForID(
AST.sourceManager().getMainFileID());

EXPECT_EQ("\"foo.h\"", spellHeader({Header{*FM.getFile(testPath("foo.h"))},
HS, MainFile}));
EXPECT_EQ("\"foo.h\"",
spellHeader({Header{*FM.getOptionalFileRef(testPath("foo.h"))}, HS,
MainFile}));
EXPECT_EQ("<header.h>",
spellHeader({Header{*FM.getFile("dir/header.h")}, HS, MainFile}));
spellHeader({Header{*FM.getOptionalFileRef("dir/header.h")}, HS,
MainFile}));
}

TEST(IncludeSpeller, CanOverrideSystemHeaders) {
Expand Down
4 changes: 2 additions & 2 deletions clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ MATCHER_P(named, N, "") {
}

MATCHER_P(FileNamed, N, "") {
if (arg->tryGetRealPathName() == N)
if (arg.getFileEntry().tryGetRealPathName() == N)
return true;
*result_listener << arg->tryGetRealPathName().str();
*result_listener << arg.getFileEntry().tryGetRealPathName().str();
return false;
}

Expand Down
5 changes: 2 additions & 3 deletions clang-tools-extra/include-cleaner/unittests/TypesTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,8 @@ TEST(RecordedIncludesTest, Match) {
Inc.add(Include{"vector", B, SourceLocation(), 5});
Inc.add(Include{"missing", std::nullopt, SourceLocation(), 6});

EXPECT_THAT(Inc.match(&A.getFileEntry()), ElementsAre(line(1), line(2)));
EXPECT_THAT(Inc.match(&B.getFileEntry()),
ElementsAre(line(3), line(4), line(5)));
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)));
}
Expand Down

0 comments on commit 98e6deb

Please sign in to comment.