Reapply "[clang] Use FileError in FileManager::getFileRef, getDirectoryRef"

clang/include/clang/Basic/FileManager.h

@@ -31,6 +31,7 @@
 #include <ctime>
 #include <map>
 #include <memory>
+#include <optional>
 #include <string>
 
 namespace llvm {
@@ -134,6 +135,19 @@ class FileManager : public RefCountedBase<FileManager> {
   /// Fills the RealPathName in file entry.
   void fillRealPathName(FileEntry *UFE, llvm::StringRef FileName);
 
+  /// Implementation for getFileRef and getOptionalFileRef. Uses \c ErrorOr for
+  /// efficiency when an error will be ignored.
+  llvm::ErrorOr<FileEntryRef> getFileRefImpl(StringRef Filename, bool OpenFile,
+                                             bool CacheFailure, bool IsText);
+
+  /// Implementation for getDirectoryRef and getOptionalDirectoryRef. Uses
+  /// \c ErrorOr for efficiency when an error will be ignored.
+  llvm::ErrorOr<DirectoryEntryRef> getDirectoryRefImpl(StringRef DirName,
+                                                       bool CacheFailure);
+
+  llvm::ErrorOr<DirectoryEntryRef> getDirectoryFromFile(StringRef Filename,
+                                                        bool CacheFailure);
+
 public:
   /// Construct a file manager, optionally with a custom VFS.
   ///
@@ -169,12 +183,19 @@ class FileManager : public RefCountedBase<FileManager> {
   /// \param CacheFailure If true and the file does not exist, we'll cache
   /// the failure to find this file.
   llvm::Expected<DirectoryEntryRef> getDirectoryRef(StringRef DirName,
-                                                    bool CacheFailure = true);
+                                                    bool CacheFailure = true) {
+    auto Ref = getDirectoryRefImpl(DirName, CacheFailure);
+    if (Ref)
+      return *Ref;
+    return llvm::createFileError(DirName, Ref.getError());
+  }
 
   /// Get a \c DirectoryEntryRef if it exists, without doing anything on error.
   OptionalDirectoryEntryRef getOptionalDirectoryRef(StringRef DirName,
                                                     bool CacheFailure = true) {
-    return llvm::expectedToOptional(getDirectoryRef(DirName, CacheFailure));
+    if (auto Ref = getDirectoryRefImpl(DirName, CacheFailure))
+      return *Ref;
+    return std::nullopt;
   }
 
   /// Lookup, cache, and verify the specified file (real or virtual). Return the
@@ -194,7 +215,12 @@ class FileManager : public RefCountedBase<FileManager> {
   llvm::Expected<FileEntryRef> getFileRef(StringRef Filename,
                                           bool OpenFile = false,
                                           bool CacheFailure = true,
-                                          bool IsText = true);
+                                          bool IsText = true) {
+    auto Ref = getFileRefImpl(Filename, OpenFile, CacheFailure, IsText);
+    if (Ref)
+      return *Ref;
+    return llvm::createFileError(Filename, Ref.getError());
+  }
 
   /// Get the FileEntryRef for stdin, returning an error if stdin cannot be
   /// read.
@@ -209,8 +235,9 @@ class FileManager : public RefCountedBase<FileManager> {
                                           bool OpenFile = false,
                                           bool CacheFailure = true,
                                           bool IsText = true) {
-    return llvm::expectedToOptional(
-        getFileRef(Filename, OpenFile, CacheFailure, IsText));
+    if (auto Ref = getFileRefImpl(Filename, OpenFile, CacheFailure, IsText))
+      return *Ref;
+    return std::nullopt;
   }
 
   /// Returns the current file system options

clang/lib/Basic/FileManager.cpp

@@ -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"
@@ -84,22 +85,20 @@ void FileManager::clearStatCache() { StatCache.reset(); }
 
 /// Retrieve the directory that the given file name resides in.
 /// Filename can point to either a real file or a virtual file.
-static llvm::Expected<DirectoryEntryRef>
-getDirectoryFromFile(FileManager &FileMgr, StringRef Filename,
-                     bool CacheFailure) {
+llvm::ErrorOr<DirectoryEntryRef>
+FileManager::getDirectoryFromFile(StringRef Filename, bool CacheFailure) {
   if (Filename.empty())
-    return llvm::errorCodeToError(
-        make_error_code(std::errc::no_such_file_or_directory));
+    return 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 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.
   if (DirName.empty())
     DirName = ".";
 
-  return FileMgr.getDirectoryRef(DirName, CacheFailure);
+  return getDirectoryRefImpl(DirName, CacheFailure);
 }
 
 DirectoryEntry *&FileManager::getRealDirEntry(const llvm::vfs::Status &Status) {
@@ -162,8 +161,8 @@ void FileManager::addAncestorsAsVirtualDirs(StringRef Path) {
   addAncestorsAsVirtualDirs(OriginalDirName);
 }
 
-llvm::Expected<DirectoryEntryRef>
-FileManager::getDirectoryRef(StringRef DirName, bool CacheFailure) {
+llvm::ErrorOr<DirectoryEntryRef>
+FileManager::getDirectoryRefImpl(StringRef DirName, bool CacheFailure) {
   std::optional<std::string> DirNameStr;
   normalizeCacheKey(DirName, DirNameStr);
 
@@ -176,7 +175,7 @@ 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 SeenDirInsertResult.first->second.getError();
   }
 
   // We've not seen this before. Fill it in.
@@ -198,7 +197,7 @@ FileManager::getDirectoryRef(StringRef DirName, bool CacheFailure) {
       NamedDirEnt.second = statError;
     else
       SeenDirEntries.erase(DirName);
-    return llvm::errorCodeToError(statError);
+    return statError;
   }
 
   // It exists.
@@ -208,19 +207,18 @@ FileManager::getDirectoryRef(StringRef DirName, bool CacheFailure) {
   return DirectoryEntryRef(NamedDirEnt);
 }
 
-llvm::Expected<FileEntryRef> FileManager::getFileRef(StringRef Filename,
-                                                     bool openFile,
-                                                     bool CacheFailure,
-                                                     bool IsText) {
+llvm::ErrorOr<FileEntryRef> FileManager::getFileRefImpl(StringRef Filename,
+                                                        bool openFile,
+                                                        bool CacheFailure,
+                                                        bool IsText) {
   ++NumFileLookups;
 
   // See if there is already an entry in the map.
   auto SeenFileInsertResult =
       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 SeenFileInsertResult.first->second.getError();
     return FileEntryRef(*SeenFileInsertResult.first);
   }
 
@@ -238,15 +236,15 @@ llvm::Expected<FileEntryRef> FileManager::getFileRef(StringRef Filename,
   // subdirectory.  This will let us avoid having to waste time on known-to-fail
   // searches when we go to find sys/bar.h, because all the search directories
   // without a 'sys' subdir will get a cached failure result.
-  auto DirInfoOrErr = getDirectoryFromFile(*this, Filename, CacheFailure);
+  auto DirInfoOrErr = getDirectoryFromFile(Filename, CacheFailure);
   if (!DirInfoOrErr) { // Directory doesn't exist, file can't exist.
-    std::error_code Err = errorToErrorCode(DirInfoOrErr.takeError());
+    std::error_code Err = DirInfoOrErr.getError();
     if (CacheFailure)
       NamedFileEnt->second = Err;
     else
       SeenFileEntries.erase(Filename);
 
-    return llvm::errorCodeToError(Err);
+    return Err;
   }
   DirectoryEntryRef DirInfo = *DirInfoOrErr;
 
@@ -265,7 +263,7 @@ llvm::Expected<FileEntryRef> FileManager::getFileRef(StringRef Filename,
     else
       SeenFileEntries.erase(Filename);
 
-    return llvm::errorCodeToError(statError);
+    return statError;
   }
 
   assert((openFile || !F) && "undesired open file");
@@ -367,7 +365,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(),
@@ -410,8 +408,8 @@ FileEntryRef FileManager::getVirtualFileRef(StringRef Filename, off_t Size,
   // from a source location preprocessor directive with an empty filename as
   // an example, so we need to pretend it has a name to ensure a valid directory
   // entry can be returned.
-  auto DirInfo = expectedToOptional(getDirectoryFromFile(
-      *this, Filename.empty() ? "." : Filename, /*CacheFailure=*/true));
+  auto DirInfo = getDirectoryFromFile(Filename.empty() ? "." : Filename,
+                                      /*CacheFailure=*/true);
   assert(DirInfo &&
          "The directory of a virtual file should already be in the cache.");
 

clang/lib/Lex/HeaderSearch.cpp

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

clang/lib/Lex/ModuleMap.cpp

@@ -180,8 +180,7 @@ OptionalFileEntryRef ModuleMap::findHeader(
   SmallString<128> FullPathName(Directory->getName());
 
   auto GetFile = [&](StringRef Filename) -> OptionalFileEntryRef {
-    auto File =
-        expectedToOptional(SourceMgr.getFileManager().getFileRef(Filename));
+    auto File = SourceMgr.getFileManager().getOptionalFileRef(Filename);
     if (!File || (Header.Size && File->getSize() != *Header.Size) ||
         (Header.ModTime && File->getModificationTime() != *Header.ModTime))
       return std::nullopt;

clang/lib/Lex/PPDirectives.cpp

@@ -1195,9 +1195,8 @@ OptionalFileEntryRef Preprocessor::LookupEmbedFile(StringRef Filename,
   FileManager &FM = this->getFileManager();
   if (llvm::sys::path::is_absolute(Filename)) {
     // lookup path or immediately fail
-    llvm::Expected<FileEntryRef> ShouldBeEntry = FM.getFileRef(
-        Filename, OpenFile, /*CacheFailure=*/true, /*IsText=*/false);
-    return llvm::expectedToOptional(std::move(ShouldBeEntry));
+    return FM.getOptionalFileRef(Filename, OpenFile, /*CacheFailure=*/true,
+                                 /*IsText=*/false);
   }
 
   auto SeparateComponents = [](SmallVectorImpl<char> &LookupPath,
@@ -1224,39 +1223,36 @@ OptionalFileEntryRef Preprocessor::LookupEmbedFile(StringRef Filename,
       TmpDir = LookupFromFile->getDir().getName();
       llvm::sys::path::append(TmpDir, Filename);
       if (!TmpDir.empty()) {
-        llvm::Expected<FileEntryRef> ShouldBeEntry = FM.getFileRef(
+        OptionalFileEntryRef ShouldBeEntry = FM.getOptionalFileRef(
             TmpDir, OpenFile, /*CacheFailure=*/true, /*IsText=*/false);
         if (ShouldBeEntry)
-          return llvm::expectedToOptional(std::move(ShouldBeEntry));
-        llvm::consumeError(ShouldBeEntry.takeError());
+          return ShouldBeEntry;
       }
     }
 
     // Otherwise, do working directory lookup.
     LookupPath.clear();
-    auto MaybeWorkingDirEntry = FM.getDirectoryRef(".");
+    auto MaybeWorkingDirEntry = FM.getOptionalDirectoryRef(".");
     if (MaybeWorkingDirEntry) {
       DirectoryEntryRef WorkingDirEntry = *MaybeWorkingDirEntry;
       StringRef WorkingDir = WorkingDirEntry.getName();
       if (!WorkingDir.empty()) {
         SeparateComponents(LookupPath, WorkingDir, Filename, false);
-        llvm::Expected<FileEntryRef> ShouldBeEntry = FM.getFileRef(
+        OptionalFileEntryRef ShouldBeEntry = FM.getOptionalFileRef(
             LookupPath, OpenFile, /*CacheFailure=*/true, /*IsText=*/false);
         if (ShouldBeEntry)
-          return llvm::expectedToOptional(std::move(ShouldBeEntry));
-        llvm::consumeError(ShouldBeEntry.takeError());
+          return ShouldBeEntry;
       }
     }
   }
 
   for (const auto &Entry : PPOpts.EmbedEntries) {
     LookupPath.clear();
     SeparateComponents(LookupPath, Entry, Filename, false);
-    llvm::Expected<FileEntryRef> ShouldBeEntry = FM.getFileRef(
+    OptionalFileEntryRef ShouldBeEntry = FM.getOptionalFileRef(
         LookupPath, OpenFile, /*CacheFailure=*/true, /*IsText=*/false);
     if (ShouldBeEntry)
-      return llvm::expectedToOptional(std::move(ShouldBeEntry));
-    llvm::consumeError(ShouldBeEntry.takeError());
+      return ShouldBeEntry;
   }
   return std::nullopt;
 }

clang/unittests/Basic/FileManagerTest.cpp

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