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

[via lldb-commits]

Author: Jonas Devlieghere
Date: 2024-06-20T10:46:26-07:00
New Revision: 5e9f247c064cb2361cd641f62eb4b7049d21641a

URL: https://github.com/llvm/llvm-project/commit/5e9f247c064cb2361cd641f62eb4b7049d21641a
DIFF: https://github.com/llvm/llvm-project/commit/5e9f247c064cb2361cd641f62eb4b7049d21641a.diff

LOG: [lldb] Make LanguageRuntime::GetTypeBitSize return an optional (NFC) (#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.

Added: 
    

Modified: 
    lldb/include/lldb/Target/LanguageRuntime.h
    lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
    lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
    lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Target/LanguageRuntime.h b/lldb/include/lldb/Target/LanguageRuntime.h
index d093dccfeae85..954d454c78589 100644
--- a/lldb/include/lldb/Target/LanguageRuntime.h
+++ b/lldb/include/lldb/Target/LanguageRuntime.h
@@ -170,9 +170,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 0270c67366d1d..eb7f6232ac981 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -4744,11 +4744,11 @@ 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)) {
+          if (std::optional<uint64_t> bit_size =
+                  objc_runtime->GetTypeBitSize(GetType(qual_type)))
+            return *bit_size;
         }
       } else {
         static bool g_printed = false;