Skip to content

[clang] Use FileError in FileManager::getFileRef, getDirectoryRef #199126

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.

Sign up for GitHub

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

Open
benlangmuir wants to merge 1 commit into
base: main
Choose a base branch
from
+84 −16
Open

[clang] Use FileError in FileManager::getFileRef, getDirectoryRef #199126

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
23 changes: 13 additions & 10 deletions clang/lib/Basic/FileManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/Statistic.h"
#include "llvm/Config/llvm-config.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/IOSandbox.h"
#include "llvm/Support/MemoryBuffer.h"
Expand Down Expand Up @@ -88,11 +89,12 @@ static llvm::Expected<DirectoryEntryRef>
getDirectoryFromFile(FileManager &FileMgr, StringRef Filename,
bool CacheFailure) {
if (Filename.empty())
return llvm::errorCodeToError(
make_error_code(std::errc::no_such_file_or_directory));
return llvm::createFileError(
Filename, make_error_code(std::errc::no_such_file_or_directory));

if (llvm::sys::path::is_separator(Filename[Filename.size() - 1]))
return llvm::errorCodeToError(make_error_code(std::errc::is_a_directory));
return llvm::createFileError(Filename,
make_error_code(std::errc::is_a_directory));

StringRef DirName = llvm::sys::path::parent_path(Filename);
// Use the current directory if file has no path component.
Expand Down Expand Up @@ -176,7 +178,8 @@ FileManager::getDirectoryRef(StringRef DirName, bool CacheFailure) {
if (!SeenDirInsertResult.second) {
if (SeenDirInsertResult.first->second)
return DirectoryEntryRef(*SeenDirInsertResult.first);
return llvm::errorCodeToError(SeenDirInsertResult.first->second.getError());
return llvm::createFileError(DirName,
SeenDirInsertResult.first->second.getError());
}

// We've not seen this before. Fill it in.
Expand All @@ -198,7 +201,7 @@ FileManager::getDirectoryRef(StringRef DirName, bool CacheFailure) {
NamedDirEnt.second = statError;
else
SeenDirEntries.erase(DirName);
return llvm::errorCodeToError(statError);
return llvm::createFileError(DirName, statError);
}

// It exists.
Expand All @@ -219,8 +222,8 @@ llvm::Expected<FileEntryRef> FileManager::getFileRef(StringRef Filename,
SeenFileEntries.insert({Filename, std::errc::no_such_file_or_directory});
if (!SeenFileInsertResult.second) {
if (!SeenFileInsertResult.first->second)
return llvm::errorCodeToError(
SeenFileInsertResult.first->second.getError());
return llvm::createFileError(
Filename, SeenFileInsertResult.first->second.getError());
return FileEntryRef(*SeenFileInsertResult.first);
}

Expand All @@ -246,7 +249,7 @@ llvm::Expected<FileEntryRef> FileManager::getFileRef(StringRef Filename,
else
SeenFileEntries.erase(Filename);

return llvm::errorCodeToError(Err);
return llvm::createFileError(Filename, Err);
}
DirectoryEntryRef DirInfo = *DirInfoOrErr;

Expand All @@ -265,7 +268,7 @@ llvm::Expected<FileEntryRef> FileManager::getFileRef(StringRef Filename,
else
SeenFileEntries.erase(Filename);

return llvm::errorCodeToError(statError);
return llvm::createFileError(Filename, statError);
}

assert((openFile || !F) && "undesired open file");
Expand Down Expand Up @@ -367,7 +370,7 @@ llvm::Expected<FileEntryRef> FileManager::getSTDIN() {
}();

if (!ContentOrError)
return llvm::errorCodeToError(ContentOrError.getError());
return llvm::createFileError("-", ContentOrError.getError());

auto Content = std::move(*ContentOrError);
STDIN = getVirtualFileRef(Content->getBufferIdentifier(),
Expand Down
8 changes: 2 additions & 6 deletions clang/lib/Lex/HeaderSearch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -907,15 +907,13 @@ void HeaderSearch::diagnoseHeaderShadowing(
const auto &IncluderAndDir = Includers[i];
SmallString<1024> TmpDir = IncluderAndDir.second.getName();
llvm::sys::path::append(TmpDir, Filename);
if (auto File = getFileMgr().getFileRef(TmpDir, false, false)) {
if (auto File = getFileMgr().getOptionalFileRef(TmpDir, false, false)) {
if (&File->getFileEntry() == *FE)
continue;
Diags.Report(IncludeLoc, diag::warn_header_shadowing)
<< Filename << (*FE).getDir().getName()
<< IncluderAndDir.second.getName();
return;
} else {
llvm::errorToErrorCode(File.takeError());
}
}
}
Expand All @@ -934,14 +932,12 @@ void HeaderSearch::diagnoseHeaderShadowing(
continue;
SmallString<1024> TmpPath = It->getName();
llvm::sys::path::append(TmpPath, Filename);
if (auto File = getFileMgr().getFileRef(TmpPath, false, false)) {
if (auto File = getFileMgr().getOptionalFileRef(TmpPath, false, false)) {
if (&File->getFileEntry() == *FE)
continue;
Diags.Report(IncludeLoc, diag::warn_header_shadowing)
<< Filename << (*FE).getDir().getName() << It->getName();
return;
} else {
llvm::errorToErrorCode(File.takeError());
}
}
}
Expand Down
69 changes: 69 additions & 0 deletions clang/unittests/Basic/FileManagerTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,75 @@ TEST_F(FileManagerTest, getFileReturnsErrorForNonexistentFile) {
std::make_error_code(std::errc::not_a_directory));
}

TEST_F(FileManagerTest, getFileRefErrorIncludesFilename) {
auto FS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
auto EmptyBuffer = llvm::MemoryBuffer::getMemBuffer("");
ASSERT_TRUE(
FS->addFileNoOwn("/MyDirectory/file", 0, EmptyBuffer->getMemBufferRef()));
FileSystemOptions Opts;
FileManager Mgr(Opts, FS);

// Build the expected message for a given filename and error code, since the
// system-provided message text (and capitalization) for std::errc values
// varies by platform.
auto ExpectedMsg = [](StringRef Name, std::errc EC) {
return ("'" + Name + "': " + std::make_error_code(EC).message()).str();
};

// Nonexistent file.
auto Missing = Mgr.getFileRef("/xyz.txt");
ASSERT_FALSE(Missing);
EXPECT_EQ(ExpectedMsg("/xyz.txt", std::errc::no_such_file_or_directory),
llvm::toString(Missing.takeError()));

// Cached failure
auto MissingAgain = Mgr.getFileRef("/xyz.txt");
ASSERT_FALSE(MissingAgain);
EXPECT_EQ(std::make_error_code(std::errc::no_such_file_or_directory),
llvm::errorToErrorCode(MissingAgain.takeError()));

// Reading a directory as a file.
auto DirAsFile = Mgr.getFileRef("/MyDirectory");
ASSERT_FALSE(DirAsFile);
EXPECT_EQ(ExpectedMsg("/MyDirectory", std::errc::is_a_directory),
llvm::toString(DirAsFile.takeError()));

auto Trailing = Mgr.getFileRef("/some/dir/");
ASSERT_FALSE(Trailing);
EXPECT_EQ(ExpectedMsg("/some/dir/", std::errc::is_a_directory),
llvm::toString(Trailing.takeError()));
}

TEST_F(FileManagerTest, getDirectoryRefErrorIncludesFilename) {
auto FS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
auto EmptyBuffer = llvm::MemoryBuffer::getMemBuffer("");
ASSERT_TRUE(FS->addFileNoOwn("/foo.cpp", 0, EmptyBuffer->getMemBufferRef()));
FileSystemOptions Opts;
FileManager Mgr(Opts, FS);

auto ExpectedMsg = [](StringRef Name, std::errc EC) {
return ("'" + Name + "': " + std::make_error_code(EC).message()).str();
};

// Nonexistent directory.
auto Missing = Mgr.getDirectoryRef("/no_such_dir");
ASSERT_FALSE(Missing);
EXPECT_EQ(ExpectedMsg("/no_such_dir", std::errc::no_such_file_or_directory),
llvm::toString(Missing.takeError()));

// Cached failure
auto MissingAgain = Mgr.getDirectoryRef("/no_such_dir");
ASSERT_FALSE(MissingAgain);
EXPECT_EQ(std::make_error_code(std::errc::no_such_file_or_directory),
llvm::errorToErrorCode(MissingAgain.takeError()));

// Reading a file as a directory.
auto FileAsDir = Mgr.getDirectoryRef("/foo.cpp");
ASSERT_FALSE(FileAsDir);
EXPECT_EQ(ExpectedMsg("/foo.cpp", std::errc::not_a_directory),
llvm::toString(FileAsDir.takeError()));
}

// The following tests apply to Unix-like system only.

#ifndef _WIN32
Expand Down