Skip to content

[lldb] Change interface of StructuredData::Array::GetItemAtIndexAsInteger #71993

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 8, 2024

Conversation

bulbazord
Copy link
Member

This is a follow-up to (#71613) and (#71961).

@llvmbot
Copy link
Member

llvmbot commented Nov 10, 2023

@llvm/pr-subscribers-lldb

Author: Alex Langford (bulbazord)

Changes

This is a follow-up to (#71613) and (#71961).


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

4 Files Affected:

  • (modified) lldb/include/lldb/Utility/StructuredData.h (+7-21)
  • (modified) lldb/source/Breakpoint/BreakpointResolverName.cpp (+4-4)
  • (modified) lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp (+3-2)
  • (modified) lldb/source/Target/DynamicRegisterInfo.cpp (+7-8)
diff --git a/lldb/include/lldb/Utility/StructuredData.h b/lldb/include/lldb/Utility/StructuredData.h
index 8d0ae372f43c6bf..35fcfca8dd98c67 100644
--- a/lldb/include/lldb/Utility/StructuredData.h
+++ b/lldb/include/lldb/Utility/StructuredData.h
@@ -221,31 +221,17 @@ class StructuredData {
     }
 
     template <class IntType>
-    bool GetItemAtIndexAsInteger(size_t idx, IntType &result) const {
-      ObjectSP value_sp = GetItemAtIndex(idx);
-      if (value_sp.get()) {
+    std::optional<IntType> GetItemAtIndexAsInteger(size_t idx) const {
+      if (auto item_sp = GetItemAtIndex(idx)) {
         if constexpr (std::numeric_limits<IntType>::is_signed) {
-          if (auto signed_value = value_sp->GetAsSignedInteger()) {
-            result = static_cast<IntType>(signed_value->GetValue());
-            return true;
-          }
+          if (auto *signed_value = item_sp->GetAsSignedInteger())
+            return static_cast<IntType>(signed_value->GetValue());
         } else {
-          if (auto unsigned_value = value_sp->GetAsUnsignedInteger()) {
-            result = static_cast<IntType>(unsigned_value->GetValue());
-            return true;
-          }
+          if (auto *unsigned_value = item_sp->GetAsUnsignedInteger())
+            return static_cast<IntType>(unsigned_value->GetValue());
         }
       }
-      return false;
-    }
-
-    template <class IntType>
-    bool GetItemAtIndexAsInteger(size_t idx, IntType &result,
-                                 IntType default_val) const {
-      bool success = GetItemAtIndexAsInteger(idx, result);
-      if (!success)
-        result = default_val;
-      return success;
+      return {};
     }
 
     std::optional<llvm::StringRef> GetItemAtIndexAsString(size_t idx) const {
diff --git a/lldb/source/Breakpoint/BreakpointResolverName.cpp b/lldb/source/Breakpoint/BreakpointResolverName.cpp
index 0097046cf511b5d..07ff597e10a5ca0 100644
--- a/lldb/source/Breakpoint/BreakpointResolverName.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolverName.cpp
@@ -161,14 +161,14 @@ BreakpointResolverSP BreakpointResolverName::CreateFromStructuredData(
         error.SetErrorString("BRN::CFSD: name entry is not a string.");
         return nullptr;
       }
-      std::underlying_type<FunctionNameType>::type fnt;
-      success = names_mask_array->GetItemAtIndexAsInteger(i, fnt);
-      if (!success) {
+      auto maybe_fnt = names_mask_array->GetItemAtIndexAsInteger<
+          std::underlying_type<FunctionNameType>::type>(i);
+      if (!maybe_fnt) {
         error.SetErrorString("BRN::CFSD: name mask entry is not an integer.");
         return nullptr;
       }
       names.push_back(std::string(*maybe_name));
-      name_masks.push_back(static_cast<FunctionNameType>(fnt));
+      name_masks.push_back(static_cast<FunctionNameType>(*maybe_fnt));
     }
 
     std::shared_ptr<BreakpointResolverName> resolver_sp =
diff --git a/lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp b/lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
index 2a35256a6fb0bf3..72293c5331f40da 100644
--- a/lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
+++ b/lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
@@ -592,9 +592,10 @@ addr_t InstrumentationRuntimeTSan::GetFirstNonInternalFramePc(
     if (skip_one_frame && i == 0)
       continue;
 
-    addr_t addr;
-    if (!trace_array->GetItemAtIndexAsInteger(i, addr))
+    auto maybe_addr = trace_array->GetItemAtIndexAsInteger<addr_t>(i);
+    if (!maybe_addr)
       continue;
+    addr_t addr = *maybe_addr;
 
     lldb_private::Address so_addr;
     if (!process_sp->GetTarget().GetSectionLoadList().ResolveLoadAddress(
diff --git a/lldb/source/Target/DynamicRegisterInfo.cpp b/lldb/source/Target/DynamicRegisterInfo.cpp
index 7469c1d4259afc2..1a817449fa95896 100644
--- a/lldb/source/Target/DynamicRegisterInfo.cpp
+++ b/lldb/source/Target/DynamicRegisterInfo.cpp
@@ -349,10 +349,8 @@ DynamicRegisterInfo::SetRegisterInfo(const StructuredData::Dictionary &dict,
       const size_t num_regs = invalidate_reg_list->GetSize();
       if (num_regs > 0) {
         for (uint32_t idx = 0; idx < num_regs; ++idx) {
-          uint64_t invalidate_reg_num;
-          std::optional<llvm::StringRef> maybe_invalidate_reg_name =
-              invalidate_reg_list->GetItemAtIndexAsString(idx);
-          if (maybe_invalidate_reg_name) {
+          if (auto maybe_invalidate_reg_name =
+                  invalidate_reg_list->GetItemAtIndexAsString(idx)) {
             const RegisterInfo *invalidate_reg_info =
                 GetRegisterInfo(*maybe_invalidate_reg_name);
             if (invalidate_reg_info) {
@@ -365,10 +363,11 @@ DynamicRegisterInfo::SetRegisterInfo(const StructuredData::Dictionary &dict,
                      "\"%s\" while parsing register \"%s\"\n",
                      maybe_invalidate_reg_name->str().c_str(), reg_info.name);
             }
-          } else if (invalidate_reg_list->GetItemAtIndexAsInteger(
-                         idx, invalidate_reg_num)) {
-            if (invalidate_reg_num != UINT64_MAX)
-              m_invalidate_regs_map[i].push_back(invalidate_reg_num);
+          } else if (auto maybe_invalidate_reg_num =
+                         invalidate_reg_list->GetItemAtIndexAsInteger<uint64_t>(
+                             idx)) {
+            if (*maybe_invalidate_reg_num != UINT64_MAX)
+              m_invalidate_regs_map[i].push_back(*maybe_invalidate_reg_num);
             else
               printf("error: 'invalidate-regs' list value wasn't a valid "
                      "integer\n");

Copy link
Member

@JDevlieghere JDevlieghere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bulbazord bulbazord merged commit 16b8a0d into llvm:main Jan 8, 2024
@bulbazord bulbazord deleted the get-item-at-index-as-integer branch January 8, 2024 21:31
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants