[PATCH] D19109: [ELF] - Refactoring of end/edata/etext implementation.

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 14 11:47:06 PDT 2016


On Thu, Apr 14, 2016 at 9:53 AM, George Rimar <grimar at accesssoftek.com>
wrote:

> grimar added a comment.
>
> In http://reviews.llvm.org/D19109#401395, @ruiu wrote:
>
> > I probably wouldn't do that because it doesn't seem to be worth to
> create a new type. If you don't like the repetition of `if (...) ... =
> Val;`, then how about defining a helper lambda for assignment like this?
> >
> >   auto Set = [&](DefinedRegular<ELFT> *S, uint64_t V) { If (S) S->Value
> = V; }
>
>
> I would prefer mine version, I dont like not only repetitions of
> assignments, but also additional variables declarations.
> And that's not real type, just typedef for pair, so I dont find this to
> excessive.
>

I think I actually prefer to have additional variable declaration for each
symbol because I think in this case less abstracted code is easier to read.
At least it helps readers who read only a limited part of the linker.

Since it got LGTM and submitted, I don't see immediate need to roll this
back. I'll see if it feels comfortable to me. I may edit this in future.


> But returning to subject - since that was already committed, do you think
> this should be reverted ?
>
>
> Repository:
>   rL LLVM
>
> http://reviews.llvm.org/D19109
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160414/cbe8af8d/attachment.html>


More information about the llvm-commits mailing list