alexshap added inline comments.

Comment at: lib/Serialization/ASTReaderDecl.cpp:154
@@ -153,3 +153,3 @@
-      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).


cfe-commits mailing list

Reply via email to