[PATCH] D28386: Add the ability to iterate across all attributes in a DIE.

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp:1271
+  Error Err = Error::success();
+  for (auto &AttrValue : CUDie.attributes(&Err)) {
+    switch (AttrValue.Attr) {
----------------
clayborg wrote:
> This might be awkward, but it is how users will use it and this verifies things come out in order. I would rather test this like users will test it rather than use the range manually since this tests everything that the iterator needs.
It'll test the same functionality - the range-for loop has a pretty narrow interaction with a range & users may use it in any number of ways - the important thing is the contract that it returns a range that is usable as a range (begin, end, dereference, etc):

  auto R = CUDie.attributes(Err);
  auto I = R.begin();
  auto E = R.end();

  ASSERT_NE(E, I);
  EXPECT_EQ(DW_AT_name, I->Attr);
  EXPECT_EQ(CUPath, I->Value.getAsCString()); // change CUPath to a StringRef type for this to work correctly, I expect.
  
  ASSERT_NE(E, ++I);
  EXPECT_EQ(DW_AT_declaration, I->Attr);
  EXPECT_EQ(1ull, I->Value.getAsUnsignedConstant());

  ASSERT_NE(E, ++I);
  EXPECT_EQ(DW_AT_low_pc, I->Attr);
  EXPECT_EQ(CULowPC, I->Value.getAsAddress());

  EXPECT_EQ(E, ++I);



================
Comment at: unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp:1302
+  EXPECT_TRUE(GotLowPC);
+  EXPECT_FALSE((bool)Err);
+  if (Err) {
----------------
Could you use Chris Bieneman's work on yaml2obj to test error cases of this API? (at least one error case)


https://reviews.llvm.org/D28386