From 4b48bee51493f0f7b48f45c8da213bf70d496b69 Mon Sep 17 00:00:00 2001 From: Congcong Cai 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(E)) + return true; + if (isa(E)) + return true; + if (const auto *PE = dyn_cast(E)) + return isExprAllowedInMemberInit(PE->getSubExpr()); + if (const auto *UO = dyn_cast(E); UO && UO->isArithmeticOp()) + return isExprAllowedInMemberInit(UO->getSubExpr()); + if (const auto *BO = dyn_cast(E)) + return isExprAllowedInMemberInit(BO->getLHS()) && + isExprAllowedInMemberInit(BO->getRHS()); + if (const auto *CE = dyn_cast(E)) + return isExprAllowedInMemberInit(CE->getSubExpr()); + if (const auto *DRE = dyn_cast(E)) { + if (const ValueDecl *D = DRE->getDecl()) { + if (isa(D)) + return true; + if (const auto *VD = dyn_cast(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 + ` check to + improve the robustness of the member initializer detection. + - Improved :doc:`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 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 ` check to - improve the robustness of the member initializer detection. + enhance the robustness of the member initializer detection. - Improved :doc:`modernize-use-designated-initializers ` check to From 6c216e13298d71e3bdefd8a36cb204a7bd9360dc Mon Sep 17 00:00:00 2001 From: Congcong Cai 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(E)) - return true; - if (isa(E)) - return true; - if (const auto *PE = dyn_cast(E)) - return isExprAllowedInMemberInit(PE->getSubExpr()); - if (const auto *UO = dyn_cast(E); UO && UO->isArithmeticOp()) - return isExprAllowedInMemberInit(UO->getSubExpr()); - if (const auto *BO = dyn_cast(E)) - return isExprAllowedInMemberInit(BO->getLHS()) && - isExprAllowedInMemberInit(BO->getRHS()); - if (const auto *CE = dyn_cast(E)) - return isExprAllowedInMemberInit(CE->getSubExpr()); - if (const auto *DRE = dyn_cast(E)) { - if (const ValueDecl *D = DRE->getDecl()) { - if (isa(D)) - return true; - if (const auto *VD = dyn_cast(D)) - return VD->isConstexpr() || VD->getStorageClass() == SC_Static; - } - return false; - } - return false; + return llvm::TypeSwitch(E) + .Case( + [](const auto *) { return true; }) + .Case([](const auto *) { return true; }) + .Case([](const ParenExpr *PE) { + return isExprAllowedInMemberInit(PE->getSubExpr()); + }) + .Case([](const UnaryOperator *UO) { + return isExprAllowedInMemberInit(UO->getSubExpr()); + }) + .Case([](const BinaryOperator *BO) { + return isExprAllowedInMemberInit(BO->getLHS()) && + isExprAllowedInMemberInit(BO->getRHS()); + }) + .Case([](const CastExpr *CE) { + return isExprAllowedInMemberInit(CE->getSubExpr()); + }) + .Case([](const DeclRefExpr *DRE) { + if (const ValueDecl *D = DRE->getDecl()) { + if (isa(D)) + return true; + if (const auto *VD = dyn_cast(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