jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.

LGTM.

It's a bit wrong that UserExpression::Evaluate takes an error and a 
result_valobj_sp.  What would it mean if result_valobj_sp->GetError() was 
something, but something different from the error parameter?  ValueObjects are 
supposed to carry their error with them, so really this should just always make 
a ValueObject and put the error in it.

But that wrongness is not of your making, and this patch does what should 
happen, make the ValueObject carry the significant error.  So as a short term 
fix this is fine.

It's also unfortunate that we don't have a ValueObjectError that just knows its 
error & its user-chosen name.  ValueObjectConstResult is carrying a lot of 
water at this point.  OTOH, it's the best choice of the classes we currently 
have.


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

https://reviews.llvm.org/D135998

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to