[clang-tidy] Ignore default arguments in modernize-default-member-init

[malcolm.parsons]

Summary

Default member initializers cannot refer to constructor parameters, but modernize-default-member-init was trying to when the default constructor had default arguments.

Change the check to ignore default arguments to the default constructor.

Fixes PR31524.

[aaron.ballman]

Btw, when creating a patch, it's helpful to the reviewers (especially ones who are only casually interested) to put some of the context from the bug report into the patch summary rather than only list the PR.

clang-tidy/modernize/UseDefaultMemberInitCheck.cpp

161

Any varDecl? Or do we want to limit it to only parmVarDecl instead?

int i = 12;
struct S {
  int j;
  S() : j(i) {}
};

It seems reasonable to suggest the member initialization be "fixed" to: int j = i;

[malcolm.parsons]

clang-tidy/modernize/UseDefaultMemberInitCheck.cpp

161

I would find a default member initializer that is not a constant to be surprising.
Maybe I want to limit it to enumConstantDecl.

[malcolm.parsons]

Only match DeclRefExprs to EnumConstantDecls.

[aaron.ballman]

clang-tidy/modernize/UseDefaultMemberInitCheck.cpp

161

I wouldn't; consider a global synchronization object, like a mutex (this is a relatively common thing).

[malcolm.parsons]

clang-tidy/modernize/UseDefaultMemberInitCheck.cpp

161

Note that this check doesn't consider initializations of members that are classes, e.g. std::lock_guard.
I have no plans to make it consider them.

[aaron.ballman]

clang-tidy/modernize/UseDefaultMemberInitCheck.cpp

161

Ah, okay, then it's less pressing, though the pattern is still not unheard of for non-class types. I don't have a strong opinion any longer, though.

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

224

Can we also have a positive test with an enumerator?

[malcolm.parsons]

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

224

See lines 159-166.

[aaron.ballman]

LGTM!

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

224

Ah, thanks, my eyes missed that. :-)

[malcolm.parsons]

In D28287#635442, @aaron.ballman wrote:

LGTM!

Thanks!

clang-tidy/modernize/UseDefaultMemberInitCheck.cpp

void UseDefaultMemberInitCheck::registerMatchers(MatchFinder *Finder) {
   auto Init =
       anyOf(stringLiteral(), characterLiteral(), integerLiteral(),
             unaryOperator(anyOf(hasOperatorName("+"), hasOperatorName("-")),
                           hasUnaryOperand(integerLiteral())),
             floatLiteral(),
             unaryOperator(anyOf(hasOperatorName("+"), hasOperatorName("-")),
                           hasUnaryOperand(floatLiteral())),
             cxxBoolLiteral(), cxxNullPtrLiteralExpr(), implicitValueInitExpr(),
-            declRefExpr());
+            declRefExpr(unless(to(varDecl()))));
 
   Finder->addMatcher(
       cxxConstructorDecl(
           isDefaultConstructor(), unless(isInstantiated()),
           forEachConstructorInitializer(
               allOf(forField(unless(anyOf(isBitField(),
                                           hasInClassInitializer(anything())))),
                     cxxCtorInitializer(isWritten(),

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

 };
 
 struct NegativeNotDefaultInt
 {
   NegativeNotDefaultInt(int) : i(7) {}
   int i;
 };
 
+struct NegativeDefaultArg
+{
+  NegativeDefaultArg(int i = 4) : i(i) {}
+  int i;
+};
+
 struct ExistingChar {
   ExistingChar(short) : e1(), e2(), e3(), e4() {}
   // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: member initializer for 'e1' is redundant [modernize-use-default-member-init]
   // CHECK-MESSAGES: :[[@LINE-2]]:31: warning: member initializer for 'e2' is redundant
   // CHECK-MESSAGES: :[[@LINE-3]]:37: warning: member initializer for 'e3' is redundant
   // CHECK-FIXES: ExistingChar(short) :  e4() {}
   ExistingChar(int) : e1(0), e2(0), e3(0), e4(0) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: member initializer for 'e1' is redundant