[PATCH] D27772: Add the ability to get attribute values as Optional<T>

Adrian Prantl via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 14 14:15:09 PST 2016


> On Dec 14, 2016, at 2:11 PM, Greg Clayton <clayborg at gmail.com> wrote:
> 
>> 
>> On Dec 14, 2016, at 1:55 PM, David Blaikie <dblaikie at gmail.com> wrote:
>> 
>> 
>> 
>> On Wed, Dec 14, 2016 at 1:51 PM Greg Clayton via Phabricator <reviews at reviews.llvm.org> wrote:
>> clayborg created this revision.
>> clayborg added reviewers: aprantl, dblaikie, echristo, probinson, llvm-commits.
>> Herald added subscribers: jgosnell, mehdi_amini.
>> 
>> When getting attributes it is sometimes nicer to use Optional<T> some of the time instead of magic values. I tried to cut over to only using the Optional values but it made many of the call sites very messy, so it makes sense the leave in the calls that can return a default value. Otherwise code that looks like this:
>> 
>>  uint64_t CallColumn = Die.getAttributeValueAsAddress(DW_AT_call_line, 0);
>> 
>> Has to be turned into:
>> 
>>  uint64_t CallColumn = 0;
>>  if (auto CallColumnValue = Die.getAttributeValueAsAddress(DW_AT_call_line))
>>      CallColumn = *CallColumnValue;
>> 
>> We could have a utility for this - either in Optional, or as a free function, so could write it as something like this:
>> 
>> auto CallColumn = Die.getAttributeValueAsAddress(DW_AT_call_line).withDefault(0);
>> 
>> for example.
>> 
>> (actually, since we'll probably want to migrate from llvm::Optional to std::optional one day (when we get it, etc) - probably best to keep it as a non-member:
>> 
>> = withDefault(..., 0);
>> 
>> )
>> 
>> Don't have to - just a thought.
> 
> I like that idea. Add something like this to Optional.h?:
> 
> template <typename T> T withDefault(const Optional<T> &Opt, T DefaultValue) {
>  if (Opt)
>    return *Opt;
>  return DefaultValue;
> }
> 
> This would be useful for simple type "T" values. Not sure if we should return a "T&" and have "const T& DefaultValue" as the second parameter?

Adding this functionality to Optional should probably be a separate stand-alone review, perhaps with a unit test.

-- adrian


More information about the llvm-commits mailing list