[clang-tidy] improve robustness of the member initializer detection in modernize-use-default-member-init (llvm#159392)

Fixed llvm#156295
Fixed llvm#122480
Fixed llvm#160394
The original ast matcher based member initializer detection is bug
prone. In this PR we introduce a new recusively detection which can be
easier extended to detect whether we can move the initializer to member

Author: HerrCai0907

clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp

@@ -8,17 +8,57 @@
 
 #include "UseDefaultMemberInitCheck.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/Expr.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Lex/Lexer.h"
+#include "llvm/ADT/TypeSwitch.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang::tidy::modernize {
 
+static bool isExprAllowedInMemberInit(const Expr *E) {
+  if (!E)
+    return false;
+  return llvm::TypeSwitch<const Expr *, bool>(E)
+      .Case<IntegerLiteral, FloatingLiteral, CXXBoolLiteralExpr,
+            CXXNullPtrLiteralExpr, CharacterLiteral, StringLiteral>(
+          [](const auto *) { return true; })
+      .Case<ImplicitValueInitExpr>([](const auto *) { return true; })
+      .Case<ParenExpr>([](const ParenExpr *PE) {
+        return isExprAllowedInMemberInit(PE->getSubExpr());
+      })
+      .Case<UnaryOperator>([](const UnaryOperator *UO) {
+        return isExprAllowedInMemberInit(UO->getSubExpr());
+      })
+      .Case<BinaryOperator>([](const BinaryOperator *BO) {
+        return isExprAllowedInMemberInit(BO->getLHS()) &&
+               isExprAllowedInMemberInit(BO->getRHS());
+      })
+      .Case<CastExpr>([](const CastExpr *CE) {
+        return isExprAllowedInMemberInit(CE->getSubExpr());
+      })
+      .Case<DeclRefExpr>([](const DeclRefExpr *DRE) {
+        if (const ValueDecl *D = DRE->getDecl()) {
+          if (isa<EnumConstantDecl>(D))
+            return true;
+          if (const auto *VD = dyn_cast<VarDecl>(D))
+            return VD->isConstexpr() || VD->getStorageClass() == SC_Static;
+        }
+        return false;
+      })
+      .Default(false);
+}
+
 namespace {
+
 AST_MATCHER_P(InitListExpr, initCountIs, unsigned, N) {
   return Node.getNumInits() == N;
 }
+
+AST_MATCHER(Expr, allowedInitExpr) { return isExprAllowedInMemberInit(&Node); }
+
 } // namespace
 
 static StringRef getValueOfValueInit(const QualType InitType) {
@@ -206,30 +246,10 @@ void UseDefaultMemberInitCheck::storeOptions(
 }
 
 void UseDefaultMemberInitCheck::registerMatchers(MatchFinder *Finder) {
-  auto NumericLiteral = anyOf(integerLiteral(), floatLiteral());
-  auto UnaryNumericLiteral = unaryOperator(hasAnyOperatorName("+", "-"),
-                                           hasUnaryOperand(NumericLiteral));
-
-  auto ConstExprRef = varDecl(anyOf(isConstexpr(), isStaticStorageClass()));
-  auto ImmutableRef =
-      declRefExpr(to(decl(anyOf(enumConstantDecl(), ConstExprRef))));
-
-  auto BinaryNumericExpr = binaryOperator(
-      hasOperands(anyOf(NumericLiteral, ImmutableRef, binaryOperator()),
-                  anyOf(NumericLiteral, ImmutableRef, binaryOperator())));
-
-  auto InitBase =
-      anyOf(stringLiteral(), characterLiteral(), NumericLiteral,
-            UnaryNumericLiteral, cxxBoolLiteral(), cxxNullPtrLiteralExpr(),
-            implicitValueInitExpr(), ImmutableRef, BinaryNumericExpr);
-
-  auto ExplicitCastExpr = castExpr(hasSourceExpression(InitBase));
-  auto InitMatcher = anyOf(InitBase, ExplicitCastExpr);
-
-  auto Init =
-      anyOf(initListExpr(anyOf(allOf(initCountIs(1), hasInit(0, InitMatcher)),
-                               initCountIs(0), hasType(arrayType()))),
-            InitBase, ExplicitCastExpr);
+  auto Init = anyOf(
+      initListExpr(anyOf(allOf(initCountIs(1), hasInit(0, allowedInitExpr())),
+                         initCountIs(0), hasType(arrayType()))),
+      allowedInitExpr());
 
   Finder->addMatcher(
       cxxConstructorDecl(forEachConstructorInitializer(

clang-tools-extra/docs/ReleaseNotes.rst

@@ -301,6 +301,10 @@ Changes in existing checks
   uses of non-standard ``enable_if`` with a signature different from
   ``std::enable_if`` (such as ``boost::enable_if``).
 
+- Improved :doc:`modernize-use-default-member-init
+  <clang-tidy/checks/modernize/use-default-member-init>` check to
+  enhance the robustness of the member initializer detection.
+
 - Improved :doc:`modernize-use-designated-initializers
   <clang-tidy/checks/modernize/use-designated-initializers>` check to
   suggest using designated initializers for aliased aggregate types.

clang-tools-extra/test/clang-tidy/checkers/modernize/use-default-member-init.cpp

@@ -596,3 +596,26 @@ class DefaultMemberInitWithArithmetic {
 };
 
 } //namespace PR122480
+
+namespace GH156295 {
+
+class NotFix {
+  NotFix(int v) : x(0 + 0 + (0 * 0 * (((((((v)))) - 20))) + 10)) {}
+  int x;
+};
+
+class ShouldFix {
+  ShouldFix(int v) : x(0 + 0 + (0 * 0 * (((((((1)))) - 20))) + 10)) {}
+  int x;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use default member initializer for 'x' [modernize-use-default-member-init]
+  // CHECK-FIXES: int x{0 + 0 + (0 * 0 * (((((((1)))) - 20))) + 10)};
+};
+
+} // namespace GH156295
+
+namespace GH160394 {
+struct A {
+    A(int i) : f((i & 0x1f) == 1) {}
+    bool f;
+};
+} // namespace GH160394