Skip to content

[clang] NFCI: Use FileEntryRef for FileID creation #67838

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 3, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -152,8 +152,8 @@ int main(int argc, const char **argv) {
for (auto I = ChangedFiles.begin(), E = ChangedFiles.end(); I != E; ++I) {
OS << " {\n";
OS << " \"FilePath\": \"" << *I << "\",\n";
const auto Entry = FileMgr.getFile(*I);
auto ID = Sources.getOrCreateFileID(*Entry, SrcMgr::C_User);
auto Entry = llvm::cantFail(FileMgr.getFileRef(*I));
auto ID = Sources.getOrCreateFileID(Entry, SrcMgr::C_User);
std::string Content;
llvm::raw_string_ostream ContentStream(Content);
Rewrite.getEditBuffer(ID).write(ContentStream);
@@ -170,9 +170,9 @@ int main(int argc, const char **argv) {
}

for (const auto &File : ChangedFiles) {
const auto Entry = FileMgr.getFile(File);
auto Entry = llvm::cantFail(FileMgr.getFileRef(File));

auto ID = Sources.getOrCreateFileID(*Entry, SrcMgr::C_User);
auto ID = Sources.getOrCreateFileID(Entry, SrcMgr::C_User);
outs() << "============== " << File << " ==============\n";
Rewrite.getEditBuffer(ID).write(llvm::outs());
outs() << "\n============================================\n";
2 changes: 1 addition & 1 deletion clang-tools-extra/clang-move/Move.cpp
Original file line number Diff line number Diff line change
@@ -843,7 +843,7 @@ void ClangMoveTool::moveDeclsToNewFiles() {
// Move all contents from OldFile to NewFile.
void ClangMoveTool::moveAll(SourceManager &SM, StringRef OldFile,
StringRef NewFile) {
auto FE = SM.getFileManager().getFile(makeAbsolutePath(OldFile));
auto FE = SM.getFileManager().getOptionalFileRef(makeAbsolutePath(OldFile));
if (!FE) {
llvm::errs() << "Failed to get file: " << OldFile << "\n";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommend getFileRef and then print the error message as well if it fails.

return;
Original file line number Diff line number Diff line change
@@ -84,8 +84,8 @@ int main(int argc, const char **argv) {
Tool.applyAllReplacements(Rewrite);

for (const auto &File : Files) {
auto Entry = FileMgr.getFile(File);
const auto ID = Sources.getOrCreateFileID(*Entry, SrcMgr::C_User);
auto Entry = llvm::cantFail(FileMgr.getFileRef(File));
const auto ID = Sources.getOrCreateFileID(Entry, SrcMgr::C_User);
Rewrite.getEditBuffer(ID).write(outs());
}

2 changes: 1 addition & 1 deletion clang-tools-extra/clang-tidy/ClangTidy.cpp
Original file line number Diff line number Diff line change
@@ -243,7 +243,7 @@ class ErrorReporter {
if (FilePath.empty())
return {};

auto File = SourceMgr.getFileManager().getFile(FilePath);
auto File = SourceMgr.getFileManager().getOptionalFileRef(FilePath);
if (!File)
return {};

6 changes: 3 additions & 3 deletions clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
Original file line number Diff line number Diff line change
@@ -194,9 +194,9 @@ DiagnosticBuilder ClangTidyContext::diag(

DiagnosticBuilder ClangTidyContext::diag(const tooling::Diagnostic &Error) {
SourceManager &SM = DiagEngine->getSourceManager();
llvm::ErrorOr<const FileEntry *> File =
SM.getFileManager().getFile(Error.Message.FilePath);
FileID ID = SM.getOrCreateFileID(*File, SrcMgr::C_User);
FileManager &FM = SM.getFileManager();
FileEntryRef File = llvm::cantFail(FM.getFileRef(Error.Message.FilePath));
FileID ID = SM.getOrCreateFileID(File, SrcMgr::C_User);
SourceLocation FileStartLoc = SM.getLocForStartOfFile(ID);
SourceLocation Loc = FileStartLoc.getLocWithOffset(
static_cast<SourceLocation::IntTy>(Error.Message.FileOffset));
9 changes: 1 addition & 8 deletions clang/include/clang/Basic/SourceManager.h
Original file line number Diff line number Diff line change
@@ -876,13 +876,6 @@ class SourceManager : public RefCountedBase<SourceManager> {

/// Create a new FileID that represents the specified file
/// being \#included from the specified IncludePosition.
///
/// This translates NULL into standard input.
FileID createFileID(const FileEntry *SourceFile, SourceLocation IncludePos,
SrcMgr::CharacteristicKind FileCharacter,
int LoadedID = 0,
SourceLocation::UIntTy LoadedOffset = 0);

FileID createFileID(FileEntryRef SourceFile, SourceLocation IncludePos,
SrcMgr::CharacteristicKind FileCharacter,
int LoadedID = 0,
@@ -908,7 +901,7 @@ class SourceManager : public RefCountedBase<SourceManager> {

/// Get the FileID for \p SourceFile if it exists. Otherwise, create a
/// new FileID for the \p SourceFile.
FileID getOrCreateFileID(const FileEntry *SourceFile,
FileID getOrCreateFileID(FileEntryRef SourceFile,
SrcMgr::CharacteristicKind FileCharacter);

/// Creates an expansion SLocEntry for the substitution of an argument into a
18 changes: 4 additions & 14 deletions clang/lib/Basic/SourceManager.cpp
Original file line number Diff line number Diff line change
@@ -527,17 +527,6 @@ FileID SourceManager::getNextFileID(FileID FID) const {

/// Create a new FileID that represents the specified file
/// being \#included from the specified IncludePosition.
///
/// This translates NULL into standard input.
FileID SourceManager::createFileID(const FileEntry *SourceFile,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were there no callers that could pass NULL?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not that I could find.

SourceLocation IncludePos,
SrcMgr::CharacteristicKind FileCharacter,
int LoadedID,
SourceLocation::UIntTy LoadedOffset) {
return createFileID(SourceFile->getLastRef(), IncludePos, FileCharacter,
LoadedID, LoadedOffset);
}

FileID SourceManager::createFileID(FileEntryRef SourceFile,
SourceLocation IncludePos,
SrcMgr::CharacteristicKind FileCharacter,
@@ -585,7 +574,7 @@ FileID SourceManager::createFileID(const llvm::MemoryBufferRef &Buffer,
/// Get the FileID for \p SourceFile if it exists. Otherwise, create a
/// new FileID for the \p SourceFile.
FileID
SourceManager::getOrCreateFileID(const FileEntry *SourceFile,
SourceManager::getOrCreateFileID(FileEntryRef SourceFile,
SrcMgr::CharacteristicKind FileCharacter) {
FileID ID = translateFile(SourceFile);
return ID.isValid() ? ID : createFileID(SourceFile, SourceLocation(),
@@ -2375,8 +2364,9 @@ SourceManagerForFile::SourceManagerForFile(StringRef FileName,
IntrusiveRefCntPtr<DiagnosticIDs>(new DiagnosticIDs),
new DiagnosticOptions);
SourceMgr = std::make_unique<SourceManager>(*Diagnostics, *FileMgr);
FileID ID = SourceMgr->createFileID(*FileMgr->getFile(FileName),
SourceLocation(), clang::SrcMgr::C_User);
FileEntryRef FE = llvm::cantFail(FileMgr->getFileRef(FileName));
FileID ID =
SourceMgr->createFileID(FE, SourceLocation(), clang::SrcMgr::C_User);
assert(ID.isValid());
SourceMgr->setMainFileID(ID);
}
2 changes: 1 addition & 1 deletion clang/lib/Tooling/Core/Replacement.cpp
Original file line number Diff line number Diff line change
@@ -67,7 +67,7 @@ bool Replacement::isApplicable() const {

bool Replacement::apply(Rewriter &Rewrite) const {
SourceManager &SM = Rewrite.getSourceMgr();
auto Entry = SM.getFileManager().getFile(FilePath);
auto Entry = SM.getFileManager().getOptionalFileRef(FilePath);
if (!Entry)
return false;

5 changes: 1 addition & 4 deletions clang/lib/Tooling/Refactoring.cpp
Original file line number Diff line number Diff line change
@@ -78,10 +78,7 @@ bool formatAndApplyAllReplacements(
const std::string &FilePath = FileAndReplaces.first;
auto &CurReplaces = FileAndReplaces.second;

const FileEntry *Entry = nullptr;
if (auto File = Files.getFile(FilePath))
Entry = *File;

FileEntryRef Entry = llvm::cantFail(Files.getFileRef(FilePath));
FileID ID = SM.getOrCreateFileID(Entry, SrcMgr::C_User);
StringRef Code = SM.getBufferData(ID);

2 changes: 1 addition & 1 deletion clang/tools/clang-rename/ClangRename.cpp
Original file line number Diff line number Diff line change
@@ -228,7 +228,7 @@ int main(int argc, const char **argv) {

Tool.applyAllReplacements(Rewrite);
for (const auto &File : Files) {
auto Entry = FileMgr.getFile(File);
auto Entry = FileMgr.getOptionalFileRef(File);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another place we could use getFileRef and print the error message.

if (!Entry) {
errs() << "clang-rename: " << File << " does not exist.\n";
return 1;
2 changes: 1 addition & 1 deletion clang/unittests/Analysis/UnsafeBufferUsageTest.cpp
Original file line number Diff line number Diff line change
@@ -25,7 +25,7 @@ class UnsafeBufferUsageTest : public ::testing::Test {
} // namespace

TEST_F(UnsafeBufferUsageTest, FixItHintsConflict) {
const FileEntry *DummyFile = FileMgr.getVirtualFile("<virtual>", 100, 0);
FileEntryRef DummyFile = FileMgr.getVirtualFileRef("<virtual>", 100, 0);
FileID DummyFileID = SourceMgr.getOrCreateFileID(DummyFile, SrcMgr::C_User);
SourceLocation StartLoc = SourceMgr.getLocForStartOfFile(DummyFileID);