aaron.ballman added inline comments.

================
Comment at: clang-tidy/misc/CopyConstructorInitCheck.cpp:24
+    withInitializer(cxxConstructExpr(unless(hasDescendant(implicitCastExpr())))
+                        .bind("cruct-expr")));
+
----------------
You pick a more readable name than `cruct-expr`, like `construct-expr`?


================
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(
----------------
Wouldn't registering this matcher achieve the same goal instead of needing to 
re-match?


================
Comment at: clang-tidy/misc/CopyConstructorInitCheck.cpp:59
+      // We want to write in the FixIt the template arguments too.
+      if (const auto *Decl = dyn_cast<ClassTemplateSpecializationDecl>(
+              Init->getBaseClass()->getAsCXXRecordDecl())) {
----------------
Please pick a name other than `Decl`, since that's a type name.


================
Comment at: clang-tidy/misc/CopyConstructorInitCheck.cpp:80
+
+  auto &SM = Result.Context->getSourceManager();
+  SourceLocation StartLoc = Ctor->getLocation();
----------------
Please do not use `auto` here.


================
Comment at: clang-tidy/misc/CopyConstructorInitCheck.cpp:95
+  if (Tok.is(tok::l_brace))
+    FixItMsg += ": ";
+  FixItMsg += FixItInitList;
----------------
Space before the colon?


================
Comment at: clang-tidy/misc/CopyConstructorInitCheck.cpp:104
+
+  diag(Tok.getLocation(),
+       "calling an inherited constructor other than the copy constructor")
----------------
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()`.


================
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) 
{};
+};
----------------
Don't we want the ctor-inits to be in the same order as the bases are specified?


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