vsk added inline comments.
================
Comment at: lldb/include/lldb/Symbol/Function.h:258
+
+using CallSiteParameterArray = std::unique_ptr<std::vector<CallSiteParameter>>;
+
----------------
aprantl wrote:
> vsk wrote:
> > vsk wrote:
> > > aprantl wrote:
> > > > 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?
> > > Done. And, thanks @grandinj for pointing this out, I dug a bit more and
> > > found that we're *not* doing this in Function for the CallEdge vector,
> > > but probably should be. Added a TODO there.
> > Actually, there's no need to do this in both CallEdge and Function: edges
> > are parsed lazily, but parameters aren't. Let's just leave a note about
> > this in Function.
> Is a SmallVector<0> (16 bytes on x86_64) smaller than a libcxx std::vector<>?
Oh, yes.
================
Comment at: lldb/source/Expression/DWARFExpression.cpp:1199
+ } else {
+ for (CallEdge &edge : parent_func->GetTailCallingEdges()) {
+ if (edge.GetCallee(modlist) == current_func) {
----------------
aprantl wrote:
> std::find_if or something?
I tried this, but the resulting code did not look clearer to me.
================
Comment at: lldb/source/Expression/DWARFExpression.cpp:1236
+ // expression in the call site parameter are known to have the same length.
+ // Check whether they are equal.
+ if (memcmp(subexpr_data, param_subexpr_data, subexpr_len) == 0) {
----------------
aprantl wrote:
> Here the comments are not enough for me to follow why we are doing this?
> Could you explain it to me and then add that to the comment as well?
>
> What would an example DW_OP_entry_value and matching call site parameter look
> like?
I added a worked-through example at the start of the function -- is that ok?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67376/new/
https://reviews.llvm.org/D67376
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
