Change GetChildCompilerTypeAtIndex to return Expected (NFC) #92979

adrian-prantl · 2024-05-22T00:43:42Z

This change is a general improvement of the internal API. My motivation is to use this in the Swift typesystem plugin.

llvmbot · 2024-05-22T00:44:16Z

@llvm/pr-subscribers-lldb

Author: Adrian Prantl (adrian-prantl)

Changes

This change is a general improvement of the internal API. My motivation is to use this in the Swift typesystem plugin.

Full diff: https://github.com/llvm/llvm-project/pull/92979.diff

10 Files Affected:

(modified) lldb/include/lldb/Symbol/CompilerType.h (+1-1)
(modified) lldb/include/lldb/Symbol/TypeSystem.h (+1-1)
(modified) lldb/source/Core/ValueObject.cpp (+23-8)
(modified) lldb/source/Core/ValueObjectConstResultImpl.cpp (+10-2)
(modified) lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc64.cpp (+2-1)
(modified) lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp (+11-7)
(modified) lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp (+3-3)
(modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp (+10-8)
(modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h (+1-1)
(modified) lldb/source/Symbol/CompilerType.cpp (+1-1)

diff --git a/lldb/include/lldb/Symbol/CompilerType.h b/lldb/include/lldb/Symbol/CompilerType.h
index 28c723abf2794..70dacdcb7986f 100644
--- a/lldb/include/lldb/Symbol/CompilerType.h
+++ b/lldb/include/lldb/Symbol/CompilerType.h
@@ -436,7 +436,7 @@ class CompilerType {
                                    uint32_t *bitfield_bit_size_ptr = nullptr,
                                    bool *is_bitfield_ptr = nullptr) const;
 
-  CompilerType GetChildCompilerTypeAtIndex(
+  llvm::Expected<CompilerType> GetChildCompilerTypeAtIndex(
       ExecutionContext *exe_ctx, size_t idx, bool transparent_pointers,
       bool omit_empty_base_classes, bool ignore_array_bounds,
       std::string &child_name, uint32_t &child_byte_size,
diff --git a/lldb/include/lldb/Symbol/TypeSystem.h b/lldb/include/lldb/Symbol/TypeSystem.h
index 7bcb8d69387a0..b4025c173a186 100644
--- a/lldb/include/lldb/Symbol/TypeSystem.h
+++ b/lldb/include/lldb/Symbol/TypeSystem.h
@@ -359,7 +359,7 @@ class TypeSystem : public PluginInterface,
     return CompilerDecl();
   }
 
-  virtual CompilerType GetChildCompilerTypeAtIndex(
+  virtual llvm::Expected<CompilerType> GetChildCompilerTypeAtIndex(
       lldb::opaque_compiler_type_t type, ExecutionContext *exe_ctx, size_t idx,
       bool transparent_pointers, bool omit_empty_base_classes,
       bool ignore_array_bounds, std::string &child_name,
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index f39bd07a25536..1443d9dfc3280 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -505,15 +505,23 @@ ValueObject *ValueObject::CreateChildAtIndex(size_t idx,
   uint64_t language_flags = 0;
 
   const bool transparent_pointers = !synthetic_array_member;
-  CompilerType child_compiler_type;
 
   ExecutionContext exe_ctx(GetExecutionContextRef());
 
-  child_compiler_type = GetCompilerType().GetChildCompilerTypeAtIndex(
-      &exe_ctx, idx, transparent_pointers, omit_empty_base_classes,
-      ignore_array_bounds, child_name_str, child_byte_size, child_byte_offset,
-      child_bitfield_bit_size, child_bitfield_bit_offset, child_is_base_class,
-      child_is_deref_of_parent, this, language_flags);
+  auto child_compiler_type_or_err =
+      GetCompilerType().GetChildCompilerTypeAtIndex(
+          &exe_ctx, idx, transparent_pointers, omit_empty_base_classes,
+          ignore_array_bounds, child_name_str, child_byte_size,
+          child_byte_offset, child_bitfield_bit_size, child_bitfield_bit_offset,
+          child_is_base_class, child_is_deref_of_parent, this, language_flags);
+  CompilerType child_compiler_type;
+  if (!child_compiler_type_or_err)
+    LLDB_LOG_ERROR(GetLog(LLDBLog::Types),
+                   child_compiler_type_or_err.takeError(),
+                   "could not find child: {0}");
+  else
+    child_compiler_type = *child_compiler_type_or_err;
+
   if (child_compiler_type) {
     if (synthetic_index)
       child_byte_offset += child_byte_size * synthetic_index;
@@ -2624,16 +2632,23 @@ ValueObjectSP ValueObject::Dereference(Status &error) {
     bool child_is_deref_of_parent = false;
     const bool transparent_pointers = false;
     CompilerType compiler_type = GetCompilerType();
-    CompilerType child_compiler_type;
     uint64_t language_flags = 0;
 
     ExecutionContext exe_ctx(GetExecutionContextRef());
 
-    child_compiler_type = compiler_type.GetChildCompilerTypeAtIndex(
+    CompilerType child_compiler_type;
+    auto child_compiler_type_or_err = compiler_type.GetChildCompilerTypeAtIndex(
         &exe_ctx, 0, transparent_pointers, omit_empty_base_classes,
         ignore_array_bounds, child_name_str, child_byte_size, child_byte_offset,
         child_bitfield_bit_size, child_bitfield_bit_offset, child_is_base_class,
         child_is_deref_of_parent, this, language_flags);
+    if (!child_compiler_type_or_err)
+      LLDB_LOG_ERROR(GetLog(LLDBLog::Types),
+                     child_compiler_type_or_err.takeError(),
+                     "could not find child: {0}");
+    else
+      child_compiler_type = *child_compiler_type_or_err;
+
     if (child_compiler_type && child_byte_size) {
       ConstString child_name;
       if (!child_name_str.empty())
diff --git a/lldb/source/Core/ValueObjectConstResultImpl.cpp b/lldb/source/Core/ValueObjectConstResultImpl.cpp
index e2db3ace19247..493980d7ea960 100644
--- a/lldb/source/Core/ValueObjectConstResultImpl.cpp
+++ b/lldb/source/Core/ValueObjectConstResultImpl.cpp
@@ -17,6 +17,8 @@
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/Endian.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
 #include "lldb/Utility/Scalar.h"
 
 #include <string>
@@ -66,15 +68,21 @@ ValueObject *ValueObjectConstResultImpl::CreateChildAtIndex(
 
   const bool transparent_pointers = !synthetic_array_member;
   CompilerType compiler_type = m_impl_backend->GetCompilerType();
-  CompilerType child_compiler_type;
 
   ExecutionContext exe_ctx(m_impl_backend->GetExecutionContextRef());
 
-  child_compiler_type = compiler_type.GetChildCompilerTypeAtIndex(
+  auto child_compiler_type_or_err = compiler_type.GetChildCompilerTypeAtIndex(
       &exe_ctx, idx, transparent_pointers, omit_empty_base_classes,
       ignore_array_bounds, child_name_str, child_byte_size, child_byte_offset,
       child_bitfield_bit_size, child_bitfield_bit_offset, child_is_base_class,
       child_is_deref_of_parent, m_impl_backend, language_flags);
+  CompilerType child_compiler_type;
+  if (!child_compiler_type_or_err)
+    LLDB_LOG_ERROR(GetLog(LLDBLog::Types),
+                   child_compiler_type_or_err.takeError(),
+                   "could not find child: {0}");
+  else
+    child_compiler_type = *child_compiler_type_or_err;
 
   // One might think we should check that the size of the children
   // is always strictly positive, hence we could avoid creating a
diff --git a/lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc64.cpp b/lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc64.cpp
index 173b5613d1b80..3d9b4566ca1c9 100644
--- a/lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc64.cpp
+++ b/lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc64.cpp
@@ -903,7 +903,8 @@ class ReturnValueExtractor {
   }
 
   // get child
-  CompilerType GetChildType(uint32_t i, std::string &name, uint32_t &size) {
+  llvm::Expected<CompilerType> GetChildType(uint32_t i, std::string &name,
+                                            uint32_t &size) {
     // GetChild constant inputs
     const bool transparent_pointers = false;
     const bool omit_empty_base_classes = true;
diff --git a/lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp b/lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
index 9a6e135e00834..2c9b3c425397a 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
@@ -12,6 +12,7 @@
 #include "Plugins/ExpressionParser/Clang/ClangPersistentVariables.h"
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "lldb/Core/ValueObject.h"
+#include "lldb/Core/ValueObjectConstResult.h"
 #include "lldb/DataFormatters/FormattersHelpers.h"
 #include "lldb/Symbol/CompilerType.h"
 #include "lldb/Symbol/TypeSystem.h"
@@ -105,13 +106,16 @@ class BlockPointerSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
     bool child_is_deref_of_parent = false;
     uint64_t language_flags = 0;
 
-    const CompilerType child_type =
-        m_block_struct_type.GetChildCompilerTypeAtIndex(
-            &exe_ctx, idx, transparent_pointers, omit_empty_base_classes,
-            ignore_array_bounds, child_name, child_byte_size, child_byte_offset,
-            child_bitfield_bit_size, child_bitfield_bit_offset,
-            child_is_base_class, child_is_deref_of_parent, value_object,
-            language_flags);
+    auto child_type_or_err = m_block_struct_type.GetChildCompilerTypeAtIndex(
+        &exe_ctx, idx, transparent_pointers, omit_empty_base_classes,
+        ignore_array_bounds, child_name, child_byte_size, child_byte_offset,
+        child_bitfield_bit_size, child_bitfield_bit_offset, child_is_base_class,
+        child_is_deref_of_parent, value_object, language_flags);
+    if (!child_type_or_err)
+      return ValueObjectConstResult::Create(
+          exe_ctx.GetBestExecutionContextScope(),
+          Status(child_type_or_err.takeError()));
+    CompilerType child_type = *child_type_or_err;
 
     ValueObjectSP struct_pointer_sp =
         m_backend.Cast(m_block_struct_type.GetPointerType());
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
index ec5b320e2218c..d4cc47b4546cd 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -295,13 +295,13 @@ void lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetValueOffset(
     bool child_is_base_class;
     bool child_is_deref_of_parent;
     uint64_t language_flags;
-    if (tree_node_type
+    auto child_type = llvm::expectedToStdOptional(tree_node_type
             .GetChildCompilerTypeAtIndex(
                 nullptr, 4, true, true, true, child_name, child_byte_size,
                 child_byte_offset, child_bitfield_bit_size,
                 child_bitfield_bit_offset, child_is_base_class,
-                child_is_deref_of_parent, nullptr, language_flags)
-            .IsValid())
+                child_is_deref_of_parent, nullptr, language_flags));
+    if (child_type && child_type->IsValid())
       m_skip_size = (uint32_t)child_byte_offset;
   }
 }
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 582d9eac3e1dd..1b2235b4b2b5b 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -6130,7 +6130,7 @@ uint32_t TypeSystemClang::GetNumPointeeChildren(clang::QualType type) {
   return 0;
 }
 
-CompilerType TypeSystemClang::GetChildCompilerTypeAtIndex(
+llvm::Expected<CompilerType> TypeSystemClang::GetChildCompilerTypeAtIndex(
     lldb::opaque_compiler_type_t type, ExecutionContext *exe_ctx, size_t idx,
     bool transparent_pointers, bool omit_empty_base_classes,
     bool ignore_array_bounds, std::string &child_name,
@@ -6156,11 +6156,8 @@ CompilerType TypeSystemClang::GetChildCompilerTypeAtIndex(
 
   auto num_children_or_err =
       GetNumChildren(type, omit_empty_base_classes, exe_ctx);
-  if (!num_children_or_err) {
-    LLDB_LOG_ERRORV(GetLog(LLDBLog::Types), num_children_or_err.takeError(),
-                    "{0}");
-    return {};
-  }
+  if (!num_children_or_err)
+    return num_children_or_err.takeError();
 
   const bool idx_is_valid = idx < *num_children_or_err;
   int32_t bit_offset;
@@ -6242,7 +6239,10 @@ CompilerType TypeSystemClang::GetChildCompilerTypeAtIndex(
             std::optional<uint64_t> size =
                 base_class_clang_type.GetBitSize(get_exe_scope());
             if (!size)
-              return {};
+              return llvm::make_error<llvm::StringError>(
+                  "no size info for base class",
+                  llvm::inconvertibleErrorCode());
+
             uint64_t base_class_clang_type_bit_size = *size;
 
             // Base classes bit sizes should be a multiple of 8 bits in size
@@ -6274,7 +6274,9 @@ CompilerType TypeSystemClang::GetChildCompilerTypeAtIndex(
           std::optional<uint64_t> size =
               field_clang_type.GetByteSize(get_exe_scope());
           if (!size)
-            return {};
+            return llvm::make_error<llvm::StringError>(
+                "no size info for field", llvm::inconvertibleErrorCode());
+
           child_byte_size = *size;
           const uint32_t child_bit_size = child_byte_size * 8;
 
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
index 042379d40bcb3..d67b7a4c9fe72 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -887,7 +887,7 @@ class TypeSystemClang : public TypeSystem {
 
   static uint32_t GetNumPointeeChildren(clang::QualType type);
 
-  CompilerType GetChildCompilerTypeAtIndex(
+  llvm::Expected<CompilerType> GetChildCompilerTypeAtIndex(
       lldb::opaque_compiler_type_t type, ExecutionContext *exe_ctx, size_t idx,
       bool transparent_pointers, bool omit_empty_base_classes,
       bool ignore_array_bounds, std::string &child_name,
diff --git a/lldb/source/Symbol/CompilerType.cpp b/lldb/source/Symbol/CompilerType.cpp
index 072dbccec44fb..b5269cf66235c 100644
--- a/lldb/source/Symbol/CompilerType.cpp
+++ b/lldb/source/Symbol/CompilerType.cpp
@@ -902,7 +902,7 @@ uint32_t CompilerType::GetIndexOfFieldWithName(
   return UINT32_MAX;
 }
 
-CompilerType CompilerType::GetChildCompilerTypeAtIndex(
+llvm::Expected<CompilerType> CompilerType::GetChildCompilerTypeAtIndex(
     ExecutionContext *exe_ctx, size_t idx, bool transparent_pointers,
     bool omit_empty_base_classes, bool ignore_array_bounds,
     std::string &child_name, uint32_t &child_byte_size,

github-actions · 2024-05-22T00:46:46Z

✅ With the latest revision this PR passed the C/C++ code formatter.

walter-erquinigo

Pretty nice! This will be useful for Mojo as well

This change is a general improvement of the internal API. My motivation is to use this in the Swift typesystem plugin. (cherry picked from commit ac1dc05) Conflicts: lldb/include/lldb/Symbol/TypeSystem.h

adrian-prantl requested a review from JDevlieghere as a code owner May 22, 2024 00:43

adrian-prantl requested review from bulbazord and removed request for JDevlieghere May 22, 2024 00:43

llvmbot added the lldb label May 22, 2024

adrian-prantl requested a review from JDevlieghere May 22, 2024 00:43

walter-erquinigo approved these changes May 22, 2024

View reviewed changes

Change GetChildCompilerTypeAtIndex to return Expected (NFC)

a8503fb

adrian-prantl force-pushed the child-compilertype-expected branch from 8a6202f to a8503fb Compare May 22, 2024 15:44

adrian-prantl merged commit ac1dc05 into llvm:main May 22, 2024
4 checks passed

AbdAlRahmanGad mentioned this pull request Sep 5, 2024

[lldb] Change ValueObject::AddressOf() to return Expected (NFC) #106831

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Change GetChildCompilerTypeAtIndex to return Expected (NFC) #92979

Change GetChildCompilerTypeAtIndex to return Expected (NFC) #92979

Uh oh!

adrian-prantl commented May 22, 2024

Uh oh!

llvmbot commented May 22, 2024

Uh oh!

github-actions bot commented May 22, 2024 •

edited

Loading

Uh oh!

walter-erquinigo left a comment

Uh oh!

Uh oh!

Uh oh!

Change GetChildCompilerTypeAtIndex to return Expected (NFC) #92979

Change GetChildCompilerTypeAtIndex to return Expected (NFC) #92979

Uh oh!

Conversation

adrian-prantl commented May 22, 2024

Uh oh!

llvmbot commented May 22, 2024

Uh oh!

github-actions bot commented May 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

walter-erquinigo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented May 22, 2024 •

edited

Loading