Convert FileSystem::Open() to return Expected<FileUP>

Summary:
This patch converts FileSystem::Open from this prototype:

Status
Open(File &File, const FileSpec &file_spec, ...);

to this one:

llvm::Expected<std::unique_ptr<File>>
Open(const FileSpec &file_spec, ...);

This is beneficial on its own, as llvm::Expected is a more modern
and recommended error type than Status.  It is also a necessary step
towards https://reviews.llvm.org/D67891, and further developments
for lldb_private::File.

Reviewers: JDevlieghere, jasonmolenda, labath

Reviewed By: labath

Subscribers: mgorny, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D67996

llvm-svn: 373003

8 files changed, 85 insertions, 65 deletions

diff --git a/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
index 38bd79d68cf..ad1b3411c67 100644
--- a/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
@@ -2651,14 +2651,14 @@ bool RenderScriptRuntime::SaveAllocation(Stream &strm, const uint32_t alloc_id,
   // Check we can create writable file
   FileSpec file_spec(path);
   FileSystem::Instance().Resolve(file_spec);
-  File file;
-  FileSystem::Instance().Open(file, file_spec,
-                              File::eOpenOptionWrite |
-                                  File::eOpenOptionCanCreate |
-                                  File::eOpenOptionTruncate);
+  auto file = FileSystem::Instance().Open(
+      file_spec, File::eOpenOptionWrite | File::eOpenOptionCanCreate |
+                     File::eOpenOptionTruncate);
 
   if (!file) {
-    strm.Printf("Error: Failed to open '%s' for writing", path);
+    std::string error = llvm::toString(file.takeError());
+    strm.Printf("Error: Failed to open '%s' for writing: %s", path,
+                error.c_str());
     strm.EOL();
     return false;
   }
@@ -2690,7 +2690,7 @@ bool RenderScriptRuntime::SaveAllocation(Stream &strm, const uint32_t alloc_id,
   LLDB_LOGF(log, "%s - writing File Header, 0x%" PRIx64 " bytes", __FUNCTION__,
             (uint64_t)num_bytes);
 
-  Status err = file.Write(&head, num_bytes);
+  Status err = file.get()->Write(&head, num_bytes);
   if (!err.Success()) {
     strm.Printf("Error: '%s' when writing to file '%s'", err.AsCString(), path);
     strm.EOL();
@@ -2715,7 +2715,7 @@ bool RenderScriptRuntime::SaveAllocation(Stream &strm, const uint32_t alloc_id,
   LLDB_LOGF(log, "%s - writing element headers, 0x%" PRIx64 " bytes.",
             __FUNCTION__, (uint64_t)num_bytes);
 
-  err = file.Write(element_header_buffer.get(), num_bytes);
+  err = file.get()->Write(element_header_buffer.get(), num_bytes);
   if (!err.Success()) {
     strm.Printf("Error: '%s' when writing to file '%s'", err.AsCString(), path);
     strm.EOL();
@@ -2727,7 +2727,7 @@ bool RenderScriptRuntime::SaveAllocation(Stream &strm, const uint32_t alloc_id,
   LLDB_LOGF(log, "%s - writing 0x%" PRIx64 " bytes", __FUNCTION__,
             (uint64_t)num_bytes);
 
-  err = file.Write(buffer.get(), num_bytes);
+  err = file.get()->Write(buffer.get(), num_bytes);
   if (!err.Success()) {
     strm.Printf("Error: '%s' when writing to file '%s'", err.AsCString(), path);
     strm.EOL();
@@ -4578,32 +4578,36 @@ public:
       return false;
     }
 
-    Stream *output_strm = nullptr;
-    StreamFile outfile_stream;
+    Stream *output_stream_p = nullptr;
+    std::unique_ptr<Stream> output_stream_storage;
+
     const FileSpec &outfile_spec =
         m_options.m_outfile; // Dump allocation to file instead
     if (outfile_spec) {
       // Open output file
       std::string path = outfile_spec.GetPath();
-      auto error = FileSystem::Instance().Open(
-          outfile_stream.GetFile(), outfile_spec,
-          File::eOpenOptionWrite | File::eOpenOptionCanCreate);
-      if (error.Success()) {
-        output_strm = &outfile_stream;
+      auto file = FileSystem::Instance().Open(
+          outfile_spec, File::eOpenOptionWrite | File::eOpenOptionCanCreate);
+      if (file) {
+        output_stream_storage =
+            std::make_unique<StreamFile>(std::move(file.get()));
+        output_stream_p = output_stream_storage.get();
         result.GetOutputStream().Printf("Results written to '%s'",
                                         path.c_str());
         result.GetOutputStream().EOL();
       } else {
-        result.AppendErrorWithFormat("Couldn't open file '%s'", path.c_str());
+        std::string error = llvm::toString(file.takeError());
+        result.AppendErrorWithFormat("Couldn't open file '%s': %s",
+                                     path.c_str(), error.c_str());
         result.SetStatus(eReturnStatusFailed);
         return false;
       }
     } else
-      output_strm = &result.GetOutputStream();
+      output_stream_p = &result.GetOutputStream();
 
-    assert(output_strm != nullptr);
+    assert(output_stream_p != nullptr);
     bool dumped =
-        runtime->DumpAllocation(*output_strm, m_exe_ctx.GetFramePtr(), id);
+        runtime->DumpAllocation(*output_stream_p, m_exe_ctx.GetFramePtr(), id);
 
     if (dumped)
       result.SetStatus(eReturnStatusSuccessFinishResult);
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 577d80f03cd..2fad73c702a 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -6271,22 +6271,23 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp,
             buffer.PutHex32(segment.flags);
           }
 
-          File core_file;
           std::string core_file_path(outfile.GetPath());
-          error = FileSystem::Instance().Open(core_file, outfile,
-                                              File::eOpenOptionWrite |
-                                                  File::eOpenOptionTruncate |
-                                                  File::eOpenOptionCanCreate);
-          if (error.Success()) {
+          auto core_file = FileSystem::Instance().Open(
+              outfile, File::eOpenOptionWrite | File::eOpenOptionTruncate |
+                           File::eOpenOptionCanCreate);
+          if (!core_file) {
+            error = core_file.takeError();
+          } else {
             // Read 1 page at a time
             uint8_t bytes[0x1000];
             // Write the mach header and load commands out to the core file
             size_t bytes_written = buffer.GetString().size();
-            error = core_file.Write(buffer.GetString().data(), bytes_written);
+            error = core_file.get()->Write(buffer.GetString().data(),
+                                           bytes_written);
             if (error.Success()) {
               // Now write the file data for all memory segments in the process
               for (const auto &segment : segment_load_commands) {
-                if (core_file.SeekFromStart(segment.fileoff) == -1) {
+                if (core_file.get()->SeekFromStart(segment.fileoff) == -1) {
                   error.SetErrorStringWithFormat(
                       "unable to seek to offset 0x%" PRIx64 " in '%s'",
                       segment.fileoff, core_file_path.c_str());
@@ -6311,7 +6312,7 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp,
 
                   if (bytes_read == bytes_to_read) {
                     size_t bytes_written = bytes_read;
-                    error = core_file.Write(bytes, bytes_written);
+                    error = core_file.get()->Write(bytes, bytes_written);
                     bytes_left -= bytes_read;
                     addr += bytes_read;
                   } else {
@@ -6319,7 +6320,7 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp,
                     // be zero filled
                     memset(bytes, 0, bytes_to_read);
                     size_t bytes_written = bytes_to_read;
-                    error = core_file.Write(bytes, bytes_written);
+                    error = core_file.get()->Write(bytes, bytes_written);
                     bytes_left -= bytes_to_read;
                     addr += bytes_to_read;
                   }
diff --git a/lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm b/lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
index 03bb70f5163..1d259554fbc 100644
--- a/lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
+++ b/lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
@@ -382,7 +382,7 @@ bool CoreSimulatorSupport::Device::Shutdown(Status &err) {
 
 static Status HandleFileAction(ProcessLaunchInfo &launch_info,
                                NSMutableDictionary *options, NSString *key,
-                               const int fd, File &file) {
+                               const int fd, lldb::FileSP &file) {
   Status error;
   const FileAction *file_action = launch_info.GetFileActionForFD(fd);
   if (file_action) {
@@ -434,7 +434,7 @@ static Status HandleFileAction(ProcessLaunchInfo &launch_info,
             file_options |= File::eOpenOptionRead;
           if ((oflag & O_RDWR) || (oflag & O_RDONLY))
             file_options |= File::eOpenOptionWrite;
-          file.SetDescriptor(created_fd, file_options, true);
+          file = std::make_shared<File>(created_fd, file_options, true);
           [options setValue:[NSNumber numberWithInteger:created_fd] forKey:key];
           return error; // Success
         } else {
@@ -494,9 +494,9 @@ CoreSimulatorSupport::Device::Spawn(ProcessLaunchInfo &launch_info) {
   [options setObject:env_dict forKey:kSimDeviceSpawnEnvironment];
 
   Status error;
-  File stdin_file;
-  File stdout_file;
-  File stderr_file;
+  lldb::FileSP stdin_file;
+  lldb::FileSP stdout_file;
+  lldb::FileSP stderr_file;
   error = HandleFileAction(launch_info, options, kSimDeviceSpawnStdin,
                            STDIN_FILENO, stdin_file);
 
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
index 1fdd089dd5d..f60991034aa 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -512,14 +512,23 @@ GDBRemoteCommunicationServerCommon::Handle_vFile_Open(
         mode_t mode = packet.GetHexMaxU32(false, 0600);
         FileSpec path_spec(path);
         FileSystem::Instance().Resolve(path_spec);
-        File file;
         // Do not close fd.
-        Status error =
-            FileSystem::Instance().Open(file, path_spec, flags, mode, false);
-        const int save_errno = error.GetError();
+        auto file = FileSystem::Instance().Open(path_spec, flags, mode, false);
+
+        int save_errno = 0;
+        int descriptor = File::kInvalidDescriptor;
+        if (file) {
+          descriptor = file.get()->GetDescriptor();
+        } else {
+          std::error_code code = errorToErrorCode(file.takeError());
+          if (code.category() == std::system_category()) {
+            save_errno = code.value();
+          }
+        }
+
         StreamString response;
         response.PutChar('F');
-        response.Printf("%i", file.GetDescriptor());
+        response.Printf("%i", descriptor);
         if (save_errno)
           response.Printf(",%i", save_errno);
         return SendPacketNoLock(response.GetString());
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index d54363cae26..ab1e572316a 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -99,12 +99,14 @@ namespace lldb {
 // namespace. This allows you to attach with a debugger and call this function
 // and get the packet history dumped to a file.
 void DumpProcessGDBRemotePacketHistory(void *p, const char *path) {
-  StreamFile strm;
-  Status error = FileSystem::Instance().Open(strm.GetFile(), FileSpec(path),
-                                             File::eOpenOptionWrite |
-                                                 File::eOpenOptionCanCreate);
-  if (error.Success())
-    ((ProcessGDBRemote *)p)->GetGDBRemote().DumpHistory(strm);
+  auto file = FileSystem::Instance().Open(
+      FileSpec(path), File::eOpenOptionWrite | File::eOpenOptionCanCreate);
+  if (!file) {
+    llvm::consumeError(file.takeError());
+    return;
+  }
+  StreamFile stream(std::move(file.get()));
+  ((ProcessGDBRemote *)p)->GetGDBRemote().DumpHistory(stream);
 }
 } // namespace lldb
 
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
index 97f8388d771..bb7f77fa514 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -949,11 +949,6 @@ PythonFile::PythonFile() : PythonObject() {}
 
 PythonFile::PythonFile(File &file, const char *mode) { Reset(file, mode); }
 
-PythonFile::PythonFile(const char *path, const char *mode) {
-  lldb_private::File file;
-  FileSystem::Instance().Open(file, FileSpec(path), GetOptionsFromMode(mode));
-  Reset(file, mode);
-}
 
 PythonFile::PythonFile(PyRefType type, PyObject *o) { Reset(type, o); }
 
@@ -1036,17 +1031,19 @@ uint32_t PythonFile::GetOptionsFromMode(llvm::StringRef mode) {
       .Default(0);
 }
 
-bool PythonFile::GetUnderlyingFile(File &file) const {
+FileUP PythonFile::GetUnderlyingFile() const {
   if (!IsValid())
-    return false;
+    return nullptr;
 
-  file.Close();
   // We don't own the file descriptor returned by this function, make sure the
   // File object knows about that.
   PythonString py_mode = GetAttributeValue("mode").AsType<PythonString>();
   auto options = PythonFile::GetOptionsFromMode(py_mode.GetString());
-  file.SetDescriptor(PyObject_AsFileDescriptor(m_py_obj), options, false);
-  return file.IsValid();
+  auto file = std::make_unique<File>(PyObject_AsFileDescriptor(m_py_obj),
+                                     options, false);
+  if (!file->IsValid())
+    return nullptr;
+  return file;
 }
 
 #endif
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
index 049ce90bb1b..3e1a3541bc7 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -455,7 +455,6 @@ class PythonFile : public PythonObject {
 public:
   PythonFile();
   PythonFile(File &file, const char *mode);
-  PythonFile(const char *path, const char *mode);
   PythonFile(PyRefType type, PyObject *o);
 
   ~PythonFile() override;
@@ -469,7 +468,7 @@ public:
 
   static uint32_t GetOptionsFromMode(llvm::StringRef mode);
 
-  bool GetUnderlyingFile(File &file) const;
+  lldb::FileUP GetUnderlyingFile() const;
 };
 
 } // namespace lldb_private
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
index e8af9db2ce4..1d130672825 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -45,6 +45,7 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FormatAdapters.h"
 
 #include <memory>
 #include <mutex>
@@ -901,17 +902,24 @@ bool ScriptInterpreterPythonImpl::ExecuteOneLine(
         debugger.AdoptTopIOHandlerFilesIfInvalid(input_file_sp, output_file_sp,
                                                  error_file_sp);
     } else {
-      input_file_sp = std::make_shared<StreamFile>();
-      FileSystem::Instance().Open(input_file_sp->GetFile(),
+      auto nullin = FileSystem::Instance().Open(
                                   FileSpec(FileSystem::DEV_NULL),
                                   File::eOpenOptionRead);
-
-      output_file_sp = std::make_shared<StreamFile>();
-      FileSystem::Instance().Open(output_file_sp->GetFile(),
+      auto nullout = FileSystem::Instance().Open(
                                   FileSpec(FileSystem::DEV_NULL),
                                   File::eOpenOptionWrite);
-
-      error_file_sp = output_file_sp;
+      if (!nullin) {
+        result->AppendErrorWithFormatv("failed to open /dev/null: {0}\n",
+                                       llvm::fmt_consume(nullin.takeError()));
+        return false;
+      }
+      if (!nullout) {
+        result->AppendErrorWithFormatv("failed to open /dev/null: {0}\n",
+                                       llvm::fmt_consume(nullout.takeError()));
+        return false;
+      }
+      input_file_sp = std::make_shared<StreamFile>(std::move(nullin.get()));
+      error_file_sp = output_file_sp = std::make_shared<StreamFile>(std::move(nullout.get()));
     }
 
     FILE *in_file = input_file_sp->GetFile().GetStream();