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

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 11 10:01:51 PST 2017


On Wed, Jan 11, 2017 at 9:53 AM Greg Clayton via Phabricator <
reviews at reviews.llvm.org> wrote:

> clayborg added a comment.
>
> I will leave the tests using getValueOr() for now as I would like it to
> fail nicely so we can get a complete snapshot of what is failing instead of
> crashing on any failure and not knowing how many others tests would fail.


Fair enough - another tradeoff to be aware of there is that by collapsing
the value-or-no-value states with getValueOr (or similarly with the
previous code) it makes the failures harder to understand in some ways - if
the EXPECT fails and the value is 0, is it because the value wasn't
present, or because the value was present but was 0?

All good - mostly I mentioned it because the lines seemed to be egtting a
bit long/verbose and figured it might also make the tests easier to follow
by removing some clutter.


> I will read up on EXPECT_* and make a separate change for switching arg
> order if needed.
>

Cool cool - thanks! The order of arguments catches me up every time.


>
>
> https://reviews.llvm.org/D28569
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170111/5b30f425/attachment.html>


More information about the llvm-commits mailing list