[PATCH] D37511: [dwarfdump] Verify line table prologue

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 6 10:50:21 PDT 2017


On Wed, Sep 6, 2017 at 10:47 AM Jonas Devlieghere via Phabricator <
reviews at reviews.llvm.org> wrote:

> JDevlieghere added inline comments.
>
>
> ================
> Comment at: lib/DebugInfo/DWARF/DWARFVerifier.cpp:455
> +      } else {
> +        llvm_unreachable("Invalid index?");
> +      }
> ----------------
> JDevlieghere wrote:
> > dblaikie wrote:
> > > Don't branch-to-unreachable. Use an assert instead.
> > >
> > > Is the false return from getFileNameByIndex really impossible? (has
> the FileIndex been checked for validity? What other error paths does
> getFileNameByIndex have, if any?)
> > Thanks, an assert definitely better expresses the intent. The only path
> that returns false is this:
> > ```
> >   if (Kind == FileLineInfoKind::None || !hasFileAtIndex(FileIndex))
> >     return false;
> > ```
> > The `FileLineInfoKind` is hard-coded so that can't trigger it. The index
> can't really be invalid either as we're iterating over the list of indices.
> If this returns false here it must be a programmer error.
> What about the unused variable though? I need the function call to
> populate the string, so I'd have to assign it to a bool that's only used in
> the assert.
>

Generally a "(void)Var;" next to the assert is done to suppress the unused
variable warning.


>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D37511
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170906/d9b92fed/attachment.html>


More information about the llvm-commits mailing list