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

[Adrian Prantl via llvm-commits]

> On Jan 11, 2017, at 11:33 AM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
> On Wed, Jan 11, 2017 at 11:08 AM Greg Clayton <clayborg at gmail.com> wrote:
>> 
>>> 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.
> 
> I'd still probably go towards the second - for orthogonality of features. Means a DWARFFormValue can be passed around and its value retrieved later, etc.
> 
> But would be good to hear from Adrian too.

No matter which version we end up using I would expect any heavy users of this to re-wrap it in something more specialized again so

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

becomes

  x = getAttrAsOr<StringRef>(Die, DW_AT_name, "")

to reclaim some readability again. From that point of view I don't think the decision of where asString lives is very important. I am wondering how to pass any default value other than an empty value into getValueOr(DWARFFormValue Default)? Is it easy to construct an on-the-fly DWARFFormValue for e.g., a string?

>  
> 
> Another option that Adrian might have thoughts on, would be to remove the Optional wrapper around the DWARFFormValue from getAttributeValue. It's not something I'd be totally happy with - Optional's nice and explicit about the fact that there might not be a value, but if DWARFFormValue already has a "not present" state in it, we could arguably use that state to indicate that.

I think the answer to this depends on my previous question.
-- adrian