[PATCH] D76225: [llvm-objdump] Add constant value opcodes to variable display

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/unittests/DebugInfo/DWARF/DWARFExpressionCompactPrinterTest.cpp:161-162
+TEST_F(DWARFExpressionCompactPrinterTest, Test_OP_constu) {
+  TestExprPrinter({DW_OP_constu, 0x2a, DW_OP_stack_value}, "42");
+  TestExprPrinter({DW_OP_constu, 0x7f, DW_OP_stack_value}, "127");
+  TestExprPrinter({DW_OP_constu, 0x80, 0x01, DW_OP_stack_value}, "128");
----------------
I'd recommend adding a 0 case too.


================
Comment at: llvm/unittests/DebugInfo/DWARF/DWARFExpressionCompactPrinterTest.cpp:167-168
+TEST_F(DWARFExpressionCompactPrinterTest, Test_OP_consts) {
+  TestExprPrinter({DW_OP_consts, 0x2a, DW_OP_stack_value}, "42");
+  TestExprPrinter({DW_OP_consts, 0x7f, DW_OP_stack_value}, "-1");
+  TestExprPrinter({DW_OP_consts, 0x80, 0x01, DW_OP_stack_value}, "128");
----------------
Please add 0, max single-byte signed and min single-byte signed (i.e. -127 or whatever the value is) too.


================
Comment at: llvm/unittests/DebugInfo/DWARF/DWARFExpressionCompactPrinterTest.cpp:183-189
+  TestExprPrinter({DW_OP_const1s, 0x80, DW_OP_stack_value}, "-128");
+  TestExprPrinter({DW_OP_const2s, 0x00, 0x80, DW_OP_stack_value}, "-32768");
+  TestExprPrinter({DW_OP_const4s, 0x00, 0x00, 0x00, 0x80, DW_OP_stack_value},
+                  "-2147483648");
+  TestExprPrinter({DW_OP_const8s, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                   0x80, DW_OP_stack_value},
+                  "-9223372036854775808");
----------------
You should have the maximum value also tested for all of these. -1/0/1 cases probably make sense too, but up to you.


================
Comment at: llvm/unittests/DebugInfo/DWARF/DWARFExpressionCompactPrinterTest.cpp:193-194
+TEST_F(DWARFExpressionCompactPrinterTest, Test_OP_lit) {
+  TestExprPrinter({DW_OP_lit0, DW_OP_stack_value}, "0");
+  TestExprPrinter({DW_OP_lit31, DW_OP_stack_value}, "31");
+}
----------------
You should probably have a test case for each DW_OP_lit value.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76225