[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
Tue Sep 23 20:16:22 PDT 2025
https://github.com/HerrCai0907 updated https://github.com/llvm/llvm-project/pull/159392
>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 1/3] [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
>From 775f4890fd0259354855609d85762fcb06b2c8e8 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Thu, 18 Sep 2025 23:25:42 +0800
Subject: [PATCH 2/3] Update clang-tools-extra/docs/ReleaseNotes.rst
---
clang-tools-extra/docs/ReleaseNotes.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 3e332a62abdc8..eefd687101bbe 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -284,7 +284,7 @@ Changes in existing checks
- 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.
+ enhance the robustness of the member initializer detection.
- Improved :doc:`modernize-use-designated-initializers
<clang-tidy/checks/modernize/use-designated-initializers>` check to
>From 6c216e13298d71e3bdefd8a36cb204a7bd9360dc Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Wed, 24 Sep 2025 11:16:01 +0800
Subject: [PATCH 3/3] fix
---
.../modernize/UseDefaultMemberInitCheck.cpp | 55 ++++++++++---------
.../modernize/use-default-member-init.cpp | 7 +++
2 files changed, 37 insertions(+), 25 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
index e06154578938b..0d2c3a79b9ece 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
@@ -12,38 +12,43 @@
#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 == nullptr)
+ if (!E)
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;
+ 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 {
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 4d8de6a97925b..52b15dec37cd5 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
@@ -612,3 +612,10 @@ class ShouldFix {
};
} // namespace GH156295
+
+namespace GH160394 {
+struct A {
+ A(int i) : f((i & 0x1f) == 1) {}
+ bool f;
+};
+} // namespace GH160394
More information about the cfe-commits
mailing list