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

Congcong Cai via cfe-commits cfe-commits at lists.llvm.org
Wed Sep 17 09:00:39 PDT 2025


https://github.com/HerrCai0907 created https://github.com/llvm/llvm-project/pull/159392

Fixed #156295, #122480
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

>From 4b48bee51493f0f7b48f45c8da213bf70d496b69 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Wed, 17 Sep 2025 23:59:25 +0800
Subject: [PATCH] [clang-tidy] improve robustness of the member initializer
 detection in modernize-use-default-member-init

Fixed #156295, #122480
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
---
 .../modernize/UseDefaultMemberInitCheck.cpp   | 63 ++++++++++++-------
 clang-tools-extra/docs/ReleaseNotes.rst       |  4 ++
 .../modernize/use-default-member-init.cpp     | 16 +++++
 3 files changed, 59 insertions(+), 24 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
index d920af7fc477b..e06154578938b 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
@@ -8,17 +8,52 @@
 
 #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"
 
 using namespace clang::ast_matchers;
 
 namespace clang::tidy::modernize {
 
+static bool isExprAllowedInMemberInit(const Expr *E) {
+  if (E == nullptr)
+    return false;
+  if (isa<IntegerLiteral, FloatingLiteral, CXXBoolLiteralExpr,
+          CXXNullPtrLiteralExpr, CharacterLiteral, StringLiteral>(E))
+    return true;
+  if (isa<ImplicitValueInitExpr>(E))
+    return true;
+  if (const auto *PE = dyn_cast<ParenExpr>(E))
+    return isExprAllowedInMemberInit(PE->getSubExpr());
+  if (const auto *UO = dyn_cast<UnaryOperator>(E); UO && UO->isArithmeticOp())
+    return isExprAllowedInMemberInit(UO->getSubExpr());
+  if (const auto *BO = dyn_cast<BinaryOperator>(E))
+    return isExprAllowedInMemberInit(BO->getLHS()) &&
+           isExprAllowedInMemberInit(BO->getRHS());
+  if (const auto *CE = dyn_cast<CastExpr>(E))
+    return isExprAllowedInMemberInit(CE->getSubExpr());
+  if (const auto *DRE = dyn_cast<DeclRefExpr>(E)) {
+    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;
+  }
+  return 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 +241,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(
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index a4652a7a54858..3e332a62abdc8 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -282,6 +282,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
+  improve 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.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-default-member-init.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-default-member-init.cpp
index 015216c4a9d59..4d8de6a97925b 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-default-member-init.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-default-member-init.cpp
@@ -596,3 +596,19 @@ 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



More information about the cfe-commits mailing list