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.
On 22 Sep 2016 12:41 pm, "Alexander Shaposhnikov" <shal1t...@gmail.com> wrote: > alexshap added inline comments. > > ================ > Comment at: lib/Serialization/ASTReaderDecl.cpp:154 > @@ -153,3 +153,3 @@ > public: > - RedeclarableResult(GlobalDeclID FirstID, Decl *MergeWith, bool > IsKeyDecl) > - : FirstID(FirstID), MergeWith(MergeWith), IsKeyDecl(IsKeyDecl) > {} > + RedeclarableResult(Decl *MergeWith, GlobalDeclID FirstID, bool > IsKeyDecl) > + : MergeWith(MergeWith), FirstID(FirstID), IsKeyDecl(IsKeyDecl) > {} > ---------------- > vsk wrote: > > alexshap wrote: > > > vsk wrote: > > > > Why do you need to change the order of the parameters in the > constructor? > > > To avoid inconsistency. Here the parameters match the fields and it > seemed to me that it would be better to update the order. > > OK. But why not do the same thing for ObjCCategoriesVisitor? > 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). > Having said that - yea, the case of ObjCCategoriesVisitor is simple and i > will update that diff as well (thanks for pointing out). > > > Repository: > rL LLVM > > https://reviews.llvm.org/D24754 > > > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits