<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Apr 14, 2016 at 9:53 AM, George Rimar <span dir="ltr"><<a href="mailto:grimar@accesssoftek.com" target="_blank">grimar@accesssoftek.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">grimar added a comment.<br>
<span class=""><br>
In <a href="http://reviews.llvm.org/D19109#401395" rel="noreferrer" target="_blank">http://reviews.llvm.org/D19109#401395</a>, @ruiu wrote:<br>
<br>
> 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?<br>
><br>
>   auto Set = [&](DefinedRegular<ELFT> *S, uint64_t V) { If (S) S->Value = V; }<br>
<br>
<br>
</span>I would prefer mine version, I dont like not only repetitions of assignments, but also additional variables declarations.<br>
And that's not real type, just typedef for pair, so I dont find this to excessive.<br></blockquote><div><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
But returning to subject - since that was already committed, do you think this should be reverted ?<br>
<div class="HOEnZb"><div class="h5"><br>
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="http://reviews.llvm.org/D19109" rel="noreferrer" target="_blank">http://reviews.llvm.org/D19109</a><br>
<br>
<br>
<br>
</div></div></blockquote></div><br></div></div>