[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.


================
Comment at: lldb/include/lldb/Symbol/Function.h:258
+
+using CallSiteParameterArray = std::unique_ptr<std::vector<CallSiteParameter>>;
+
----------------
vsk wrote:
> grandinj wrote:
> > the way this is being used seems to indicate it can be 
> >    std::vector<CallSiteParameter>
> > no need for unique_ptr
> That's a totally fair point. The reason I've used unique_ptr here is to save space in CallEdge in the common case, where no call site information is loaded for the function. Call site info is lazily parsed, so we'd like to take a minimal memory hit for functions that aren't in a backtrace.
> 
> Also, note that using a pointer allows for a further PointerIntPair memory optimization mentioned below.
Can you document this decision up there?


================
Comment at: lldb/include/lldb/Symbol/Function.h:249
 
+/// \class CallSiteParameter Function.h "lldb/Symbol/Function.h"
+///
----------------
Out of curiosity: What's the effect of this line? It appears to have totally redundant information in it that Doxygen should already know about.


================
Comment at: lldb/source/Expression/DWARFExpression.cpp:1136
+    if (parent_frame && !parent_frame->IsInlined())
+      break;
+  }
----------------
What does it mean if there is a null parent_frame and shouldn't we return false in that case?


================
Comment at: lldb/source/Expression/DWARFExpression.cpp:2672
     case DW_OP_push_object_address:
+      // TODO: Reject DW_OP_push_object_address within entry value exprs.
       if (object_address_ptr)
----------------
because...?


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

https://reviews.llvm.org/D67376