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

LGTM. It fixes the issue at hand and gets us closer to conforming to the 
standard.



================
Comment at: lldb/source/Expression/DWARFExpression.cpp:948
+  // there.
+  auto to_generic = [&](auto v) {
+    bool is_signed = std::is_signed<decltype(v)>::value;
----------------
Can't wait for C++20's templated lambdas ;-) 


================
Comment at: lldb/source/Expression/DWARFExpression.cpp:1270
     case DW_OP_const1u:
-      stack.push_back(Scalar((uint8_t)opcodes.GetU8(&offset)));
+      stack.emplace_back(to_generic(opcodes.GetU8(&offset)));
       break;
----------------
labath wrote:
> dblaikie wrote:
> > Why the change to emplace_back? Generally I'd advocate for push_back where 
> > possible, since it can't invoke explicit conversions which can require more 
> > scrutiny (eg: a vector of unique_ptrs can have a unique_ptr rvalue 
> > push_back'd, but can't have a raw pointer push_back'd (a raw pointer would 
> > have to be emplace_back'd) - so when I see push_back I can assume it's a 
> > "safe" operation)
> I don't know. I wanted to be fancy? Normally, I am also against the overuse 
> of emplace_back...
TIL


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90840

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

Reply via email to