Author: Congcong Cai Date: 2025-09-24T03:27:08Z New Revision: 06b49eb6d6e3344a92b2f31b9ed15cd29b739ccf
URL: https://github.com/llvm/llvm-project/commit/06b49eb6d6e3344a92b2f31b9ed15cd29b739ccf DIFF: https://github.com/llvm/llvm-project/commit/06b49eb6d6e3344a92b2f31b9ed15cd29b739ccf.diff LOG: [clang-tidy] improve robustness of the member initializer detection in modernize-use-default-member-init (#159392) Fixed #156295 Fixed #122480 Fixed #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 Added: Modified: clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/test/clang-tidy/checkers/modernize/use-default-member-init.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp index d920af7fc477b..0d2c3a79b9ece 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp +++ b/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( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index bc916396a14ca..d13f83079e591 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -301,6 +301,10 @@ Changes in existing checks uses of non-standard ``enable_if`` with a signature diff erent 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. 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..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 @@ -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 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
