[clang-tools-extra] f6307b2 - [include-cleaner] Switch Include from FileEntry* -> FileEntryRef
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Wed Jul 26 04:42:04 PDT 2023
Author: Sam McCall
Date: 2023-07-26T13:41:55+02:00
New Revision: f6307b260b0c0d93d25d13bab7ec02d762f4a2a6
URL: https://github.com/llvm/llvm-project/commit/f6307b260b0c0d93d25d13bab7ec02d762f4a2a6
DIFF: https://github.com/llvm/llvm-project/commit/f6307b260b0c0d93d25d13bab7ec02d762f4a2a6.diff
LOG: [include-cleaner] Switch Include from FileEntry* -> FileEntryRef
Unlike Header, we really do have a preferred spelling for an include: the one
that we used to open the file.
The fact that Header is still FileEntry* makes it difficult to accidentally
use path equality when we want inode equality.
Differential Revision: https://reviews.llvm.org/D155885
Added:
Modified:
clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
clang-tools-extra/include-cleaner/lib/Analysis.cpp
clang-tools-extra/include-cleaner/lib/Record.cpp
clang-tools-extra/include-cleaner/lib/Types.cpp
clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
clang-tools-extra/include-cleaner/unittests/TypesTest.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
index 064eccd9cd6678..f6292d765451c2 100644
--- a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
@@ -147,7 +147,7 @@ void IncludeCleanerCheck::check(const MatchFinder::MatchResult &Result) {
continue;
// Check if main file is the public interface for a private header. If so
// we shouldn't diagnose it as unused.
- if (auto PHeader = RecordedPI.getPublic(I.Resolved); !PHeader.empty()) {
+ if (auto PHeader = RecordedPI.getPublic(*I.Resolved); !PHeader.empty()) {
PHeader = PHeader.trim("<>\"");
// Since most private -> public mappings happen in a verbatim way, we
// check textually here. This might go wrong in presence of symlinks or
@@ -156,9 +156,10 @@ void IncludeCleanerCheck::check(const MatchFinder::MatchResult &Result) {
continue;
}
- if (llvm::none_of(IgnoreHeadersRegex,
- [Resolved = I.Resolved->tryGetRealPathName()](
- const llvm::Regex &R) { return R.match(Resolved); }))
+ if (llvm::none_of(
+ IgnoreHeadersRegex,
+ [Resolved = (*I.Resolved).getFileEntry().tryGetRealPathName()](
+ const llvm::Regex &R) { return R.match(Resolved); }))
Unused.push_back(&I);
}
diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp
index 9708c67ca2883c..78dff4dd31bc1c 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -359,10 +359,10 @@ convertIncludes(const SourceManager &SM,
SM.getComposedLoc(SM.getMainFileID(), Inc.HashOffset);
TransformedInc.Line = Inc.HashLine + 1;
TransformedInc.Angled = WrittenRef.starts_with("<");
- auto FE = SM.getFileManager().getFile(Inc.Resolved);
+ auto FE = SM.getFileManager().getFileRef(Inc.Resolved);
if (!FE) {
elog("IncludeCleaner: Failed to get an entry for resolved path {0}: {1}",
- Inc.Resolved, FE.getError().message());
+ Inc.Resolved, FE.takeError());
continue;
}
TransformedInc.Resolved = *FE;
@@ -401,7 +401,7 @@ IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST) {
}
for (auto *Inc : ConvertedIncludes.match(H)) {
Satisfied = true;
- auto HeaderID = Includes.getID(Inc->Resolved);
+ auto HeaderID = Includes.getID(&Inc->Resolved->getFileEntry());
assert(HeaderID.has_value() &&
"ConvertedIncludes only contains resolved includes.");
Used.insert(*HeaderID);
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 91dddaf9031b4a..02269cea4bdf84 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
@@ -22,6 +22,7 @@
#ifndef CLANG_INCLUDE_CLEANER_TYPES_H
#define CLANG_INCLUDE_CLEANER_TYPES_H
+#include "clang/Basic/FileEntry.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Tooling/Inclusions/StandardLibrary.h"
#include "llvm/ADT/ArrayRef.h"
@@ -40,7 +41,6 @@ class raw_ostream;
} // namespace llvm
namespace clang {
class Decl;
-class FileEntry;
class IdentifierInfo;
namespace include_cleaner {
@@ -154,8 +154,8 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Header &);
/// A single #include directive written in the main file.
struct Include {
llvm::StringRef Spelled; // e.g. vector
- const FileEntry *Resolved = nullptr; // e.g. /path/to/c++/v1/vector
- // nullptr if the header was not found
+ OptionalFileEntryRef Resolved; // e.g. /path/to/c++/v1/vector
+ // nullopt if the header was not found
SourceLocation HashLocation; // of hash in #include <vector>
unsigned Line = 0; // 1-based line number for #include
bool Angled = false; // True if spelled with <angle> quotes.
diff --git a/clang-tools-extra/include-cleaner/lib/Analysis.cpp b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
index e76929f6824ce7..888b7026436b01 100644
--- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -88,14 +88,14 @@ analyze(llvm::ArrayRef<Decl *> ASTRoots,
AnalysisResults Results;
for (const Include &I : Inc.all()) {
if (Used.contains(&I) || !I.Resolved ||
- HeaderFilter(I.Resolved->tryGetRealPathName()))
+ HeaderFilter(I.Resolved->getFileEntry().tryGetRealPathName()))
continue;
if (PI) {
if (PI->shouldKeep(I.Line))
continue;
// Check if main file is the public interface for a private header. If so
// we shouldn't diagnose it as unused.
- if (auto PHeader = PI->getPublic(I.Resolved); !PHeader.empty()) {
+ if (auto PHeader = PI->getPublic(*I.Resolved); !PHeader.empty()) {
PHeader = PHeader.trim("<>\"");
// Since most private -> public mappings happen in a verbatim way, we
// check textually here. This might go wrong in presence of symlinks or
diff --git a/clang-tools-extra/include-cleaner/lib/Record.cpp b/clang-tools-extra/include-cleaner/lib/Record.cpp
index dc3192b8baff34..50a15229cbe55b 100644
--- a/clang-tools-extra/include-cleaner/lib/Record.cpp
+++ b/clang-tools-extra/include-cleaner/lib/Record.cpp
@@ -47,7 +47,7 @@ class PPRecorder : public PPCallbacks {
Include I;
I.HashLocation = Hash;
- I.Resolved = File ? &File->getFileEntry() : nullptr;
+ I.Resolved = File;
I.Line = SM.getSpellingLineNumber(Hash);
I.Spelled = SpelledFilename;
I.Angled = IsAngled;
diff --git a/clang-tools-extra/include-cleaner/lib/Types.cpp b/clang-tools-extra/include-cleaner/lib/Types.cpp
index 271780bc1e76d2..bcd15920797b46 100644
--- a/clang-tools-extra/include-cleaner/lib/Types.cpp
+++ b/clang-tools-extra/include-cleaner/lib/Types.cpp
@@ -102,7 +102,7 @@ void Includes::add(const Include &I) {
BySpellingIt->second.push_back(Index);
if (I.Resolved)
- ByFile[I.Resolved].push_back(Index);
+ ByFile[&I.Resolved->getFileEntry()].push_back(Index);
ByLine[I.Line] = Index;
}
diff --git a/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp b/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
index 60d05128523fc1..9ed18f8f07d4be 100644
--- a/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
@@ -171,7 +171,7 @@ TEST_F(RecordPPTest, CapturesIncludes) {
EXPECT_EQ(H.HashLocation,
AST.sourceManager().getComposedLoc(
AST.sourceManager().getMainFileID(), MainFile.point("H")));
- EXPECT_EQ(H.Resolved, AST.fileManager().getFile("header.h").get());
+ EXPECT_EQ(H.Resolved, *AST.fileManager().getOptionalFileRef("header.h"));
EXPECT_FALSE(H.Angled);
auto &M = Recorded.Includes.all().back();
@@ -179,7 +179,7 @@ TEST_F(RecordPPTest, CapturesIncludes) {
EXPECT_EQ(M.HashLocation,
AST.sourceManager().getComposedLoc(
AST.sourceManager().getMainFileID(), MainFile.point("M")));
- EXPECT_EQ(M.Resolved, nullptr);
+ EXPECT_EQ(M.Resolved, std::nullopt);
EXPECT_TRUE(M.Angled);
}
diff --git a/clang-tools-extra/include-cleaner/unittests/TypesTest.cpp b/clang-tools-extra/include-cleaner/unittests/TypesTest.cpp
index 554de211ac0e6e..a2aec2eaf316b9 100644
--- a/clang-tools-extra/include-cleaner/unittests/TypesTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/TypesTest.cpp
@@ -26,8 +26,8 @@ TEST(RecordedIncludesTest, Match) {
// 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{});
+ FileEntryRef A = FM.getVirtualFileRef("/path/a", /*Size=*/0, time_t{});
+ FileEntryRef B = FM.getVirtualFileRef("/path/b", /*Size=*/0, time_t{});
Includes Inc;
Inc.add(Include{"a", A, SourceLocation(), 1});
@@ -35,10 +35,11 @@ TEST(RecordedIncludesTest, Match) {
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});
+ Inc.add(Include{"missing", std::nullopt, 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(&A.getFileEntry()), ElementsAre(line(1), line(2)));
+ EXPECT_THAT(Inc.match(&B.getFileEntry()),
+ ElementsAre(line(3), line(4), line(5)));
EXPECT_THAT(Inc.match(*tooling::stdlib::Header::named("<vector>")),
ElementsAre(line(4), line(5)));
}
More information about the cfe-commits
mailing list