[PATCH] D81585: [AIX][XCOFF][Patch1] Provide decoding trace back table information API for xcoff object file for llvm-objdump -d

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/unittests/Object/XCOFFObjectFileTest.cpp:126
+
+const uint8_t V_T[] = {0x00, 0x00, 0x2A, 0x40, 0x80, 0x40, 0x01, 0x05,
+                       0x58, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x40,
----------------
Fix the name here please. I'm not sure I even understand what `V_T` stands for... How about `TTableData`?


================
Comment at: llvm/unittests/Object/XCOFFObjectFileTest.cpp:150-159
+TEST(XCOFFObjectFileTest, XCOFFTracebackTableAPIHasVectorInfo) {
+  uint64_t Size = 40;
+  Expected<XCOFFTracebackTable> TTOrErr =
+      XCOFFTracebackTable::create(V_T, Size);
+  ASSERT_THAT_EXPECTED(TTOrErr, Succeeded());
+  XCOFFTracebackTable TT = *TTOrErr;
+
----------------
The inputs to this test are identical to the previous case above. Could you simply merge the two together?


================
Comment at: llvm/unittests/Object/XCOFFObjectFileTest.cpp:158
+  EXPECT_TRUE(TT.hasVectorInfo());
+  ASSERT_FALSE(TT.getParmsType());
+}
----------------
There's nothing after this in the test that depends on this line. No need for it to be an `ASSERT_FALSE`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81585