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

[Greg Clayton via llvm-commits]

> On Jan 11, 2017, at 10:01 AM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
> On Wed, Jan 11, 2017 at 9:53 AM Greg Clayton via Phabricator <reviews at reviews.llvm.org <mailto: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.


The length of the DWARFDie getAttributeValueAs functions, which were ported from DWARFDebugInfoEntry, was a patch I was thinking about. Does anyone have an objection to shortening all of the DWARFDie::getAttributeValueAsXXX() calls to have shorter names?

I was thinking:

From the current:

  Optional<DWARFFormValue> getAttributeValue(dwarf::Attribute Attr) const;
  const char *getAttributeValueAsString(dwarf::Attribute Attr, const char *FailValue) const;
  Optional<uint64_t> getAttributeValueAsAddress(dwarf::Attribute Attr) const;
  Optional<int64_t> getAttributeValueAsSignedConstant(dwarf::Attribute Attr) const;
  Optional<uint64_t> getAttributeValueAsUnsignedConstant(dwarf::Attribute Attr) const;
  Optional<uint64_t> getAttributeValueAsReference(dwarf::Attribute Attr) const;
  Optional<uint64_t> getAttributeValueAsSectionOffset(dwarf::Attribute Attr) const;
  DWARFDie getAttributeValueAsReferencedDie(dwarf::Attribute Attr) const;

It would be nice to change “getAttributeValue” to “getAttr” and remove the “Constant” from UnsignedConstant” and SignedConstant”:

The shorter versions would look like:

  Optional<DWARFFormValue> getAttr(dwarf::Attribute Attr) const;
  const char *getAttrAsString(dwarf::Attribute Attr, const char *FailValue) const;
  Optional<uint64_t> getAttrAsAddress(dwarf::Attribute Attr) const;
  Optional<int64_t> getAttrAsSigned(dwarf::Attribute Attr) const;
  Optional<uint64_t> getAttrAsUnsigned(dwarf::Attribute Attr) const;
  Optional<uint64_t> getAttrAsReference(dwarf::Attribute Attr) const;
  Optional<uint64_t> getAttrAsSectionOffset(dwarf::Attribute Attr) const;
  DWARFDie getAttrAsDie(dwarf::Attribute Attr) const;

This would help a lot of the DWARF code out. Any thoughts or objections?

>  
> 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 <https://reviews.llvm.org/D28569>
> 
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170111/d23f6917/attachment.html>