Revert "[lldb] Change the implementation of Status to store an llvm::Error (NFC) (#106774)"

This reverts commit 104b249 because
it has caused 2 test failures on Windows:
https://lab.llvm.org/buildbot/#/builders/141/builds/2544

Failed Tests (2):
  lldb-api :: functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
  lldb-unit :: Utility/./UtilityTests.exe/StatusTest/ErrorWin32

I reckon the cause is the same, that we construct an error with the Win32
NO_ERROR value which means there was no error but we're assuming anything
with an error code is a failure.

Author: DavidSpickett

lldb/include/lldb/Utility/Status.h

@@ -28,67 +28,6 @@ namespace lldb_private {
 
 const char *ExpressionResultAsCString(lldb::ExpressionResults result);
 
-/// Going a bit against the spirit of llvm::Error,
-/// lldb_private::Status need to store errors long-term and sometimes
-/// copy them. This base class defines an interface for this
-/// operation.
-class CloneableError
-    : public llvm::ErrorInfo<CloneableError, llvm::ErrorInfoBase> {
-public:
-  using llvm::ErrorInfo<CloneableError, llvm::ErrorInfoBase>::ErrorInfo;
-  CloneableError() : ErrorInfo() {}
-  virtual std::unique_ptr<CloneableError> Clone() const = 0;
-  static char ID;
-};
-
-/// Common base class for all error-code errors.
-class CloneableECError
-    : public llvm::ErrorInfo<CloneableECError, CloneableError> {
-public:
-  using llvm::ErrorInfo<CloneableECError, CloneableError>::ErrorInfo;
-  std::error_code convertToErrorCode() const override { return EC; }
-  void log(llvm::raw_ostream &OS) const override { OS << EC.message(); }
-  static char ID;
-
-protected:
-  CloneableECError() = delete;
-  CloneableECError(std::error_code ec) : ErrorInfo(), EC(ec) {}
-  std::error_code EC;
-};
-/// FIXME: Move these declarations closer to where they're used.
-class MachKernelError
-    : public llvm::ErrorInfo<MachKernelError, CloneableECError> {
-public:
-  using llvm::ErrorInfo<MachKernelError, CloneableECError>::ErrorInfo;
-  MachKernelError(std::error_code ec) : ErrorInfo(ec) {}
-  std::string message() const override;
-  std::unique_ptr<CloneableError> Clone() const override;
-  static char ID;
-};
-
-class Win32Error : public llvm::ErrorInfo<Win32Error, CloneableECError> {
-public:
-  using llvm::ErrorInfo<Win32Error, CloneableECError>::ErrorInfo;
-  Win32Error(std::error_code ec, const llvm::Twine &msg = {}) : ErrorInfo(ec) {}
-  std::string message() const override;
-  std::unique_ptr<CloneableError> Clone() const override;
-  static char ID;
-};
-
-class ExpressionError
-    : public llvm::ErrorInfo<ExpressionError, CloneableECError> {
-public:
-  using llvm::ErrorInfo<ExpressionError, CloneableECError>::ErrorInfo;
-  ExpressionError(std::error_code ec, std::string msg = {})
-      : ErrorInfo(ec), m_string(msg) {}
-  std::unique_ptr<CloneableError> Clone() const override;
-  std::string message() const override { return m_string; }
-  static char ID;
-
-protected:
-  std::string m_string;
-};
-
 /// \class Status Status.h "lldb/Utility/Status.h" An error handling class.
 ///
 /// This class is designed to be able to hold any error code that can be
@@ -161,7 +100,9 @@ class Status {
   }
 
   static Status FromExpressionError(lldb::ExpressionResults result,
-                                    std::string msg);
+                                    std::string msg) {
+    return Status(result, lldb::eErrorTypeExpression, msg);
+  }
 
   /// Set the current error to errno.
   ///
