<div dir="ltr">The coding standard doesn't cover that kind of thing. It doesn't ban me from writing extremely long functions or name variables in Spanish or whatnot. It's just that which is better and readable code.</div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Feb 3, 2015 at 4:10 PM, Davide Italiano <span dir="ltr"><<a href="mailto:davide@freebsd.org" target="_blank">davide@freebsd.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Tue, Feb 3, 2015 at 4:01 PM, Rui Ueyama <<a href="mailto:ruiu@google.com">ruiu@google.com</a>> wrote:<br>
> On Tue, Feb 3, 2015 at 3:44 PM, Davide Italiano <<a href="mailto:davide@freebsd.org">davide@freebsd.org</a>> wrote:<br>
>><br>
>> Author: davide<br>
>> Date: Tue Feb  3 17:44:33 2015<br>
>> New Revision: 228069<br>
>><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=228069&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=228069&view=rev</a><br>
>> Log:<br>
>> Avoid two function calls of file() when not needed.<br>
>><br>
>> Reported by:    ruiu<br>
>><br>
>> Modified:<br>
>>     lld/trunk/lib/Core/DefinedAtom.cpp<br>
>><br>
>> Modified: lld/trunk/lib/Core/DefinedAtom.cpp<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Core/DefinedAtom.cpp?rev=228069&r1=228068&r2=228069&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Core/DefinedAtom.cpp?rev=228069&r1=228068&r2=228069&view=diff</a><br>
>><br>
>> ==============================================================================<br>
>> --- lld/trunk/lib/Core/DefinedAtom.cpp (original)<br>
>> +++ lld/trunk/lib/Core/DefinedAtom.cpp Tue Feb  3 17:44:33 2015<br>
>> @@ -83,12 +83,15 @@ DefinedAtom::ContentPermissions DefinedA<br>
>><br>
>>  bool DefinedAtom::compareByPosition(const DefinedAtom *lhs,<br>
>>                                      const DefinedAtom *rhs) {<br>
>> -  const File *lhsFile = &lhs->file();<br>
>> -  const File *rhsFile = &rhs->file();<br>
>> +  const File *lhsFile;<br>
>> +  const File *rhsFile;<br>
>><br>
>>    if (lhs == rhs)<br>
>>      return false;<br>
>><br>
>> +  lhsFile = &lhs->file();<br>
>> +  rhsFile = &rhs->file();<br>
>> +<br>
>>    if (lhsFile->ordinal() != rhsFile->ordinal())<br>
>>      return lhsFile->ordinal() < rhsFile->ordinal();<br>
>>    assert(lhs->ordinal() != rhs->ordinal());<br>
><br>
><br>
> Oh, the new code looks like too C89-ish. You could have intermingled<br>
> declarations and code like this. It's better to do pre-commit code review,<br>
> perhaps?<br>
><br>
<br>
</div></div>FWIW, I haven't seen anything against that in<br>
<a href="http://llvm.org/docs/CodingStandards.html" target="_blank">http://llvm.org/docs/CodingStandards.html</a> , unless I overlooked (which<br>
might be possible).<br>
I can change that again if you really want.<br>
<span class="HOEnZb"><font color="#888888"><br>
--<br>
Davide<br>
<br>
"There are no solved problems; there are only problems that are more<br>
or less solved" -- Henri Poincare<br>
</font></span></blockquote></div><br></div>