[Lldb-commits] [PATCH] D151451: [lldb][nfc] Refactor methods with out parameter

[Felipe de Azevedo Piovezan via Phabricator via lldb-commits]

fdeazeve created this revision.
Herald added a project: All.
fdeazeve requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Currently, the method `GetAttributeAddressRanges` takes a DWARFRangeList as a
parameter, just to immediately clear it. The method also returns the size of
this list. Such an API was obfuscating the intent of the call sites (it's not
clear from the method name what it returns) and it was obfuscating redundant
checks on the size of the list.

This commit refactors the method to return the list and to also make the call
sites use the more explicit `IsEmpty` method.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151451

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -840,9 +840,9 @@
   if (!dwarf_ast)
     return nullptr;
 
-  DWARFRangeList ranges;
-  if (die.GetDIE()->GetAttributeAddressRanges(die.GetCU(), ranges,
-                                              /*check_hi_lo_pc=*/true) == 0)
+  DWARFRangeList ranges = die.GetDIE()->GetAttributeAddressRanges(
+      die.GetCU(), /*check_hi_lo_pc=*/true);
+  if (ranges.IsEmpty())
     return nullptr;
 
   // Union of all ranges in the function DIE (if the function is
@@ -3208,10 +3208,9 @@
       DWARFDIE function_die = GetDIE(sc.function->GetID());
 
       dw_addr_t func_lo_pc = LLDB_INVALID_ADDRESS;
-      DWARFRangeList ranges;
-      if (function_die.GetDIE()->GetAttributeAddressRanges(
-              function_die.GetCU(), ranges,
-              /*check_hi_lo_pc=*/true))
+      DWARFRangeList ranges = function_die.GetDIE()->GetAttributeAddressRanges(
+          function_die.GetCU(), /*check_hi_lo_pc=*/true);
+      if (!ranges.IsEmpty())
         func_lo_pc = ranges.GetMinRangeBase(0);
       if (func_lo_pc != LLDB_INVALID_ADDRESS) {
         const size_t num_variables =
@@ -4282,4 +4281,3 @@
     args.insert({comp_unit, Args(flags)});
   }
 }
-
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -94,8 +94,8 @@
       uint64_t fail_value,
       bool check_specification_or_abstract_origin = false) const;
 
-  size_t GetAttributeAddressRanges(
-      DWARFUnit *cu, DWARFRangeList &ranges, bool check_hi_lo_pc,
+  DWARFRangeList GetAttributeAddressRanges(
+      DWARFUnit *cu, bool check_hi_lo_pc,
       bool check_specification_or_abstract_origin = false) const;
 
   const char *GetName(const DWARFUnit *cu) const;
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -636,15 +636,16 @@
   return false;
 }
 
-size_t DWARFDebugInfoEntry::GetAttributeAddressRanges(
-    DWARFUnit *cu, DWARFRangeList &ranges, bool check_hi_lo_pc,
+DWARFRangeList DWARFDebugInfoEntry::GetAttributeAddressRanges(
+    DWARFUnit *cu, bool check_hi_lo_pc,
     bool check_specification_or_abstract_origin) const {
-  ranges.Clear();
 
   DWARFFormValue form_value;
-  if (GetAttributeValue(cu, DW_AT_ranges, form_value)) {
-    ranges = GetRangesOrReportError(*cu, *this, form_value);
-  } else if (check_hi_lo_pc) {
+  if (GetAttributeValue(cu, DW_AT_ranges, form_value))
+    return GetRangesOrReportError(*cu, *this, form_value);
+
+  DWARFRangeList ranges;
+  if (check_hi_lo_pc) {
     dw_addr_t lo_pc = LLDB_INVALID_ADDRESS;
     dw_addr_t hi_pc = LLDB_INVALID_ADDRESS;
     if (GetAttributeAddressRange(cu, lo_pc, hi_pc, LLDB_INVALID_ADDRESS,
@@ -653,7 +654,7 @@
         ranges.Append(DWARFRangeList::Entry(lo_pc, hi_pc - lo_pc));
     }
   }
-  return ranges.GetSize();
+  return ranges;
 }
 
 // GetName
@@ -716,9 +717,8 @@
     DWARFUnit *cu, DWARFDebugAranges *debug_aranges) const {
   if (m_tag) {
     if (m_tag == DW_TAG_subprogram) {
-      DWARFRangeList ranges;
-      GetAttributeAddressRanges(cu, ranges,
-                                /*check_hi_lo_pc=*/true);
+      DWARFRangeList ranges =
+          GetAttributeAddressRanges(cu, /*check_hi_lo_pc=*/true);
       for (const auto &r : ranges) {
         debug_aranges->AppendRange(GetOffset(), r.GetRangeBase(),
                                    r.GetRangeEnd());
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
@@ -168,10 +168,9 @@
   }
 
   if (match_addr_range) {
-    DWARFRangeList ranges;
-    if (m_die->GetAttributeAddressRanges(m_cu, ranges,
-                                         /*check_hi_lo_pc=*/true) &&
-        ranges.FindEntryThatContains(address)) {
+    DWARFRangeList ranges =
+        m_die->GetAttributeAddressRanges(m_cu, /*check_hi_lo_pc=*/true);
+    if (ranges.FindEntryThatContains(address)) {
       check_children = true;
       switch (Tag()) {
       default:
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
@@ -40,18 +40,14 @@
 
   const dw_offset_t cu_offset = GetOffset();
   if (die) {
-    DWARFRangeList ranges;
-    const size_t num_ranges =
-        die->GetAttributeAddressRanges(this, ranges, /*check_hi_lo_pc=*/true);
-    if (num_ranges > 0) {
-      for (size_t i = 0; i < num_ranges; ++i) {
-        const DWARFRangeList::Entry &range = ranges.GetEntryRef(i);
-        debug_aranges->AppendRange(cu_offset, range.GetRangeBase(),
-                                   range.GetRangeEnd());
-      }
+    DWARFRangeList ranges =
+        die->GetAttributeAddressRanges(this, /*check_hi_lo_pc=*/true);
+    for (const DWARFRangeList::Entry &range : ranges)
+      debug_aranges->AppendRange(cu_offset, range.GetRangeBase(),
+                                 range.GetRangeEnd());
 
+    if (!ranges.IsEmpty())
       return;
-    }
   }
 
   if (debug_aranges->GetNumRanges() == num_debug_aranges) {

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits