[Lldb-commits] [PATCH] D85836: [lldb/Utility] Simplify and generalize Scalar class

Thu Aug 13 08:47:39 PDT 2020

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

I guess you'll have to rebase this, but LGTM

================
Comment at: lldb/include/lldb/Utility/Scalar.h:258

+  static Type PromoteToMaxType(const Scalar &lhs, const Scalar &rhs,
+                               Scalar &temp_value,
----------------
labath wrote:
> JDevlieghere wrote:
> > I really hate this signature. How do you feel about having this return a struct with the Type, the temp Scalar and the two references? That doesn't have to be part of this patch though. 
> Yeah, that has been bothering me too, but I haven't gotten around to it yet. If I understand what you mean, then the struct solution will not work correctly (without some extra goo) because the address of the temp Scalar will change while it is being returned, invalidating the pointers.
> 
> How about something like D85906, which just deals away with all the pointer business?
I think with guaranteed copy elision in C++14 it should've been fine, but I agree that it's a bit fragile. I like D85906!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85836