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

[Greg Clayton via Phabricator via lldb-commits]

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?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62425/new/

https://reviews.llvm.org/D62425