>What's the purpose of this change PaddingChecker generates a report on it, yeah, you are right that it won't save too much (just a little), but it was easy to fix. Another reason why i am doing this - it reduces the number of reports produced by static analyzer (less noisy) - and since it saves smth (and i don't see other regressions) - decided to change it. (btw - the order FirstId, IsKeyDecl, MergeWith is another option and also looks natural (if i am not mistaken)).
On Thu, Sep 22, 2016 at 1:04 PM, Richard Smith <rich...@metafoo.co.uk> wrote: > 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