[Lldb-commits] [PATCH] D62425: [DWARFExpression] Remove ctor that takes just a compile unit.

Greg Clayton via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Tue May 28 08:05:33 PDT 2019

clayborg added inline comments.

Comment at: lldb/include/lldb/Expression/DWARFExpression.h:311-312
   lldb::addr_t m_loclist_slide; ///< A value used to slide the location list
-                                ///offsets so that
+                                /// offsets so that
+                                /// m_c
   ///< they are relative to the object that owns the location list
correct the comment or remove changes?

Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:508-509
               uint32_t block_length = form_value.Unsigned();
-              frame_base->SetOpcodeData(module, debug_info_data, block_offset,
-                                        block_length);
+              frame_base = new (frame_base) DWARFExpression(
+                  module, debug_info_data, cu, block_offset, block_length);
             } else {
labath wrote:
> This is not the copy assignment constructor. That would be something like:
> `*frame_base = DWARFExpression(...)`.
> This code looks pretty dodgy, as you're trampling over an already constructed object. I'm not sure whether that in itself is UB, but it definitely is not a good idea.
Use move?:
*frame_base = std::move(DWARFExpression(...));

Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:518-519
               if (loc_list_length > 0) {
-                frame_base->SetOpcodeData(module, debug_loc_data,
-                                          debug_loc_offset, loc_list_length);
+                frame_base = new (frame_base)
+                    DWARFExpression(module, debug_loc_data, cu,
+                                    debug_loc_offset, loc_list_length);
use move like suggested above?



More information about the lldb-commits mailing list