From: Duncan P. N. Exon Smith Date: Mon, 26 Aug 2019 18:29:51 +0000 (+0000) Subject: FileManager: Use llvm::Expected in new getFileRef API X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=c2b43094158a8d126ba0c9a1b9f22a160ccde194;p=clang FileManager: Use llvm::Expected in new getFileRef API `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`, to help clients that care about errors to ensure nothing is missed. However, not all clients care. I've also added another path for those that don't: - `FileEntryRef` is now copy- and move-assignable (using a pointer instead of a reference). - `FileManager::getOptionalFileRef` returns an `llvm::Optional` instead of `llvm::Expected`. - Added an `llvm::expectedToOptional` utility in case this is useful elsewhere. https://reviews.llvm.org/D66705 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@369943 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/FileManager.h b/include/clang/Basic/FileManager.h index b22afec54e..9e54669efc 100644 --- a/include/clang/Basic/FileManager.h +++ b/include/clang/Basic/FileManager.h @@ -110,26 +110,27 @@ public: /// accessed by the FileManager's client. class FileEntryRef { public: + FileEntryRef() = delete; FileEntryRef(StringRef Name, const FileEntry &Entry) - : Name(Name), Entry(Entry) {} + : Name(Name), Entry(&Entry) {} const StringRef getName() const { return Name; } - const FileEntry &getFileEntry() const { return Entry; } + const FileEntry &getFileEntry() const { return *Entry; } - off_t getSize() const { return Entry.getSize(); } + off_t getSize() const { return Entry->getSize(); } - unsigned getUID() const { return Entry.getUID(); } + unsigned getUID() const { return Entry->getUID(); } const llvm::sys::fs::UniqueID &getUniqueID() const { - return Entry.getUniqueID(); + return Entry->getUniqueID(); } - time_t getModificationTime() const { return Entry.getModificationTime(); } + time_t getModificationTime() const { return Entry->getModificationTime(); } private: StringRef Name; - const FileEntry &Entry; + const FileEntry *Entry; }; /// Implements support for file system lookup, file system caching, @@ -284,9 +285,17 @@ public: /// /// \param CacheFailure If true and the file does not exist, we'll cache /// the failure to find this file. - llvm::ErrorOr getFileRef(StringRef Filename, - bool OpenFile = false, - bool CacheFailure = true); + llvm::Expected getFileRef(StringRef Filename, + bool OpenFile = false, + bool CacheFailure = true); + + /// Get a FileEntryRef if it exists, without doing anything on error. + llvm::Optional getOptionalFileRef(StringRef Filename, + bool OpenFile = false, + bool CacheFailure = true) { + return llvm::expectedToOptional( + getFileRef(Filename, OpenFile, CacheFailure)); + } /// Returns the current file system options FileSystemOptions &getFileSystemOpts() { return FileSystemOpts; } diff --git a/lib/Basic/FileManager.cpp b/lib/Basic/FileManager.cpp index 649e4d2239..8e186713a9 100644 --- a/lib/Basic/FileManager.cpp +++ b/lib/Basic/FileManager.cpp @@ -187,10 +187,10 @@ FileManager::getFile(StringRef Filename, bool openFile, bool CacheFailure) { auto Result = getFileRef(Filename, openFile, CacheFailure); if (Result) return &Result->getFileEntry(); - return Result.getError(); + return llvm::errorToErrorCode(Result.takeError()); } -llvm::ErrorOr +llvm::Expected FileManager::getFileRef(StringRef Filename, bool openFile, bool CacheFailure) { ++NumFileLookups; @@ -199,7 +199,8 @@ FileManager::getFileRef(StringRef Filename, bool openFile, bool CacheFailure) { 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 @@ FileManager::getFileRef(StringRef Filename, bool openFile, bool CacheFailure) { else SeenFileEntries.erase(Filename); - return DirInfoOrErr.getError(); + return llvm::errorCodeToError(DirInfoOrErr.getError()); } const DirectoryEntry *DirInfo = *DirInfoOrErr; @@ -249,7 +250,7 @@ FileManager::getFileRef(StringRef Filename, bool openFile, bool CacheFailure) { else SeenFileEntries.erase(Filename); - return statError; + return llvm::errorCodeToError(statError); } assert((openFile || !F) && "undesired open file"); diff --git a/lib/Frontend/CompilerInstance.cpp b/lib/Frontend/CompilerInstance.cpp index f0227d0501..a7b7114f79 100644 --- a/lib/Frontend/CompilerInstance.cpp +++ b/lib/Frontend/CompilerInstance.cpp @@ -833,6 +833,8 @@ bool CompilerInstance::InitializeSourceManager( if (InputFile != "-") { auto FileOrErr = FileMgr.getFileRef(InputFile, /*OpenFile=*/true); if (!FileOrErr) { + // FIXME: include the error in the diagnostic. + consumeError(FileOrErr.takeError()); Diags.Report(diag::err_fe_error_reading) << InputFile; return false; } diff --git a/lib/Lex/HeaderMap.cpp b/lib/Lex/HeaderMap.cpp index 1c7fb1a476..d44ef29c05 100644 --- a/lib/Lex/HeaderMap.cpp +++ b/lib/Lex/HeaderMap.cpp @@ -204,9 +204,7 @@ Optional HeaderMap::LookupFile(StringRef Filename, if (Dest.empty()) return None; - if (auto File = FM.getFileRef(Dest)) - return *File; - return None; + return FM.getOptionalFileRef(Dest); } StringRef HeaderMapImpl::lookupFilename(StringRef Filename, diff --git a/lib/Lex/HeaderSearch.cpp b/lib/Lex/HeaderSearch.cpp index 84af65c30d..0160677b2e 100644 --- a/lib/Lex/HeaderSearch.cpp +++ b/lib/Lex/HeaderSearch.cpp @@ -314,7 +314,7 @@ Optional HeaderSearch::getFileAndSuggestModule( 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) { @@ -401,7 +401,7 @@ Optional DirectoryLookup::LookupFile( FixupSearchPath(); return *Result; } - } else if (auto Res = HS.getFileMgr().getFileRef(Dest)) { + } else if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) { FixupSearchPath(); return *Res; } @@ -553,9 +553,8 @@ Optional DirectoryLookup::DoFrameworkLookup( FrameworkName.append(Filename.begin()+SlashPos+1, Filename.end()); - llvm::ErrorOr File = - FileMgr.getFileRef(FrameworkName, /*OpenFile=*/!SuggestedModule); - + auto File = + FileMgr.getOptionalFileRef(FrameworkName, /*OpenFile=*/!SuggestedModule); if (!File) { // Check "/System/Library/Frameworks/Cocoa.framework/PrivateHeaders/file.h" const char *Private = "Private"; @@ -565,7 +564,8 @@ Optional DirectoryLookup::DoFrameworkLookup( SearchPath->insert(SearchPath->begin()+OrigSize, Private, Private+strlen(Private)); - File = FileMgr.getFileRef(FrameworkName, /*OpenFile=*/!SuggestedModule); + File = FileMgr.getOptionalFileRef(FrameworkName, + /*OpenFile=*/!SuggestedModule); } // If we found the header and are allowed to suggest a module, do so now. @@ -1076,9 +1076,7 @@ Optional HeaderSearch::LookupSubframeworkHeader( } HeadersFilename.append(Filename.begin()+SlashPos+1, Filename.end()); - llvm::ErrorOr File = - FileMgr.getFileRef(HeadersFilename, /*OpenFile=*/true); - + auto File = FileMgr.getOptionalFileRef(HeadersFilename, /*OpenFile=*/true); if (!File) { // Check ".../Frameworks/HIToolbox.framework/PrivateHeaders/HIToolbox.h" HeadersFilename = FrameworkName; @@ -1090,7 +1088,7 @@ Optional HeaderSearch::LookupSubframeworkHeader( } HeadersFilename.append(Filename.begin()+SlashPos+1, Filename.end()); - File = FileMgr.getFileRef(HeadersFilename, /*OpenFile=*/true); + File = FileMgr.getOptionalFileRef(HeadersFilename, /*OpenFile=*/true); if (!File) return None;