[PATCH] D24754: [cleanup] Remove excessive padding from RedeclarableResult
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 22 13:04:24 PDT 2016
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" <shal1t712 at gmail.com>
> alexshap added inline comments.
> Comment at: lib/Serialization/ASTReaderDecl.cpp:154
> @@ -153,3 +153,3 @@
> - RedeclarableResult(GlobalDeclID FirstID, Decl *MergeWith, bool
> - : FirstID(FirstID), MergeWith(MergeWith), IsKeyDecl(IsKeyDecl)
> + RedeclarableResult(Decl *MergeWith, GlobalDeclID FirstID, bool
> + : 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
> > > 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
> Having said that - yea, the case of ObjCCategoriesVisitor is simple and i
> will update that diff as well (thanks for pointing out).
> rL LLVM
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the cfe-commits