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
