[PATCH] D28569: Remove all variants of DWARFDie::getAttributeValueAs...() that had parameters that specified default values.

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 11 09:49:21 PST 2017


dblaikie accepted this revision.
dblaikie added inline comments.
This revision is now accepted and ready to land.


================
Comment at: unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp:276-278
+  EXPECT_EQ(DieDG.getAttributeValueAsUnsignedConstant(Attr_DW_FORM_data1)
+                .getValueOr(0),
+            Data1);
----------------
Looks like all of these could use op* instead:

  EXPECT_EQ(Data1, *DieDG.getAttributeValueAsUnsignedConstant(Attr_DW_Form_data1));

Since they'll fail if the value isn't present anyway - granted, using getValueOr means they fail elegantly (with an EXPECT fail, which also allows the test to continue on to the other EXPECTS rather than assert-failing and halting immediately) & to fail in a well defined way even in non-asserts builds. But I don't think that's too important relative to the likelyhood of those failure modes.

(also, switching the arguments around probably produces better failures - the expected value is the first parameter, the value under test should be the second parameter (if I recall correctly, at least - you may want to double check the documentation for EXPECT_*))


https://reviews.llvm.org/D28569





More information about the llvm-commits mailing list