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

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.


================
Comment at: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp:21
+
+static StringRef getValueInit(const CXXCtorInitializer *Init) {
+  switch (Init->getInit()->getType()->getScalarTypeKind()) {
----------------
The function name doesn't make it clear that the value is returned for zero-initialization.


================
Comment at: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp:137
+          isDefaultConstructor(),
+          unless(ast_matchers::isTemplateInstantiation()),
+          forEachConstructorInitializer(allOf(
----------------
`isInTemplateInstantiation()` is usually a better choice, since it also checks for ancestors being templates.


================
Comment at: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp:157-161
+  const auto *Default = Result.Nodes.getNodeAs<CXXCtorInitializer>("default");
+  const auto *Existing = Result.Nodes.getNodeAs<CXXCtorInitializer>("existing");
+
+  if (Default)
+    checkDefaultInit(Result, Default);
----------------
This is a more common way to do this in LLVM:
```
if (const auto *Default = Result.Nodes.getNodeAs<...>("default"))
  checkDefaultInit(Result, Default);
else if (const auto *Existing = ...)
  checkExistingInit(...);
else
  llvm_unreachable(...);
```


================
Comment at: docs/clang-tidy/checks/modernize-use-default-member-init.rst:29
+
+.. note::
+  Only converts member initializers for built-in types, enums and pointers.
----------------
Is an empty line needed after this?


https://reviews.llvm.org/D26750