draft: [lldb] Upgrade ValueObject::GetData to return llvm::Expected

lldb/include/lldb/ValueObject/ValueObject.h

@@ -757,7 +757,7 @@ class ValueObject {
   virtual size_t GetPointeeData(DataExtractor &data, uint32_t item_idx = 0,
                                 uint32_t item_count = 1);
 
-  virtual uint64_t GetData(DataExtractor &data, Status &error);
+  virtual llvm::Expected<DataExtractor> GetData();
 
   virtual bool SetData(DataExtractor &data, Status &error);
 

lldb/source/API/SBValue.cpp

@@ -1381,11 +1381,13 @@ lldb::SBData SBValue::GetData() {
   ValueLocker locker;
   lldb::ValueObjectSP value_sp(GetSP(locker));
   if (value_sp) {
+    auto data_or_err = value_sp->GetData();
+    if (!data_or_err)
+      LLDB_LOG_ERRORV(GetLog(LLDBLog::API), data_or_err.takeError(),
+                      "SBValue GetData failed to extract info: {0}");
+
     DataExtractorSP data_sp(new DataExtractor());
-    Status error;
-    value_sp->GetData(*data_sp, error);
-    if (error.Success())
-      *sb_data = data_sp;
+    *sb_data = data_sp;
   }
 
   return sb_data;

lldb/source/Breakpoint/Watchpoint.cpp

@@ -230,18 +230,25 @@ bool Watchpoint::WatchedValueReportable(const ExecutionContext &exe_ctx) {
       exe_ctx.GetBestExecutionContextScope(), g_watch_name.GetStringRef(),
       watch_address, m_type);
   newest_valueobj_sp = newest_valueobj_sp->CreateConstantValue(g_watch_name);
-  Status error;
 
-  DataExtractor new_data;
-  DataExtractor old_data;
+  auto new_data_or_err = newest_valueobj_sp->GetData();
 
-  newest_valueobj_sp->GetData(new_data, error);
-  if (error.Fail())
+  if (!new_data_or_err) {
+    LLDB_LOG_ERRORV(GetLog(LLDBLog::Watchpoints), new_data_or_err.takeError(),
+                          "Failed to extract watchpoint new data: {0}");
     return true;
-  m_new_value_sp->GetData(old_data, error);
-  if (error.Fail())
+  }
+
+  auto new_data = std::move(*new_data_or_err);
+  auto old_data_or_err = m_new_value_sp->GetData();
+
+  if (!old_data_or_err) {
+    LLDB_LOG_ERRORV(GetLog(LLDBLog::Watchpoints), old_data_or_err.takeError(),
+                          "Failed to extract watchpoint old data: {0}");
     return true;
+  }
 
+  auto old_data = std::move(*old_data_or_err);
   if (new_data.GetByteSize() != old_data.GetByteSize() ||
       new_data.GetByteSize() == 0)
     return true;

lldb/source/DataFormatters/TypeFormat.cpp

@@ -46,19 +46,28 @@ bool TypeFormatImpl_Format::FormatObject(ValueObject *valobj,
     Value &value(valobj->GetValue());
     const Value::ContextType context_type = value.GetContextType();
     ExecutionContext exe_ctx(valobj->GetExecutionContextRef());
-    DataExtractor data;
 
+    auto data_or_err = valobj->GetData();
+    if (!data_or_err) {
+      LLDB_LOG_ERRORV(GetLog(LLDBLog::DataFormatters), data_or_err.takeError(),
+                      "Failed to extract data: {0}");
+      return false;
+    }
+    auto data = std::move(*data_or_err);
     if (context_type == Value::ContextType::RegisterInfo) {
       const RegisterInfo *reg_info = value.GetRegisterInfo();
       if (reg_info) {
-        Status error;
-        valobj->GetData(data, error);
-        if (error.Fail())
+        auto data_or_err = valobj->GetData();
+        if (!data_or_err) {
+          LLDB_LOG_ERRORV(GetLog(LLDBLog::Process), data_or_err.takeError(),
+                          "Failed to extract data for register info: {0}");
           return false;
+        }
 
         StreamString reg_sstr;
-        DumpDataExtractor(data, &reg_sstr, 0, GetFormat(), reg_info->byte_size,
-                          1, UINT32_MAX, LLDB_INVALID_ADDRESS, 0, 0,
+        DumpDataExtractor(*data_or_err, &reg_sstr, 0, GetFormat(),
+                          reg_info->byte_size, 1, UINT32_MAX,
+                          LLDB_INVALID_ADDRESS, 0, 0,
                           exe_ctx.GetBestExecutionContextScope());
         dest = std::string(reg_sstr.GetString());
       }
@@ -88,10 +97,13 @@ bool TypeFormatImpl_Format::FormatObject(ValueObject *valobj,
             }
           }
         } else {
-          Status error;
-          valobj->GetData(data, error);
-          if (error.Fail())
+          auto data_or_err = valobj->GetData();
+          if (!data_or_err) {
+            LLDB_LOG_ERRORV(
+                GetLog(LLDBLog::DataFormatters), data_or_err.takeError(),
+                "Failed to extract data for CString info to display: {0}:");
             return false;
+          }
         }
 
         ExecutionContextScope *exe_scope =
@@ -107,7 +119,7 @@ bool TypeFormatImpl_Format::FormatObject(ValueObject *valobj,
         compiler_type.DumpTypeValue(
             &sstr,                          // The stream to use for display
             GetFormat(),                    // Format to display this type with
-            data,                           // Data to extract from
+            *data_or_err,                   // Data to extract from
             0,                              // Byte offset into "m_data"
             *size_or_err,                   // Byte size of item in "m_data"
             valobj->GetBitfieldBitSize(),   // Bitfield bit size
@@ -184,11 +196,14 @@ bool TypeFormatImpl_EnumType::FormatObject(ValueObject *valobj,
     valobj_enum_type = iter->second;
   if (!valobj_enum_type.IsValid())
     return false;
-  DataExtractor data;
-  Status error;
-  valobj->GetData(data, error);
-  if (error.Fail())
+  auto data_or_err = valobj->GetData();
+  if (!data_or_err) {
+    LLDB_LOG_ERRORV(GetLog(LLDBLog::DataFormatters), data_or_err.takeError(),
+                    "Can't extract data related to enum type info: {0}");
     return false;
+  }
+
+  auto data = std::move(*data_or_err);
   ExecutionContext exe_ctx(valobj->GetExecutionContextRef());
   StreamString sstr;
   valobj_enum_type.DumpTypeValue(&sstr, lldb::eFormatEnum, data, 0,

lldb/source/Expression/Materializer.cpp

@@ -483,19 +483,20 @@ class EntityVariableBase : public Materializer::Entity {
     }
 
     if (m_is_reference) {
-      DataExtractor valobj_extractor;
-      Status extract_error;
-      valobj_sp->GetData(valobj_extractor, extract_error);
-
-      if (!extract_error.Success()) {
-        err = Status::FromErrorStringWithFormat(
-            "couldn't read contents of reference variable %s: %s",
-            GetName().AsCString(), extract_error.AsCString());
+      auto valobj_extractor_or_err = valobj_sp->GetData();
+
+      if (auto error = valobj_extractor_or_err.takeError()) {
+        err = Status::FromError(llvm::joinErrors(
+            llvm::createStringError(
+                "couldn't read contents of reference variable %s: ",
+                GetName().AsCString()),
+            std::move(error)));
         return;
       }
 
       lldb::offset_t offset = 0;
-      lldb::addr_t reference_addr = valobj_extractor.GetAddress(&offset);
+      lldb::addr_t reference_addr =
+          valobj_extractor_or_err->GetAddress(&offset);
 
       Status write_error;
       map.WritePointerToMemory(load_addr, reference_addr, write_error);
@@ -523,16 +524,17 @@ class EntityVariableBase : public Materializer::Entity {
           return;
         }
       } else {
-        DataExtractor data;
-        Status extract_error;
-        valobj_sp->GetData(data, extract_error);
-        if (!extract_error.Success()) {
-          err = Status::FromErrorStringWithFormat(
-              "couldn't get the value of %s: %s", GetName().AsCString(),
-              extract_error.AsCString());
+        auto data_or_err = valobj_sp->GetData();
+        if (auto error = data_or_err.takeError()) {
+          err = Status::FromError(llvm::joinErrors(
+              llvm::createStringError("couldn't get the value of %s: ",
+                                      GetName().AsCString()),
+              std::move(error)));
           return;
         }
 
+        auto data = std::move(*data_or_err);
+
         if (m_temporary_allocation != LLDB_INVALID_ADDRESS) {
           err = Status::FromErrorStringWithFormat(
               "trying to create a temporary region for %s but one exists",
@@ -577,8 +579,8 @@ class EntityVariableBase : public Materializer::Entity {
 
         m_temporary_allocation_size = data.GetByteSize();
 
-        m_original_data = std::make_shared<DataBufferHeap>(data.GetDataStart(),
-                                                           data.GetByteSize());
+        m_original_data = std::make_shared<DataBufferHeap>(
+            data.GetDataStart(), data.GetByteSize());
 
         if (!alloc_error.Success()) {
           err = Status::FromErrorStringWithFormat(

lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp

@@ -256,15 +256,15 @@ ABIMacOSX_arm64::SetReturnValueObject(lldb::StackFrameSP &frame_sp,
   RegisterContext *reg_ctx = thread->GetRegisterContext().get();
 
   if (reg_ctx) {
-    DataExtractor data;
-    Status data_error;
-    const uint64_t byte_size = new_value_sp->GetData(data, data_error);
-    if (data_error.Fail()) {
-      error = Status::FromErrorStringWithFormat(
-          "Couldn't convert return value to raw data: %s",
-          data_error.AsCString());
-      return error;
-    }
+    auto data_or_err = new_value_sp->GetData();
+
+    if (auto error = data_or_err.takeError())
+      return Status::FromError(llvm::joinErrors(
+          llvm::createStringError("Couldn't convert return value to raw data"),
+          std::move(error)));
+
+    auto data = std::move(*data_or_err);
+    const uint64_t byte_size = data.GetByteSize();
 
     const uint32_t type_flags = return_value_type.GetTypeInfo(nullptr);
     if (type_flags & eTypeIsScalar || type_flags & eTypeIsPointer) {

lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp

@@ -296,15 +296,14 @@ Status ABISysV_arm64::SetReturnValueObject(lldb::StackFrameSP &frame_sp,
   RegisterContext *reg_ctx = thread->GetRegisterContext().get();
 
   if (reg_ctx) {
-    DataExtractor data;
-    Status data_error;
-    const uint64_t byte_size = new_value_sp->GetData(data, data_error);
-    if (data_error.Fail()) {
-      error = Status::FromErrorStringWithFormat(
-          "Couldn't convert return value to raw data: %s",
-          data_error.AsCString());
-      return error;
+    auto data_or_err = new_value_sp->GetData();
+    if (auto error = data_or_err.takeError()) {
+      return Status::FromError(llvm::joinErrors(
+          llvm::createStringError("Couldn't convert return value to raw data"),
+          std::move(error)));
     }
+    auto data = std::move(*data_or_err);
+    const uint64_t byte_size = data.GetByteSize();
 
     const uint32_t type_flags = return_value_type.GetTypeInfo(nullptr);
     if (type_flags & eTypeIsScalar || type_flags & eTypeIsPointer) {

lldb/source/Plugins/ABI/ARC/ABISysV_arc.cpp

@@ -332,15 +332,16 @@ Status ABISysV_arc::SetReturnValueObject(StackFrameSP &frame_sp,
     return result;
   }
 
-  DataExtractor data;
-  size_t num_bytes = new_value_sp->GetData(data, result);
+  auto data_or_err = new_value_sp->GetData();
 
-  if (result.Fail()) {
-    result = Status::FromErrorStringWithFormat(
-        "Couldn't convert return value to raw data: %s", result.AsCString());
-    return result;
-  }
+  if (auto error = data_or_err.takeError())
+    return Status::FromError(llvm::joinErrors(
+        llvm::createStringError("Couldn't convert return value to raw data"),
+        std::move(error)));
+
+  auto data = std::move(*data_or_err);
 
+  size_t num_bytes = data.GetByteSize();
   if (num_bytes <= 2 * reg_size) {
     offset_t offset = 0;
     uint64_t raw_value = data.GetMaxU64(&offset, num_bytes);

lldb/source/Plugins/ABI/ARM/ABIMacOSX_arm.cpp

@@ -1703,15 +1703,16 @@ Status ABIMacOSX_arm::SetReturnValueObject(lldb::StackFrameSP &frame_sp,
   bool set_it_simple = false;
   if (compiler_type.IsIntegerOrEnumerationType(is_signed) ||
       compiler_type.IsPointerType()) {
-    DataExtractor data;
-    Status data_error;
-    size_t num_bytes = new_value_sp->GetData(data, data_error);
-    if (data_error.Fail()) {
-      error = Status::FromErrorStringWithFormat(
-          "Couldn't convert return value to raw data: %s",
-          data_error.AsCString());
-      return error;
+    auto data_or_err = new_value_sp->GetData();
+    if (auto error = data_or_err.takeError()) {
+      return Status::FromError(llvm::joinErrors(
+          llvm::createStringError("Couldn't convert return value to raw data"),
+          std::move(error)));
     }
+
+    auto data = std::move(*data_or_err);
+
+    size_t num_bytes = data.GetByteSize();
     lldb::offset_t offset = 0;
     if (num_bytes <= 8) {
       const RegisterInfo *r0_info = reg_ctx->GetRegisterInfoByName("r0", 0);

lldb/source/Plugins/ABI/ARM/ABISysV_arm.cpp

@@ -1849,15 +1849,15 @@ Status ABISysV_arm::SetReturnValueObject(lldb::StackFrameSP &frame_sp,
   bool set_it_simple = false;
   if (compiler_type.IsIntegerOrEnumerationType(is_signed) ||
       compiler_type.IsPointerType()) {
-    DataExtractor data;
-    Status data_error;
-    size_t num_bytes = new_value_sp->GetData(data, data_error);
-    if (data_error.Fail()) {
-      error = Status::FromErrorStringWithFormat(
-          "Couldn't convert return value to raw data: %s",
-          data_error.AsCString());
-      return error;
+    auto data_or_err = new_value_sp->GetData();
+    if (auto error = data_or_err.takeError()) {
+      return Status::FromError(llvm::joinErrors(
+          llvm::createStringError("Couldn't convert return value to raw data"),
+          std::move(error)));
     }
+    auto data = std::move(*data_or_err);
+
+    size_t num_bytes = data.GetByteSize();
     lldb::offset_t offset = 0;
     if (num_bytes <= 8) {
       const RegisterInfo *r0_info = reg_ctx->GetRegisterInfo(

lldb/source/Plugins/ABI/LoongArch/ABISysV_loongarch.cpp

@@ -267,15 +267,16 @@ Status ABISysV_loongarch::SetReturnValueObject(StackFrameSP &frame_sp,
     return result;
   }
 
-  DataExtractor data;
-  size_t num_bytes = new_value_sp->GetData(data, result);
+  auto data_or_err = new_value_sp->GetData();
 
-  if (result.Fail()) {
-    result = Status::FromErrorStringWithFormat(
-        "Couldn't convert return value to raw data: %s", result.AsCString());
-    return result;
-  }
+  if (auto error = data_or_err.takeError())
+    return Status::FromError(llvm::joinErrors(
+        llvm::createStringError("Couldn't convert return value to raw data"),
+        std::move(error)));
+
+  auto data = std::move(*data_or_err);
 
+  size_t num_bytes = data.GetByteSize();
   size_t reg_size = m_is_la64 ? 8 : 4;
   // Currently, we only support sizeof(data) <= 2 * reg_size.
   // 1. If the (`size` <= reg_size), the `data` will be returned through `ARG1`.

lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp

@@ -716,16 +716,15 @@ Status ABISysV_mips::SetReturnValueObject(lldb::StackFrameSP &frame_sp,
   bool set_it_simple = false;
   if (compiler_type.IsIntegerOrEnumerationType(is_signed) ||
       compiler_type.IsPointerType()) {
-    DataExtractor data;
-    Status data_error;
-    size_t num_bytes = new_value_sp->GetData(data, data_error);
-    if (data_error.Fail()) {
-      error = Status::FromErrorStringWithFormat(
-          "Couldn't convert return value to raw data: %s",
-          data_error.AsCString());
-      return error;
-    }
 
+    auto data_or_err = new_value_sp->GetData();
+    if (auto error = data_or_err.takeError())
+      return Status::FromError(llvm::joinErrors(
+          llvm::createStringError("Couldn't convert return value to raw data"),
+          std::move(error)));
+
+    auto data = std::move(*data_or_err);
+    size_t num_bytes = data.GetByteSize();
     lldb::offset_t offset = 0;
     if (num_bytes <= 8) {
       const RegisterInfo *r2_info = reg_ctx->GetRegisterInfoByName("r2", 0);