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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
