[Lldb-commits] [PATCH] D90840: [lldb/DWARF] Fix sizes of DW_OP_const[1248][us] and DW_OP_litN results

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: lldb/source/Expression/DWARFExpression.cpp:945
+  auto to_generic = [addr_size = opcodes.GetAddressByteSize()](auto v) {
+    bool is_signed = std::is_signed<decltype(v)>::value;
+    return Scalar(
----------------
This is probably not conforming either -- dwarf says the generic type is of "unspecified signedness", but I think it means a single global value -- not choosing signedness on a per-operand basis.

I've kept that for now, since that was the original behavior, though I didn't notice any issues with this bit removed.


================
Comment at: lldb/unittests/Expression/DWARFExpressionTest.cpp:191
 
-  // Truncate to default unspecified (pointer-sized) type.
-  EXPECT_THAT_EXPECTED(
-      t.Eval({DW_OP_const8u, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, //
-              DW_OP_convert, 0x00}),
-      llvm::HasValue(GetScalar(32, 0x44332211, not_signed)));
-  // Truncate to 32 bits.
-  EXPECT_THAT_EXPECTED(t.Eval({DW_OP_const8u, //
-                               0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88,//
+  // Leave as is.
+  EXPECT_THAT_EXPECTED(t.Eval({DW_OP_const4u, 0x11, 0x22, 0x33, 0x44, //
----------------
Had to rewrite these, as they were relying on the fact that DW_OP_const operations were not implicitly converting to the generic type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90840