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

[David Blaikie via llvm-commits]

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.


>
> The first snippet of code looks much better. But in cases where you want
> an offset that may or may not be there, the following code looks better:
>
>   if (auto StmtOffset =
> Die.getAttributeValueAsSectionOffset(DW_AT_stmt_list)) {
>     // Use StmtOffset
>   }
>
>
> https://reviews.llvm.org/D27772
>
> Files:
>   include/llvm/DebugInfo/DWARF/DWARFDie.h
>   include/llvm/DebugInfo/DWARF/DWARFUnit.h
>   lib/DebugInfo/DWARF/DWARFContext.cpp
>   lib/DebugInfo/DWARF/DWARFDie.cpp
>   lib/DebugInfo/DWARF/DWARFUnit.cpp
>   tools/dsymutil/DwarfLinker.cpp
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161214/f73aee64/attachment.html>