[Lldb-commits] [lldb] [lldb] Make LanguageRuntime::GetTypeBitSize return an optional (NFC) (PR #96013)

[Jonas Devlieghere via lldb-commits]

https://github.com/JDevlieghere created https://github.com/llvm/llvm-project/pull/96013

Make LanguageRuntime::GetTypeBitSize return an optional. This should be NFC, though the ObjCLanguageRuntime implementation is (possibly) more defensive against returning 0.

I'm not sure if it's possible for both `m_ivar.size` and `m_ivar.offset` to be zero. Previously, we'd return 0 and cache it, only to discard it the next time when finding it in the cache, and recomputing it again. The new code will avoid putting it in the cache in the first place.

>From 663bee087c345ed0d5759906d780cb4a75facac8 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Tue, 18 Jun 2024 19:32:44 -0700
Subject: [PATCH] [lldb] Make LanguageRuntime::GetTypeBitSize return an
 optional (NFC)

Make LanguageRuntime::GetTypeBitSize return an optional. This should be
NFC, though the ObjCLanguageRuntime implementation is (possibly) more
defensive against returning 0.

I'm not sure if it's possible for both `m_ivar.size` and `m_ivar.offset`
to be zero. Previously, we'd return 0 and cache it, only to discard it
the next time when finding it in the cache, and recomputing it again.
The new code will avoid putting it in the cache in the first place.
---
 lldb/include/lldb/Target/LanguageRuntime.h    |  6 +++---
 .../ObjC/ObjCLanguageRuntime.cpp              | 21 ++++++++++---------
 .../ObjC/ObjCLanguageRuntime.h                |  6 +++---
 .../TypeSystem/Clang/TypeSystemClang.cpp      | 11 +++++-----
 4 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/lldb/include/lldb/Target/LanguageRuntime.h b/lldb/include/lldb/Target/LanguageRuntime.h
index a2a9c0163f082..58874c4188034 100644
--- a/lldb/include/lldb/Target/LanguageRuntime.h
+++ b/lldb/include/lldb/Target/LanguageRuntime.h
@@ -169,9 +169,9 @@ class LanguageRuntime : public Runtime, public PluginInterface {
     return m_process->GetTarget().GetSearchFilterForModule(nullptr);
   }
 
-  virtual bool GetTypeBitSize(const CompilerType &compiler_type,
-                              uint64_t &size) {
-    return false;
+  virtual std::optional<uint64_t>
+  GetTypeBitSize(const CompilerType &compiler_type) {
+    return {};
   }
 
   virtual void SymbolsDidLoad(const ModuleList &module_list) {}
diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
index ba52444f0c2fc..a812ffba7a648 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
@@ -350,18 +350,17 @@ ObjCLanguageRuntime::EncodingToTypeSP ObjCLanguageRuntime::GetEncodingToType() {
   return nullptr;
 }
 
-bool ObjCLanguageRuntime::GetTypeBitSize(const CompilerType &compiler_type,
-                                         uint64_t &size) {
+std::optional<uint64_t>
+ObjCLanguageRuntime::GetTypeBitSize(const CompilerType &compiler_type) {
   void *opaque_ptr = compiler_type.GetOpaqueQualType();
-  size = m_type_size_cache.Lookup(opaque_ptr);
-  // an ObjC object will at least have an ISA, so 0 is definitely not OK
-  if (size > 0)
-    return true;
+  uint64_t cached_size = m_type_size_cache.Lookup(opaque_ptr);
+  if (cached_size > 0)
+    return cached_size;
 
   ClassDescriptorSP class_descriptor_sp =
       GetClassDescriptorFromClassName(compiler_type.GetTypeName());
   if (!class_descriptor_sp)
-    return false;
+    return {};
 
   int32_t max_offset = INT32_MIN;
   uint64_t sizeof_max = 0;
@@ -377,11 +376,13 @@ bool ObjCLanguageRuntime::GetTypeBitSize(const CompilerType &compiler_type,
     }
   }
 
-  size = 8 * (max_offset + sizeof_max);
-  if (found)
+  uint64_t size = 8 * (max_offset + sizeof_max);
+  if (found && size > 0) {
     m_type_size_cache.Insert(opaque_ptr, size);
+    return size;
+  }
 
-  return found;
+  return {};
 }
 
 lldb::BreakpointPreconditionSP
diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h b/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
index 0a8b6e8d56ed0..ffe9725fa6826 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
@@ -107,7 +107,7 @@ class ObjCLanguageRuntime : public LanguageRuntime {
                                             int64_t *value_bits = nullptr,
                                             uint64_t *payload = nullptr) = 0;
     /// @}
- 
+
     virtual uint64_t GetInstanceSize() = 0;
 
     // use to implement version-specific additional constraints on pointers
@@ -321,8 +321,8 @@ class ObjCLanguageRuntime : public LanguageRuntime {
     m_negative_complete_class_cache.clear();
   }
 
-  bool GetTypeBitSize(const CompilerType &compiler_type,
-                      uint64_t &size) override;
+  std::optional<uint64_t>
+  GetTypeBitSize(const CompilerType &compiler_type) override;
 
   /// Check whether the name is "self" or "_cmd" and should show up in
   /// "frame variable".
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index dbe6238d4fe5a..f9cdf357acdde 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -4744,11 +4744,12 @@ TypeSystemClang::GetBitSize(lldb::opaque_compiler_type_t type,
       ExecutionContext exe_ctx(exe_scope);
       Process *process = exe_ctx.GetProcessPtr();
       if (process) {
-        ObjCLanguageRuntime *objc_runtime = ObjCLanguageRuntime::Get(*process);
-        if (objc_runtime) {
-          uint64_t bit_size = 0;
-          if (objc_runtime->GetTypeBitSize(GetType(qual_type), bit_size))
-            return bit_size;
+        if (ObjCLanguageRuntime *objc_runtime =
+                ObjCLanguageRuntime::Get(*process)) {
+          std::optional<uint64_t> bit_size =
+              objc_runtime->GetTypeBitSize(GetType(qual_type));
+          if (bit_size)
+            return *bit_size;
         }
       } else {
         static bool g_printed = false;