[PATCH] D24754: [cleanup] Remove excessive padding from RedeclarableResult
Александр Шапошников via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 22 13:36:02 PDT 2016
>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 <richard at metafoo.co.uk>
> 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