[PATCH] D26750: [clang-tidy] Add modernize-use-default-member-init check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp:33-34
+  case Type::STK_Floating:
+  case Type::STK_IntegralComplex:
+  case Type::STK_FloatingComplex:
+    return "0.0";
----------------
Do these require a literal suffix to avoid conversion?


================
Comment at: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp:62
+  auto *UnaryOp = dyn_cast<UnaryOperator>(E);
+  if (UnaryOp && UnaryOp->getOpcode() == UO_Plus) {
+    return UnaryOp->getSubExpr();
----------------
Elide braces.


================
Comment at: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp:70
+  auto *InitList = dyn_cast<InitListExpr>(E);
+  if (InitList && InitList->getNumInits() == 1) {
+    return InitList->getInit(0);
----------------
Elide braces.


================
Comment at: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp:77-78
+static bool sameValue(const Expr *E1, const Expr *E2) {
+  E1 = ignoreUnaryPlus(getInitializer(E1->IgnoreImpCasts()));
+  E2 = ignoreUnaryPlus(getInitializer(E2->IgnoreImpCasts()));
+
----------------
Do you also want to ignore paren expressions?


================
Comment at: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp:80
+
+  if (isZero(E1) && isZero(E2)) {
+    return true;
----------------
Elide braces (same elsewhere).


================
Comment at: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp:103
+    return cast<DeclRefExpr>(E1)->getDecl() == cast<DeclRefExpr>(E2)->getDecl();
+  default:
+    break;
----------------
Perhaps character and string literals as well?


================
Comment at: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp:106
+  }
+  return false;
+}
----------------
Hoist this into the `default` case and put an `llvm_unreachable()` instead?


================
Comment at: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp:119
+
+AST_MATCHER(FieldDecl, hasInClassInitializer) {
+  return Node.hasInClassInitializer();
----------------
This would be useful as a public matcher, I think. Fine to do in a follow-up patch if you prefer.


================
Comment at: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp:123
+
+void UseDefaultMemberInitCheck::registerMatchers(MatchFinder *Finder) {
+  auto Init =
----------------
This check should not register matchers for C++ < 11.


================
Comment at: docs/clang-tidy/checks/modernize-use-default-member-init.rst:6
+
+This check converts a default constructor's member initializers into default
+member initializers. Other member initializers that match the default
----------------
You should mention that the check does nothing in versions older than C++11.


================
Comment at: docs/clang-tidy/checks/modernize-use-default-member-init.rst:30
+.. note::
+  Only converts member initializers for built-in types, enums and pointers.
+  The `readability-redundant-member-init` check will remove redundant member
----------------
"enums, and pointers", because Oxford commas are the best commas. ;-)


https://reviews.llvm.org/D26750