[lldb] Upgrade CompilerType::GetBitSize to return llvm::Expected (llvm#129601)

This patch pushes the error handling boundary for the GetBitSize()
methods from Runtime into the Type and CompilerType APIs. This makes it
easier to diagnose problems thanks to more meaningful error messages
being available. GetBitSize() is often the first thing LLDB asks about a
type, so this method is particularly important for a better user
experience.

rdar://145667239

Author: adrian-prantl

lldb/include/lldb/Expression/ExpressionVariable.h

@@ -33,7 +33,7 @@ class ExpressionVariable
 
   virtual ~ExpressionVariable() = default;
 
-  std::optional<uint64_t> GetByteSize() { return m_frozen_sp->GetByteSize(); }
+  llvm::Expected<uint64_t> GetByteSize() { return m_frozen_sp->GetByteSize(); }
 
   ConstString GetName() { return m_frozen_sp->GetName(); }
 

lldb/include/lldb/Symbol/CompilerType.h

@@ -391,9 +391,9 @@ class CompilerType {
   struct IntegralTemplateArgument;
 
   /// Return the size of the type in bytes.
-  std::optional<uint64_t> GetByteSize(ExecutionContextScope *exe_scope) const;
+  llvm::Expected<uint64_t> GetByteSize(ExecutionContextScope *exe_scope) const;
   /// Return the size of the type in bits.
-  std::optional<uint64_t> GetBitSize(ExecutionContextScope *exe_scope) const;
+  llvm::Expected<uint64_t> GetBitSize(ExecutionContextScope *exe_scope) const;
 
   lldb::Encoding GetEncoding(uint64_t &count) const;
 

lldb/include/lldb/Symbol/Type.h

@@ -480,7 +480,7 @@ class Type : public std::enable_shared_from_this<Type>, public UserID {
 
   ConstString GetBaseName();
 
-  std::optional<uint64_t> GetByteSize(ExecutionContextScope *exe_scope);
+  llvm::Expected<uint64_t> GetByteSize(ExecutionContextScope *exe_scope);
 
   llvm::Expected<uint32_t> GetNumChildren(bool omit_empty_base_classes);
 

lldb/include/lldb/Symbol/TypeSystem.h

@@ -312,7 +312,7 @@ class TypeSystem : public PluginInterface,
 
   virtual const llvm::fltSemantics &GetFloatTypeSemantics(size_t byte_size) = 0;
 
-  virtual std::optional<uint64_t>
+  virtual llvm::Expected<uint64_t>
   GetBitSize(lldb::opaque_compiler_type_t type,
              ExecutionContextScope *exe_scope) = 0;
 

lldb/include/lldb/Target/StackFrameRecognizer.h

@@ -180,7 +180,7 @@ class ValueObjectRecognizerSynthesizedValue : public ValueObject {
     SetName(parent.GetName());
   }
 
-  std::optional<uint64_t> GetByteSize() override {
+  llvm::Expected<uint64_t> GetByteSize() override {
     return m_parent->GetByteSize();
   }
   lldb::ValueType GetValueType() const override { return m_type; }

lldb/include/lldb/ValueObject/ValueObject.h

@@ -357,7 +357,7 @@ class ValueObject {
   virtual bool CanProvideValue();
 
   // Subclasses must implement the functions below.
-  virtual std::optional<uint64_t> GetByteSize() = 0;
+  virtual llvm::Expected<uint64_t> GetByteSize() = 0;
 
   virtual lldb::ValueType GetValueType() const = 0;
 

lldb/include/lldb/ValueObject/ValueObjectCast.h

@@ -30,7 +30,7 @@ class ValueObjectCast : public ValueObject {
   static lldb::ValueObjectSP Create(ValueObject &parent, ConstString name,
                                     const CompilerType &cast_type);
 
-  std::optional<uint64_t> GetByteSize() override;
+  llvm::Expected<uint64_t> GetByteSize() override;
 
   llvm::Expected<uint32_t> CalculateNumChildren(uint32_t max) override;
 

lldb/include/lldb/ValueObject/ValueObjectChild.h

@@ -29,7 +29,7 @@ class ValueObjectChild : public ValueObject {
 public:
   ~ValueObjectChild() override;
 
-  std::optional<uint64_t> GetByteSize() override { return m_byte_size; }
+  llvm::Expected<uint64_t> GetByteSize() override { return m_byte_size; }
 
   lldb::offset_t GetByteOffset() override { return m_byte_offset; }
 

lldb/include/lldb/ValueObject/ValueObjectConstResult.h

@@ -64,7 +64,7 @@ class ValueObjectConstResult : public ValueObject {
   static lldb::ValueObjectSP Create(ExecutionContextScope *exe_scope,
                                     Status &&error);
 
-  std::optional<uint64_t> GetByteSize() override;
+  llvm::Expected<uint64_t> GetByteSize() override;
 
   lldb::ValueType GetValueType() const override;
 

lldb/include/lldb/ValueObject/ValueObjectDynamicValue.h

@@ -35,7 +35,7 @@ class ValueObjectDynamicValue : public ValueObject {
 public:
   ~ValueObjectDynamicValue() override = default;
 
-  std::optional<uint64_t> GetByteSize() override;
+  llvm::Expected<uint64_t> GetByteSize() override;
 
   ConstString GetTypeName() override;
 

lldb/include/lldb/ValueObject/ValueObjectMemory.h

@@ -41,7 +41,7 @@ class ValueObjectMemory : public ValueObject {
                                     const Address &address,
                                     const CompilerType &ast_type);
 
-  std::optional<uint64_t> GetByteSize() override;
+  llvm::Expected<uint64_t> GetByteSize() override;
 
   ConstString GetTypeName() override;
 

lldb/include/lldb/ValueObject/ValueObjectRegister.h

@@ -37,7 +37,7 @@ class ValueObjectRegisterSet : public ValueObject {
                                     lldb::RegisterContextSP &reg_ctx_sp,
                                     uint32_t set_idx);
 
-  std::optional<uint64_t> GetByteSize() override;
+  llvm::Expected<uint64_t> GetByteSize() override;
 
   lldb::ValueType GetValueType() const override {
     return lldb::eValueTypeRegisterSet;
@@ -89,7 +89,7 @@ class ValueObjectRegister : public ValueObject {
                                     lldb::RegisterContextSP &reg_ctx_sp,
                                     const RegisterInfo *reg_info);
 
-  std::optional<uint64_t> GetByteSize() override;
+  llvm ::Expected<uint64_t> GetByteSize() override;
 
   lldb::ValueType GetValueType() const override {
     return lldb::eValueTypeRegister;

lldb/include/lldb/ValueObject/ValueObjectSyntheticFilter.h

@@ -37,7 +37,7 @@ class ValueObjectSynthetic : public ValueObject {
 public:
   ~ValueObjectSynthetic() override;
 
-  std::optional<uint64_t> GetByteSize() override;
+  llvm::Expected<uint64_t> GetByteSize() override;
 
   ConstString GetTypeName() override;
 

lldb/include/lldb/ValueObject/ValueObjectVTable.h

@@ -62,7 +62,7 @@ class ValueObjectVTable : public ValueObject {
 
   static lldb::ValueObjectSP Create(ValueObject &parent);
 
-  std::optional<uint64_t> GetByteSize() override;
+  llvm::Expected<uint64_t> GetByteSize() override;
 
   llvm::Expected<uint32_t> CalculateNumChildren(uint32_t max) override;
 

lldb/include/lldb/ValueObject/ValueObjectVariable.h

@@ -38,7 +38,7 @@ class ValueObjectVariable : public ValueObject {
   static lldb::ValueObjectSP Create(ExecutionContextScope *exe_scope,
                                     const lldb::VariableSP &var_sp);
 
-  std::optional<uint64_t> GetByteSize() override;
+  llvm::Expected<uint64_t> GetByteSize() override;
 
   ConstString GetTypeName() override;
 

lldb/source/API/SBType.cpp

@@ -127,8 +127,8 @@ uint64_t SBType::GetByteSize() {
   LLDB_INSTRUMENT_VA(this);
 
   if (IsValid())
-    if (std::optional<uint64_t> size =
-            m_opaque_sp->GetCompilerType(false).GetByteSize(nullptr))
+    if (std::optional<uint64_t> size = llvm::expectedToOptional(
+            m_opaque_sp->GetCompilerType(false).GetByteSize(nullptr)))
       return *size;
   return 0;
 }

lldb/source/API/SBValue.cpp

@@ -329,7 +329,7 @@ size_t SBValue::GetByteSize() {
   ValueLocker locker;
   lldb::ValueObjectSP value_sp(GetSP(locker));
   if (value_sp) {
-    result = value_sp->GetByteSize().value_or(0);
+    result = llvm::expectedToOptional(value_sp->GetByteSize()).value_or(0);
   }
 
   return result;

lldb/source/Commands/CommandObjectMemory.cpp

@@ -519,14 +519,14 @@ class CommandObjectMemoryRead : public CommandObjectParsed {
         --pointer_count;
       }
 
-      std::optional<uint64_t> size = compiler_type.GetByteSize(nullptr);
-      if (!size) {
+      auto size_or_err = compiler_type.GetByteSize(nullptr);
+      if (!size_or_err) {
         result.AppendErrorWithFormat(
-            "unable to get the byte size of the type '%s'\n",
-            view_as_type_cstr);
+            "unable to get the byte size of the type '%s'\n%s",
+            view_as_type_cstr, llvm::toString(size_or_err.takeError()).c_str());
         return;
       }
-      m_format_options.GetByteSizeValue() = *size;
+      m_format_options.GetByteSizeValue() = *size_or_err;
 
       if (!m_format_options.GetCountValue().OptionWasSet())
         m_format_options.GetCountValue() = 1;
@@ -639,15 +639,16 @@ class CommandObjectMemoryRead : public CommandObjectParsed {
       if (!m_format_options.GetFormatValue().OptionWasSet())
         m_format_options.GetFormatValue().SetCurrentValue(eFormatDefault);
 
-      std::optional<uint64_t> size = compiler_type.GetByteSize(nullptr);
-      if (!size) {
-        result.AppendError("can't get size of type");
+      auto size_or_err = compiler_type.GetByteSize(nullptr);
+      if (!size_or_err) {
+        result.AppendError(llvm::toString(size_or_err.takeError()));
         return;
       }
-      bytes_read = *size * m_format_options.GetCountValue().GetCurrentValue();
+      auto size = *size_or_err;
+      bytes_read = size * m_format_options.GetCountValue().GetCurrentValue();
 
       if (argc > 0)
-        addr = addr + (*size * m_memory_options.m_offset.GetCurrentValue());
+        addr = addr + (size * m_memory_options.m_offset.GetCurrentValue());
     } else if (m_format_options.GetFormatValue().GetCurrentValue() !=
                eFormatCString) {
       data_sp = std::make_shared<DataBufferHeap>(total_byte_size, '\0');
@@ -1034,8 +1035,8 @@ class CommandObjectMemoryFind : public CommandObjectParsed {
                frame, result_sp)) &&
           result_sp) {
         uint64_t value = result_sp->GetValueAsUnsigned(0);
-        std::optional<uint64_t> size =
-            result_sp->GetCompilerType().GetByteSize(nullptr);
+        std::optional<uint64_t> size = llvm::expectedToOptional(
+            result_sp->GetCompilerType().GetByteSize(nullptr));
         if (!size)
           return;
         switch (*size) {

lldb/source/Commands/CommandObjectWatchpoint.cpp

@@ -867,9 +867,10 @@ corresponding to the byte size of the data type.");
       if (addr_type == eAddressTypeLoad) {
         // We're in business.
         // Find out the size of this variable.
-        size = m_option_watchpoint.watch_size.GetCurrentValue() == 0
-                   ? valobj_sp->GetByteSize().value_or(0)
-                   : m_option_watchpoint.watch_size.GetCurrentValue();
+        size =
+            m_option_watchpoint.watch_size.GetCurrentValue() == 0
+                ? llvm::expectedToOptional(valobj_sp->GetByteSize()).value_or(0)
+                : m_option_watchpoint.watch_size.GetCurrentValue();
       }
       compiler_type = valobj_sp->GetCompilerType();
     } else {
@@ -1080,7 +1081,8 @@ class CommandObjectWatchpointSetExpression : public CommandObjectRaw {
     /// of the expression, so convert to that if we found a valid type.
     CompilerType compiler_type(valobj_sp->GetCompilerType());
 
-    std::optional<uint64_t> valobj_size = valobj_sp->GetByteSize();
+    std::optional<uint64_t> valobj_size =
+        llvm::expectedToOptional(valobj_sp->GetByteSize());
     // Set the type as a uint8_t array if the size being watched is
     // larger than the ValueObject's size (which is probably the size
     // of a pointer).

lldb/source/Core/Value.cpp

@@ -24,6 +24,8 @@
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/Endian.h"
 #include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
 #include "lldb/Utility/State.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/lldb-defines.h"
@@ -223,10 +225,16 @@ uint64_t Value::GetValueByteSize(Status *error_ptr, ExecutionContext *exe_ctx) {
   case ContextType::Variable: // Variable *
   {
     auto *scope = exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr;
-    if (std::optional<uint64_t> size = GetCompilerType().GetByteSize(scope)) {
+    auto size_or_err = GetCompilerType().GetByteSize(scope);
+    if (!size_or_err) {
+      if (error_ptr && error_ptr->Success())
+        *error_ptr = Status::FromError(size_or_err.takeError());
+      else
+        LLDB_LOG_ERRORV(GetLog(LLDBLog::Types), size_or_err.takeError(), "{0}");
+    } else {
       if (error_ptr)
         error_ptr->Clear();
-      return *size;
+      return *size_or_err;
     }
     break;
   }
@@ -321,8 +329,9 @@ Status Value::GetValueAsData(ExecutionContext *exe_ctx, DataExtractor &data,
   AddressType address_type = eAddressTypeFile;
   Address file_so_addr;
   const CompilerType &ast_type = GetCompilerType();
-  std::optional<uint64_t> type_size = ast_type.GetByteSize(
-      exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr);
+  std::optional<uint64_t> type_size =
+      llvm::expectedToOptional(ast_type.GetByteSize(
+          exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr));
   // Nothing to be done for a zero-sized type.
   if (type_size && *type_size == 0)
     return error;

lldb/source/DataFormatters/TypeFormat.cpp

@@ -96,16 +96,20 @@ bool TypeFormatImpl_Format::FormatObject(ValueObject *valobj,
 
         ExecutionContextScope *exe_scope =
             exe_ctx.GetBestExecutionContextScope();
-        std::optional<uint64_t> size = compiler_type.GetByteSize(exe_scope);
-        if (!size)
+        auto size_or_err = compiler_type.GetByteSize(exe_scope);
+        if (!size_or_err) {
+          LLDB_LOG_ERRORV(
+              GetLog(LLDBLog::Types), size_or_err.takeError(),
+              "Cannot get size of type while formatting object: {0}");
           return false;
+        }
         StreamString sstr;
         compiler_type.DumpTypeValue(
             &sstr,                          // The stream to use for display
             GetFormat(),                    // Format to display this type with
             data,                           // Data to extract from
             0,                              // Byte offset into "m_data"
-            *size,                          // Byte size of item in "m_data"
+            *size_or_err,                   // Byte size of item in "m_data"
             valobj->GetBitfieldBitSize(),   // Bitfield bit size
             valobj->GetBitfieldBitOffset(), // Bitfield bit offset
             exe_scope);

lldb/source/DataFormatters/VectorType.cpp

@@ -197,15 +197,15 @@ static lldb::Format GetItemFormatForFormat(lldb::Format format,
 static std::optional<size_t>
 CalculateNumChildren(CompilerType container_elem_type, uint64_t num_elements,
                      CompilerType element_type) {
-  std::optional<uint64_t> container_elem_size =
-      container_elem_type.GetByteSize(/* exe_scope */ nullptr);
+  std::optional<uint64_t> container_elem_size = llvm::expectedToOptional(
+      container_elem_type.GetByteSize(/* exe_scope */ nullptr));
   if (!container_elem_size)
     return {};
 
   auto container_size = *container_elem_size * num_elements;
 
-  std::optional<uint64_t> element_size =
-      element_type.GetByteSize(/* exe_scope */ nullptr);
+  std::optional<uint64_t> element_size = llvm::expectedToOptional(
+      element_type.GetByteSize(/* exe_scope */ nullptr));
   if (!element_size || !*element_size)
     return {};
 
@@ -236,10 +236,11 @@ class VectorTypeSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
           nullptr, Status::FromError(num_children_or_err.takeError()));
     if (idx >= *num_children_or_err)
       return {};
-    std::optional<uint64_t> size = m_child_type.GetByteSize(nullptr);
-    if (!size)
-      return {};
-    auto offset = idx * *size;
+    auto size_or_err = m_child_type.GetByteSize(nullptr);
+    if (!size_or_err)
+      return ValueObjectConstResult::Create(
+          nullptr, Status::FromError(size_or_err.takeError()));
+    auto offset = idx * *size_or_err;
     StreamString idx_name;
     idx_name.Printf("[%" PRIu64 "]", (uint64_t)idx);
     ValueObjectSP child_sp(m_backend.GetSyntheticChildAtOffset(

lldb/source/Expression/ExpressionVariable.cpp

@@ -20,7 +20,8 @@ char ExpressionVariable::ID;
 ExpressionVariable::ExpressionVariable() : m_flags(0) {}
 
 uint8_t *ExpressionVariable::GetValueBytes() {
-  std::optional<uint64_t> byte_size = m_frozen_sp->GetByteSize();
+  std::optional<uint64_t> byte_size =
+      llvm::expectedToOptional(m_frozen_sp->GetByteSize());
   if (byte_size && *byte_size) {
     if (m_frozen_sp->GetDataExtractor().GetByteSize() < *byte_size) {
       m_frozen_sp->GetValue().ResizeData(*byte_size);