[PATCH] D66705: FileManager: Use llvm::Expected in new getFileRef API

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith created this revision.
dexonsmith added a reviewer: arphaman.
Herald added a subscriber: ributzka.
dexonsmith added a comment.

@arphaman, is there a reason you think `ErrorOr` is more appropriate long-term here?


`FileManager::getFileRef` is a modern API which we expect to convert to
over time.  We should modernize the error handling as well, using
`llvm::Expected` instead of `llvm::ErrorOr`.

There's no functionality change intended here; it looks like all the
callers were already checking the error case.


https://reviews.llvm.org/D66705

Files:
  clang/include/clang/Basic/FileManager.h
  clang/lib/Basic/FileManager.cpp
  clang/lib/Lex/HeaderSearch.cpp


Index: clang/lib/Lex/HeaderSearch.cpp
===================================================================
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -314,7 +314,7 @@
   if (!File) {
     // For rare, surprising errors (e.g. "out of file handles"), diag the EC
     // message.
-    std::error_code EC = File.getError();
+    std::error_code EC = llvm::errorToErrorCode(File.takeError());
     if (EC != llvm::errc::no_such_file_or_directory &&
         EC != llvm::errc::invalid_argument &&
         EC != llvm::errc::is_a_directory && EC != llvm::errc::not_a_directory) {
@@ -553,9 +553,7 @@
 
   FrameworkName.append(Filename.begin()+SlashPos+1, Filename.end());
 
-  llvm::ErrorOr<FileEntryRef> File =
-      FileMgr.getFileRef(FrameworkName, /*OpenFile=*/!SuggestedModule);
-
+  auto File = FileMgr.getFileRef(FrameworkName, /*OpenFile=*/!SuggestedModule);
   if (!File) {
     // Check "/System/Library/Frameworks/Cocoa.framework/PrivateHeaders/file.h"
     const char *Private = "Private";
@@ -1076,9 +1074,7 @@
   }
 
   HeadersFilename.append(Filename.begin()+SlashPos+1, Filename.end());
-  llvm::ErrorOr<FileEntryRef> File =
-      FileMgr.getFileRef(HeadersFilename, /*OpenFile=*/true);
-
+  auto File = FileMgr.getFileRef(HeadersFilename, /*OpenFile=*/true);
   if (!File) {
     // Check ".../Frameworks/HIToolbox.framework/PrivateHeaders/HIToolbox.h"
     HeadersFilename = FrameworkName;
Index: clang/lib/Basic/FileManager.cpp
===================================================================
--- clang/lib/Basic/FileManager.cpp
+++ clang/lib/Basic/FileManager.cpp
@@ -187,10 +187,10 @@
   auto Result = getFileRef(Filename, openFile, CacheFailure);
   if (Result)
     return &Result->getFileEntry();
-  return Result.getError();
+  return llvm::errorToErrorCode(Result.takeError());
 }
 
-llvm::ErrorOr<FileEntryRef>
+llvm::Expected<FileEntryRef>
 FileManager::getFileRef(StringRef Filename, bool openFile, bool CacheFailure) {
   ++NumFileLookups;
 
@@ -199,7 +199,8 @@
       SeenFileEntries.insert({Filename, std::errc::no_such_file_or_directory});
   if (!SeenFileInsertResult.second) {
     if (!SeenFileInsertResult.first->second)
-      return SeenFileInsertResult.first->second.getError();
+      return llvm::errorCodeToError(
+          SeenFileInsertResult.first->second.getError());
     // Construct and return and FileEntryRef, unless it's a redirect to another
     // filename.
     SeenFileEntryOrRedirect Value = *SeenFileInsertResult.first->second;
@@ -230,7 +231,7 @@
     else
       SeenFileEntries.erase(Filename);
 
-    return DirInfoOrErr.getError();
+    return llvm::errorCodeToError(DirInfoOrErr.getError());
   }
   const DirectoryEntry *DirInfo = *DirInfoOrErr;
 
@@ -249,7 +250,7 @@
     else
       SeenFileEntries.erase(Filename);
 
-    return statError;
+    return llvm::errorCodeToError(statError);
   }
 
   assert((openFile || !F) && "undesired open file");
Index: clang/include/clang/Basic/FileManager.h
===================================================================
--- clang/include/clang/Basic/FileManager.h
+++ clang/include/clang/Basic/FileManager.h
@@ -284,9 +284,9 @@
   ///
   /// \param CacheFailure If true and the file does not exist, we'll cache
   /// the failure to find this file.
-  llvm::ErrorOr<FileEntryRef> getFileRef(StringRef Filename,
-                                         bool OpenFile = false,
-                                         bool CacheFailure = true);
+  llvm::Expected<FileEntryRef> getFileRef(StringRef Filename,
+                                          bool OpenFile = false,
+                                          bool CacheFailure = true);
 
   /// Returns the current file system options
   FileSystemOptions &getFileSystemOpts() { return FileSystemOpts; }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D66705.217022.patch
Type: text/x-patch
Size: 3804 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190824/27c2072a/attachment-0001.bin>