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

[David Blaikie via llvm-commits]

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.

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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170111/072cc18d/attachment.html>