[llvm] [ThinLTO] Replace `ErrorOr` with `Expected` (PR #80088)

Steven Wu via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 31 13:55:01 PST 2024


================
@@ -397,19 +397,22 @@ class ModuleCacheEntry {
   // Access the path to this entry in the cache.
   StringRef getEntryPath() { return EntryPath; }
 
-  // Try loading the buffer for this cache entry.
-  ErrorOr<std::unique_ptr<MemoryBuffer>> tryLoadingBuffer() {
-    if (EntryPath.empty())
-      return std::error_code();
+  /// Try loading the buffer for this cache entry.
+  ///
+  /// \returns The buffer on cache hit, null on cache miss, or an error when
+  /// unable to load the cache contents.
+  Expected<std::unique_ptr<MemoryBuffer>> tryLoadingBuffer() {
+    if (EntryPath.empty() || !sys::fs::exists(EntryPath))
----------------
cachemeifyoucan wrote:

This is good in general as the reported error doesn't cause any fatal error, just some logs. But on the other hand, I can see problem here as the file can be deleted between exist check and read file. The file (the cache entry) deletion can be normal from a parallel link command that finishes and decide to prune the cache.

Maybe a general error from `openNativeForRead` should just return nullptr here, but some legit errors/misuses will not be diagnosed here.

https://github.com/llvm/llvm-project/pull/80088


More information about the llvm-commits mailing list