[PATCH] D24754: [cleanup] Remove excessive padding from RedeclarableResult

[Richard Smith via cfe-commits]

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>
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
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160922/5855a755/attachment.html>