szdominik added inline comments.

================
Comment at: clang-tidy/misc/CopyConstructorInitCheck.cpp:37
+
+  // We match here because we want one warning (and FixIt) for every ctor.
+  const auto Matches = match(
----------------
aaron.ballman wrote:
> Wouldn't registering this matcher achieve the same goal instead of needing to 
> re-match?
I wanted to create //only one// FixIt to every ctor - if I move the 
forEachCtorInitializer to the register part, it would warn us twice.


================
Comment at: clang-tidy/misc/CopyConstructorInitCheck.cpp:104
+
+  diag(Tok.getLocation(),
+       "calling an inherited constructor other than the copy constructor")
----------------
aaron.ballman wrote:
> Insteaad of having to re-lex the physical source, can the AST should be 
> modified to carry the information you need if it doesn't already have it? For 
> instance, you can tell there is not initializer list by looking at 
> `CXXConstructorDecl::getNumCtorInitializers()`.
The getNumCtorInitializers method counts the generated initializers as well, 
not just the manually written ones.
My basic problem was that the AST's methods can't really distinguish the 
generated and the manually written initializers, so it's quite complicated to 
find the locations what we need. I found easier & more understandable if I 
start reading the tokens.


================
Comment at: test/clang-tidy/misc-copy-constructor-init.cpp:27
+       // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: calling an inherited 
constructor other than the copy constructor [misc-copy-constructor-init]
+       // CHECK-FIXES: X3(const X3& other): Copyable2(other), Copyable(other) 
{};
+};
----------------
aaron.ballman wrote:
> Don't we want the ctor-inits to be in the same order as the bases are 
> specified?
I think it's more readable if we put the FixIts to the beginning of the 
expression - it's easier to check that everyting is correct & it's more obvious 
what the changes are.


https://reviews.llvm.org/D33722



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to