vsk added a comment.

Looks good overall, especially the testing.



================
Comment at: lldb/include/lldb/Utility/Scalar.h:107
+  /// Return the most efficient Scalar::Type for the requested size.
+  static Type GetBestType(size_t bit_size, bool sign);
+
----------------
JDevlieghere wrote:
> How about `GetTypeForBitSize`? 
nit: bool signed ?


================
Comment at: lldb/source/Expression/DWARFExpression.cpp:2571
+      if (stack.size() < 1) {
+        if (error_ptr)
+          error_ptr->SetErrorString(
----------------
JDevlieghere wrote:
> aprantl wrote:
> > JDevlieghere wrote:
> > > Can we wrap this in a lambda? 
> > Which part specifically do you mean?
> ```auto SetErrorString = [&](llvm::StringRef msg) { if (error_ptr) 
> error_ptr->SetErrorString(msg); })```
> 
> We could deduplicate even more code by using a macro that also covers the 
> check around it and does the return, but I'm not sure if that's really worth 
> it. 
Oh, like LLDB_LOG? Seems like a nice idea. I think I need this for D67376, so I 
might do it if no one else has started already. It'd be great to use that here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67369



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

Reply via email to