[PATCH] D33722: [clang-tidy] Add checker for undelegated copy of base classes

[Aaron Ballman via Phabricator via cfe-commits]

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