[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:36 AM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> Would it make sense to push the value retrieval down into DWARFFormValue to separate getting the value from a DIE from getting the value into some particular type - they seem pretty orthogonal & it's odd that DWARFDie handles the type handling side of that rather than an attribute/form abstraction.
> 
> Perhaps it'd be a bit abusive, but we could even overload op[](dwarf::Attribute) on DWARFDie for easy map-like attribute lookup. Maybe 'find' would be a better name, though.
> 
> I'm OK omitting the 'attribute' part of the name entirely if everyone's OK with overloading in this context & if the callsites tend to be pretty self documenting. (mostly if you're searching for a particular attribute you've probably got the attribute name in a literal at the callsite anyway, right?)
> 
> So code like this, perhaps:
> 
>   x = Die.find(DW_AT_name).asString();
> 
> or perhaps:
> 
>   x = asString(Die.find(DW_AT_name));
> 
> (so that find/findAttr/[] whatever we call it can continue to return Optional<DWARFFormValue> which asString can pass along as NOne if it's passed None (or passed a non-string value), and otherwise decompose into a string, etc)
> 


We basically have find already in:

Optional<DWARFFormValue> DWARFDie::getAttributeValue(dwarf::Attribute);

We can easily rename it.

If find returns an optional then your first code example would need to be:

x = Die.find(DW_AT_name).getValueOr(DWARFFormValue()).asString();

The second form could be made to work but it doesn’t look as natural IMHO. 

Because of the first is awkward and the second seems less clear than just asking the DIE for the value I do think there is value in having the calls on DWARFDie. Let me know what you think.

Greg



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