@@ -174,7 +115,6 @@ class Status {
   const Status &operator=(Status &&);
   /// Avoid using this in new code. Migrate APIs to llvm::Expected instead.
   static Status FromError(llvm::Error error);
-
   /// FIXME: Replace this with a takeError() method.
   llvm::Error ToError() const;
   /// Don't call this function in new code. Instead, redesign the API
@@ -209,20 +149,12 @@ class Status {
 
   /// Access the error value.
   ///
-  /// If the internally stored \ref llvm::Error is an \ref
-  /// llvm::ErrorList then this returns the error value of the first
-  /// error.
-  ///
   /// \return
   ///     The error value.
   ValueType GetError() const;
 
   /// Access the error type.
   ///
-  /// If the internally stored \ref llvm::Error is an \ref
-  /// llvm::ErrorList then this returns the error value of the first
-  /// error.
-  ///
   /// \return
   ///     The error type enumeration value.
   lldb::ErrorType GetType() const;
@@ -238,9 +170,12 @@ class Status {
   bool Success() const;
 
 protected:
-  Status(llvm::Error error) : m_error(std::move(error)) {}
-  llvm::Error m_error;
-  /// TODO: Replace this with just calling toString(m_error).
+  Status(llvm::Error error);
+  /// Status code as an integer value.
+  ValueType m_code = 0;
+  /// The type of the above error code.
+  lldb::ErrorType m_type = lldb::eErrorTypeInvalid;
+  /// A string representation of the error code.
   mutable std::string m_string;
 };
 

lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp

@@ -1108,10 +1108,9 @@ template <typename Base> class OwnedPythonFile : public Base {
         py_error = Status::FromError(r.takeError());
     }
     base_error = Base::Close();
-    // Cloning since the wrapped exception may still reference the PyThread.
     if (py_error.Fail())
-      return py_error.Clone();
-    return base_error.Clone();
+      return py_error;
+    return base_error;
   };
 
   PyObject *GetPythonObject() const {
@@ -1197,17 +1196,15 @@ class PythonIOFile : public OwnedPythonFile<File> {
       return Flush();
     auto r = m_py_obj.CallMethod("close");
     if (!r)
-      // Cloning since the wrapped exception may still reference the PyThread.
-      return Status::FromError(r.takeError()).Clone();
+      return Status::FromError(r.takeError());
     return Status();
   }
 
   Status Flush() override {
     GIL takeGIL;
     auto r = m_py_obj.CallMethod("flush");
     if (!r)
-      // Cloning since the wrapped exception may still reference the PyThread.
-      return Status::FromError(r.takeError()).Clone();
+      return Status::FromError(r.takeError());
     return Status();
   }
 
@@ -1243,8 +1240,7 @@ class BinaryPythonFile : public PythonIOFile {
     PyObject *pybuffer_p = PyMemoryView_FromMemory(
         const_cast<char *>((const char *)buf), num_bytes, PyBUF_READ);
     if (!pybuffer_p)
-      // Cloning since the wrapped exception may still reference the PyThread.
-      return Status::FromError(llvm::make_error<PythonException>()).Clone();
+      return Status::FromError(llvm::make_error<PythonException>());
     auto pybuffer = Take<PythonObject>(pybuffer_p);
     num_bytes = 0;
     auto bytes_written = As<long long>(m_py_obj.CallMethod("write", pybuffer));
@@ -1264,8 +1260,7 @@ class BinaryPythonFile : public PythonIOFile {
     auto pybuffer_obj =
         m_py_obj.CallMethod("read", (unsigned long long)num_bytes);
     if (!pybuffer_obj)
-      // Cloning since the wrapped exception may still reference the PyThread.
-      return Status::FromError(pybuffer_obj.takeError()).Clone();
+      return Status::FromError(pybuffer_obj.takeError());
     num_bytes = 0;
     if (pybuffer_obj.get().IsNone()) {
       // EOF
@@ -1274,8 +1269,7 @@ class BinaryPythonFile : public PythonIOFile {
     }
     auto pybuffer = PythonBuffer::Create(pybuffer_obj.get());
     if (!pybuffer)
-      // Cloning since the wrapped exception may still reference the PyThread.
-      return Status::FromError(pybuffer.takeError()).Clone();
+      return Status::FromError(pybuffer.takeError());
     memcpy(buf, pybuffer.get().get().buf, pybuffer.get().get().len);
     num_bytes = pybuffer.get().get().len;
     return Status();
@@ -1306,8 +1300,7 @@ class TextPythonFile : public PythonIOFile {
     auto bytes_written =
         As<long long>(m_py_obj.CallMethod("write", pystring.get()));
     if (!bytes_written)
-      // Cloning since the wrapped exception may still reference the PyThread.
-      return Status::FromError(bytes_written.takeError()).Clone();
+      return Status::FromError(bytes_written.takeError());
     if (bytes_written.get() < 0)
       return Status::FromErrorString(
           ".write() method returned a negative number!");
@@ -1328,16 +1321,14 @@ class TextPythonFile : public PythonIOFile {
     auto pystring = As<PythonString>(
         m_py_obj.CallMethod("read", (unsigned long long)num_chars));
     if (!pystring)
-      // Cloning since the wrapped exception may still reference the PyThread.
-      return Status::FromError(pystring.takeError()).Clone();
+      return Status::FromError(pystring.takeError());
     if (pystring.get().IsNone()) {
       // EOF
       return Status();
     }
     auto stringref = pystring.get().AsUTF8();
     if (!stringref)
-      // Cloning since the wrapped exception may still reference the PyThread.
-      return Status::FromError(stringref.takeError()).Clone();
+      return Status::FromError(stringref.takeError());
     num_bytes = stringref.get().size();
     memcpy(buf, stringref.get().begin(), num_bytes);
     return Status();