[PATCH] D47414: [llvm-objcopy] Fix null symbol handling

Jake Ehrlich via llvm-commits llvm-commits at lists.llvm.org
Thu May 31 08:42:17 PDT 2018


Don't wait on me until June 10th. I'm just skimming emails.

On Thu, May 31, 2018, 4:09 PM James Henderson via Phabricator <
reviews at reviews.llvm.org> wrote:

> jhenderson accepted this revision.
> jhenderson added a comment.
> This revision is now accepted and ready to land.
>
> LGTM with the inline comments, but please wait for @alexshap or
> @jakehehrlich to confirm they're happy.
>
>
>
> ================
> Comment at: test/tools/llvm-objcopy/null-symbol.test:21
> +#CHECK-NEXT:  Symbol {
> +#CHECK-NEXT:    Name:{{[ ]*}}(0)
> +#CHECK-NEXT:    Value: 0x0
> ----------------
> I think this can be simplified to `Name: (0)`. Whitespace (except new
> lines) is not significant for FileCheck, except to divide other
> non-whitespace, i.e. `Name:    (0)` (many spaces) is identical to `Name:
> (0)` (one space) but not `Name:(0)` (no spaces).
>
>
> ================
> Comment at: tools/llvm-objcopy/Object.cpp:203-205
> +  for (auto Sym = std::begin(Symbols) + 1, E = std::end(Symbols); Sym !=
> E;
> +       ++Sym)
> +    Callable(**Sym);
> ----------------
> Hmm... now that I think about it, could we use std::for_each here?
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D47414
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180531/45b74b80/attachment.html>


More information about the llvm-commits mailing list