What's the purpose of this change? We only put these objects in the stack, and never have many of them at once. If we don't care about size, a natural field order seems preferable to one that minimises padding.<div class="gmail_extra"><br><div class="gmail_quote">On 22 Sep 2016 12:41 pm, "Alexander Shaposhnikov" <<a href="mailto:shal1t712@gmail.com">shal1t712@gmail.com</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">alexshap added inline comments.<br>
<br>
================<br>
Comment at: lib/Serialization/<wbr>ASTReaderDecl.cpp:154<br>
@@ -153,3 +153,3 @@<br>
public:<br>
- RedeclarableResult(<wbr>GlobalDeclID FirstID, Decl *MergeWith, bool IsKeyDecl)<br>
- : FirstID(FirstID), MergeWith(MergeWith), IsKeyDecl(IsKeyDecl) {}<br>
+ RedeclarableResult(Decl *MergeWith, GlobalDeclID FirstID, bool IsKeyDecl)<br>
+ : MergeWith(MergeWith), FirstID(FirstID), IsKeyDecl(IsKeyDecl) {}<br>
----------------<br>
vsk wrote:<br>
> alexshap wrote:<br>
> > vsk wrote:<br>
> > > Why do you need to change the order of the parameters in the constructor?<br>
> > To avoid inconsistency. Here the parameters match the fields and it seemed to me that it would be better to update the order.<br>
> OK. But why not do the same thing for ObjCCategoriesVisitor?<br>
The thing is that while the order of initializers needs to match the order of fields (and compilers catch it by generating a warning (and it's awesome), the order of ctor arguments doesn't need to match the order of fields (unless we are speaking about some readability concerns etc). The tool clang-reorder-fields handles correctly aggregate types & brace initializations (updates all the call sites) but doesn't change the order of ctor arguments (for now (v0)) - sometimes (for example when some fields use several arguments, or there is a non-trivial ctor body or there are other complications) it's not always easy to reason which order of ctor arguments is "natural" - and i try to preserve the old order (especially remembering about the compatible types and some call sites like emplace_back).<br>
Having said that - yea, the case of ObjCCategoriesVisitor is simple and i will update that diff as well (thanks for pointing out).<br>
<br>
<br>
Repository:<br>
rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D24754" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D24754</a><br>
<br>
<br>
<br>
</blockquote></div></div>