[Lldb-commits] [PATCH] D82772: [lldb] Fix type conversion in the Scalar getters

Tue Jun 30 05:23:19 PDT 2020

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

Beside some minor things this LGTM.

================
Comment at: lldb/source/Utility/Scalar.cpp:864
   case e_uint512:
-    return static_cast<long_double_t>(
-        llvm::APIntOps::RoundAPIntToDouble(m_integer));
----------------
Seems unrelated to the patch? Also it would be inconsistent that this is removed here and not below. FWIW this is used to suppress `-Wdouble-promotion` warnings, so it does have a purpose.

================
Comment at: lldb/unittests/Utility/ScalarTest.cpp:97
+TEST(ScalarTest, Getters) {
+  CheckConversion((int)0x87654321);
+  CheckConversion((unsigned int)0x87654321u);
----------------
If you change this to `CheckConversion<int>(0x87654321);` then that's one C-style cast less (which will make me very happy) and if someone accidentally makes this a `long` literal or so we get a compiler warning (instead of the compiler just silently truncating the thing).

Also I guess `short` and `char` is missing?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82772