[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

Minor inline comments.

Could you please add a test case where the constructor is defined out of line? So far in every test, the constructor //body// was inside the class. Something like:

  struct S {
    int i, j;
    S();
  };
  
  S::S() {
    i = 1;
    j = 2;
  }

I'm also particularly interested in the case when the constructor is implemented in its "own" translation unit. Where will the initialiser be moved in that case? Into the header, where the record is defined, still?



================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp:21-23
+  return isa<IfStmt>(S) || isa<SwitchStmt>(S) || isa<ForStmt>(S) ||
+         isa<WhileStmt>(S) || isa<DoStmt>(S) || isa<ReturnStmt>(S) ||
+         isa<GotoStmt>(S) || isa<CXXTryStmt>(S) || isa<CXXThrowExpr>(S);
----------------
It was you who figured out in D85728 that `isa` has changed recently and is now variadic. Then this function can be simplified with the variadic version.


================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp:39-41
+  return isa<StringLiteral>(E) || isa<CharacterLiteral>(E) ||
+         isa<IntegerLiteral>(E) || isa<FloatingLiteral>(E) ||
+         isa<CXXBoolLiteralExpr>(E) || isa<CXXNullPtrLiteralExpr>(E);
----------------
Ditto.


================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp:126
+
+  for (const auto *S : Body->body()) {
+    if (isControlStatement(S))
----------------



================
Comment at: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst:6
+
+Finds member initializations in the constructor body which can be  converted
+into member initializers of the constructor instead. This not only improves
----------------



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71199/new/

https://reviews.llvm.org/D71199