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:
> > 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.
================
Comment at: lldb/include/lldb/Symbol/Function.h:249
+/// \class CallSiteParameter Function.h "lldb/Symbol/Function.h"
+///
----------------
aprantl wrote:
> 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.
No clue. I saw it elsewhere in this file and wanted to stick to the established
format. It could be worth simplifying later, though.
================
Comment at: lldb/source/Expression/DWARFExpression.cpp:1136
+ if (parent_frame && !parent_frame->IsInlined())
+ break;
+ }
----------------
aprantl wrote:
> What does it mean if there is a null parent_frame and shouldn't we return
> false in that case?
Yes, we should detect this and fail early.
================
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)
----------------
aprantl wrote:
> because...?
Actually, rejecting DW_OP_push_object_address requires no special handling.
Instead we need a TODO about actually supporting it: that belongs in the
Evaluate_DW_OP_entry_value.
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
