[PATCH] D114995: clang-tidy: improve the 'modernize-use-default-member-init'

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

Thanks for the improvement! I've commit on your behalf in 
051847cfecaea3f55fc4f822facfbf5d21bde8dd 
.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114995/new/

https://reviews.llvm.org/D114995

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114995: clang-tidy: improve the 'modernize-use-default-member-init'

[Oleg Smolsky via Phabricator via cfe-commits]

oleg.smolsky added a comment.

In D114995#3218022 , @aaron.ballman 
wrote:

> In D114995#3218004 , @oleg.smolsky 
> wrote:
>
>> @aaron.ballman could you commit this change please? I've never had commit 
>> rights... Thanks!
>
> Sure can! Is `Oleg Smolsky ` the correct attribution 
> for the patch, or would you like me to use a different name or email address?

Thanks Aaron! Yes, everything here looks right.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114995/new/

https://reviews.llvm.org/D114995

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114995: clang-tidy: improve the 'modernize-use-default-member-init'

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D114995#3218004 , @oleg.smolsky 
wrote:

> @aaron.ballman could you commit this change please? I've never had commit 
> rights... Thanks!

Sure can! Is `Oleg Smolsky ` the correct attribution 
for the patch, or would you like me to use a different name or email address?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114995/new/

https://reviews.llvm.org/D114995

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114995: clang-tidy: improve the 'modernize-use-default-member-init'

[Oleg Smolsky via Phabricator via cfe-commits]

oleg.smolsky added a comment.

@aaron.ballman could you commit this change please? I've never had commit 
rights... Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114995/new/

https://reviews.llvm.org/D114995

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114995: clang-tidy: improve the 'modernize-use-default-member-init'

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114995/new/

https://reviews.llvm.org/D114995

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114995: clang-tidy: improve the 'modernize-use-default-member-init'

[Oleg Smolsky via Phabricator via cfe-commits]

oleg.smolsky added a comment.

Done with review changes.




Comment at: 
clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp:1
+
 //===--- UseDefaultMemberInitCheck.cpp - 
clang-tidy===//

aaron.ballman wrote:
> Spurious newline?
yep, removing it...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114995/new/

https://reviews.llvm.org/D114995

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114995: clang-tidy: improve the 'modernize-use-default-member-init'

[Oleg Smolsky via Phabricator via cfe-commits]

oleg.smolsky updated this revision to Diff 397086.
oleg.smolsky marked 2 inline comments as done.
oleg.smolsky added a comment.

Addressed review comments: explicit types and doc changes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114995/new/

https://reviews.llvm.org/D114995

Files:
  clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-default-member-init.rst
  
clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
@@ -45,6 +45,42 @@
   // CHECK-FIXES: int j{1};
 };
 
+struct PositiveNotDefaultInt {
+  PositiveNotDefaultInt(int) : i(7) {}
+  // CHECK-FIXES: PositiveNotDefaultInt(int)  {}
+  int i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use default member initializer for 'i'
+  // CHECK-FIXES: int i{7};
+};
+
+// We cannot reconcile these initializers.
+struct TwoConstructors {
+TwoConstructors(int) : i(7) {}
+TwoConstructors(int, int) : i(8) {}
+int i;
+};
+
+struct PositiveNotDefaultOOLInt {
+  PositiveNotDefaultOOLInt(int);
+  int i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use default member initializer for 'i'
+  // CHECK-FIXES: int i{7};
+};
+
+PositiveNotDefaultOOLInt::PositiveNotDefaultOOLInt(int) : i(7) {}
+// CHECK-FIXES: PositiveNotDefaultOOLInt::PositiveNotDefaultOOLInt(int)  {}
+
+struct PositiveNotDefaultOOLInt2 {
+  PositiveNotDefaultOOLInt2(int, int);
+  int i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use default member initializer for 'i'
+  // CHECK-FIXES: int i{7};
+  int j;
+};
+
+PositiveNotDefaultOOLInt2::PositiveNotDefaultOOLInt2(int, int arg) : i(7), j(arg) {}
+// CHECK-FIXES: PositiveNotDefaultOOLInt2::PositiveNotDefaultOOLInt2(int, int arg) :  j(arg) {}
+
 struct PositiveUnaryMinusInt {
   PositiveUnaryMinusInt() : j(-1) {}
   // CHECK-FIXES: PositiveUnaryMinusInt()  {}
@@ -234,12 +270,6 @@
   int i : 5;
 };
 
-struct NegativeNotDefaultInt
-{
-  NegativeNotDefaultInt(int) : i(7) {}
-  int i;
-};
-
 struct NegativeDefaultArg
 {
   NegativeDefaultArg(int i = 4) : i(i) {}
Index: clang-tools-extra/docs/clang-tidy/checks/modernize-use-default-member-init.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/modernize-use-default-member-init.rst
+++ clang-tools-extra/docs/clang-tidy/checks/modernize-use-default-member-init.rst
@@ -3,7 +3,7 @@
 modernize-use-default-member-init
 =
 
-This check converts a default constructor's member initializers into the new
+This check converts constructors' member initializers into the new
 default member initializers in C++11. Other member initializers that match the
 default member initializer are removed. This can reduce repeated code or allow
 use of '= default'.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -73,6 +73,9 @@
 - Added support for `NOLINTBEGIN` ... `NOLINTEND` comments to suppress
   Clang-Tidy warnings over multiple lines.
 
+- Generalized the `modernize-use-default-member-init` check to handle non-default
+  constructors.
+
 New checks
 ^^
 
Index: clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
@@ -212,17 +212,14 @@
 InitBase);
 
   Finder->addMatcher(
-  cxxConstructorDecl(
-  isDefaultConstructor(),
-  forEachConstructorInitializer(
-  cxxCtorInitializer(
-  forField(unless(anyOf(getLangOpts().CPlusPlus20
-? unless(anything())
-: isBitField(),
-hasInClassInitializer(anything()),
-hasParent(recordDecl(isUnion()),
-  withInitializer(Init))
-  .bind("default"))),
+  cxxConstructorDecl(forEachConstructorInitializer(
+  cxxCtorInitializer(
+  forField(unless(anyOf(
+  getLangOpts().CPlusPlus20 ? unless(anything()) : isBitField(),
+  hasInClassInitializer(anything()),
+  hasParent(recordDecl(isUnion()),
+  withInitializer(Init))
+  

[PATCH] D114995: clang-tidy: improve the 'modernize-use-default-member-init'

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Mostly just minor nits at this point. Please be sure to update the 
documentation and add a release note about the changes.




Comment at: 
clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp:1
+
 //===--- UseDefaultMemberInitCheck.cpp - 
clang-tidy===//

Spurious newline?



Comment at: 
clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp:251
+  // it is hard to reconcile their sets of member initializers.
+  auto ClassDecl = dyn_cast(Field->getParent());
+  if (std::count_if(ClassDecl->ctors().begin(), ClassDecl->ctors().end(),





Comment at: 
clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp:252-253
+  auto ClassDecl = dyn_cast(Field->getParent());
+  if (std::count_if(ClassDecl->ctors().begin(), ClassDecl->ctors().end(),
+[](const auto Ctor) { return 
!Ctor->isCopyOrMoveConstructor(); }) > 1)
+return;




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114995/new/

https://reviews.llvm.org/D114995

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114995: clang-tidy: improve the 'modernize-use-default-member-init'

[Oleg Smolsky via Phabricator via cfe-commits]

oleg.smolsky added a comment.

In D114995#3198033 , @aaron.ballman 
wrote:

> In D114995#3183240 , 
> @malcolm.parsons wrote:
>
>> In D114995#3180475 , 
>> @aaron.ballman wrote:
>>
>>> was there a reason we didn't cover that case originally or was it an 
>>> oversight/left for future work?
>>
>> It was left for future work - by only considering the initializer list of 
>> the default constructor, clang-tidy did not have to work out what to do when 
>> the constructors don't agree on what value the member init should have.
>
> Thank you for verifying! @oleg.smolsky -- this would be a very useful test 
> case to add, btw.

Yep, done.

PLAL


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114995/new/

https://reviews.llvm.org/D114995

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114995: clang-tidy: improve the 'modernize-use-default-member-init'

[Oleg Smolsky via Phabricator via cfe-commits]

oleg.smolsky updated this revision to Diff 395190.
oleg.smolsky added a comment.

Added a "two constructors" test case along with the support for that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114995/new/

https://reviews.llvm.org/D114995

Files:
  clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
@@ -45,6 +45,42 @@
   // CHECK-FIXES: int j{1};
 };
 
+struct PositiveNotDefaultInt {
+  PositiveNotDefaultInt(int) : i(7) {}
+  // CHECK-FIXES: PositiveNotDefaultInt(int)  {}
+  int i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use default member initializer for 'i'
+  // CHECK-FIXES: int i{7};
+};
+
+// We cannot reconcile these initializers.
+struct TwoConstructors {
+TwoConstructors(int) : i(7) {}
+TwoConstructors(int, int) : i(8) {}
+int i;
+};
+
+struct PositiveNotDefaultOOLInt {
+  PositiveNotDefaultOOLInt(int);
+  int i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use default member initializer for 'i'
+  // CHECK-FIXES: int i{7};
+};
+
+PositiveNotDefaultOOLInt::PositiveNotDefaultOOLInt(int) : i(7) {}
+// CHECK-FIXES: PositiveNotDefaultOOLInt::PositiveNotDefaultOOLInt(int)  {}
+
+struct PositiveNotDefaultOOLInt2 {
+  PositiveNotDefaultOOLInt2(int, int);
+  int i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use default member initializer for 'i'
+  // CHECK-FIXES: int i{7};
+  int j;
+};
+
+PositiveNotDefaultOOLInt2::PositiveNotDefaultOOLInt2(int, int arg) : i(7), j(arg) {}
+// CHECK-FIXES: PositiveNotDefaultOOLInt2::PositiveNotDefaultOOLInt2(int, int arg) :  j(arg) {}
+
 struct PositiveUnaryMinusInt {
   PositiveUnaryMinusInt() : j(-1) {}
   // CHECK-FIXES: PositiveUnaryMinusInt()  {}
@@ -234,12 +270,6 @@
   int i : 5;
 };
 
-struct NegativeNotDefaultInt
-{
-  NegativeNotDefaultInt(int) : i(7) {}
-  int i;
-};
-
 struct NegativeDefaultArg
 {
   NegativeDefaultArg(int i = 4) : i(i) {}
Index: clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
@@ -1,3 +1,4 @@
+
 //===--- UseDefaultMemberInitCheck.cpp - clang-tidy===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
@@ -212,17 +213,14 @@
 InitBase);
 
   Finder->addMatcher(
-  cxxConstructorDecl(
-  isDefaultConstructor(),
-  forEachConstructorInitializer(
-  cxxCtorInitializer(
-  forField(unless(anyOf(getLangOpts().CPlusPlus20
-? unless(anything())
-: isBitField(),
-hasInClassInitializer(anything()),
-hasParent(recordDecl(isUnion()),
-  withInitializer(Init))
-  .bind("default"))),
+  cxxConstructorDecl(forEachConstructorInitializer(
+  cxxCtorInitializer(
+  forField(unless(anyOf(
+  getLangOpts().CPlusPlus20 ? unless(anything()) : isBitField(),
+  hasInClassInitializer(anything()),
+  hasParent(recordDecl(isUnion()),
+  withInitializer(Init))
+  .bind("default"))),
   this);
 
   Finder->addMatcher(
@@ -248,6 +246,13 @@
 const MatchFinder::MatchResult , const CXXCtorInitializer *Init) {
   const FieldDecl *Field = Init->getAnyMember();
 
+  // Check whether we have multiple hand-written constructors and bomb out, as
+  // it is hard to reconcile their sets of member initializers.
+  auto ClassDecl = dyn_cast(Field->getParent());
+  if (std::count_if(ClassDecl->ctors().begin(), ClassDecl->ctors().end(),
+[](const auto Ctor) { return !Ctor->isCopyOrMoveConstructor(); }) > 1)
+return;
+
   SourceLocation StartLoc = Field->getBeginLoc();
   if (StartLoc.isMacroID() && IgnoreMacros)
 return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114995: clang-tidy: improve the 'modernize-use-default-member-init'

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D114995#3183240 , @malcolm.parsons 
wrote:

> In D114995#3180475 , @aaron.ballman 
> wrote:
>
>> was there a reason we didn't cover that case originally or was it an 
>> oversight/left for future work?
>
> It was left for future work - by only considering the initializer list of the 
> default constructor, clang-tidy did not have to work out what to do when the 
> constructors don't agree on what value the member init should have.

Thank you for verifying! @oleg.smolsky -- this would be a very useful test case 
to add, btw.

In D114995#3181486 , @oleg.smolsky 
wrote:

> Sure, adding an option is easy, if that's the consensus. What would you call 
> it?

Since we left this for future work, I don't think we need to add a 
configuration option unless a user finds a need for one.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114995/new/

https://reviews.llvm.org/D114995

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114995: clang-tidy: improve the 'modernize-use-default-member-init'

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons added a comment.

In D114995#3180475 , @aaron.ballman 
wrote:

> was there a reason we didn't cover that case originally or was it an 
> oversight/left for future work?

It was left for future work - by only considering the initializer list of the 
default constructor, clang-tidy did not have to work out what to do when the 
constructors don't agree on what value the member init should have.
The next step towards the full solution is to handle classes with only one 
non-special constructor.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114995/new/

https://reviews.llvm.org/D114995

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114995: clang-tidy: improve the 'modernize-use-default-member-init'

[Oleg Smolsky via Phabricator via cfe-commits]

oleg.smolsky added a comment.

Sure, adding an option is easy, if that's the consensus. What would you call it?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114995/new/

https://reviews.llvm.org/D114995

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114995: clang-tidy: improve the 'modernize-use-default-member-init'

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: aaron.ballman, whisperity, alexfh, 
malcolm.parsons.
aaron.ballman added a comment.
Herald added a subscriber: rnkovacs.

Adding some additional reviewers. At a high level, I think this is a reasonable 
direction to go, but I wonder if this should be hidden behind a configuration 
option or not (basically, was there a reason we didn't cover that case 
originally or was it an oversight/left for future work)?




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp:48-49
 
+struct PositiveNotDefaultInt {
+  PositiveNotDefaultInt(int) : i(7) {}
+  // CHECK-FIXES: PositiveNotDefaultInt(int)  {}

The "default" here is not about the constructor but about the argument to the 
constructor, I believe. So I think this should be renamed to 
`PositiveConstantMemInit` or something like that (similar for the names below).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114995/new/

https://reviews.llvm.org/D114995

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114995: clang-tidy: improve the 'modernize-use-default-member-init'

[Oleg Smolsky via Phabricator via cfe-commits]

oleg.smolsky added a subscriber: aaron.ballman.
oleg.smolsky added a comment.

@aaron.ballman do you happen to know who is best to review a `clang-tidy` fix?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114995/new/

https://reviews.llvm.org/D114995

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114995: clang-tidy: improve the 'modernize-use-default-member-init'

[Oleg Smolsky via Phabricator via cfe-commits]

oleg.smolsky updated this revision to Diff 391503.
oleg.smolsky added a comment.

Ran `clang-format` on the test cases.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114995/new/

https://reviews.llvm.org/D114995

Files:
  clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
@@ -45,6 +45,35 @@
   // CHECK-FIXES: int j{1};
 };
 
+struct PositiveNotDefaultInt {
+  PositiveNotDefaultInt(int) : i(7) {}
+  // CHECK-FIXES: PositiveNotDefaultInt(int)  {}
+  int i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use default member initializer 
for 'i'
+  // CHECK-FIXES: int i{7};
+};
+
+struct PositiveNotDefaultOOLInt {
+  PositiveNotDefaultOOLInt(int);
+  int i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use default member initializer 
for 'i'
+  // CHECK-FIXES: int i{7};
+};
+
+PositiveNotDefaultOOLInt::PositiveNotDefaultOOLInt(int) : i(7) {}
+// CHECK-FIXES: PositiveNotDefaultOOLInt::PositiveNotDefaultOOLInt(int)  {}
+
+struct PositiveNotDefaultOOLInt2 {
+  PositiveNotDefaultOOLInt2(int, int);
+  int i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use default member initializer 
for 'i'
+  // CHECK-FIXES: int i{7};
+  int j;
+};
+
+PositiveNotDefaultOOLInt2::PositiveNotDefaultOOLInt2(int, int arg) : i(7), 
j(arg) {}
+// CHECK-FIXES: PositiveNotDefaultOOLInt2::PositiveNotDefaultOOLInt2(int, int 
arg) :  j(arg) {}
+
 struct PositiveUnaryMinusInt {
   PositiveUnaryMinusInt() : j(-1) {}
   // CHECK-FIXES: PositiveUnaryMinusInt()  {}
@@ -234,12 +263,6 @@
   int i : 5;
 };
 
-struct NegativeNotDefaultInt
-{
-  NegativeNotDefaultInt(int) : i(7) {}
-  int i;
-};
-
 struct NegativeDefaultArg
 {
   NegativeDefaultArg(int i = 4) : i(i) {}
Index: clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
@@ -212,17 +212,14 @@
 InitBase);
 
   Finder->addMatcher(
-  cxxConstructorDecl(
-  isDefaultConstructor(),
-  forEachConstructorInitializer(
-  cxxCtorInitializer(
-  forField(unless(anyOf(getLangOpts().CPlusPlus20
-? unless(anything())
-: isBitField(),
-hasInClassInitializer(anything()),
-hasParent(recordDecl(isUnion()),
-  withInitializer(Init))
-  .bind("default"))),
+  cxxConstructorDecl(forEachConstructorInitializer(
+  cxxCtorInitializer(
+  forField(unless(anyOf(
+  getLangOpts().CPlusPlus20 ? unless(anything()) : 
isBitField(),
+  hasInClassInitializer(anything()),
+  hasParent(recordDecl(isUnion()),
+  withInitializer(Init))
+  .bind("default"))),
   this);
 
   Finder->addMatcher(


Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
@@ -45,6 +45,35 @@
   // CHECK-FIXES: int j{1};
 };
 
+struct PositiveNotDefaultInt {
+  PositiveNotDefaultInt(int) : i(7) {}
+  // CHECK-FIXES: PositiveNotDefaultInt(int)  {}
+  int i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use default member initializer for 'i'
+  // CHECK-FIXES: int i{7};
+};
+
+struct PositiveNotDefaultOOLInt {
+  PositiveNotDefaultOOLInt(int);
+  int i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use default member initializer for 'i'
+  // CHECK-FIXES: int i{7};
+};
+
+PositiveNotDefaultOOLInt::PositiveNotDefaultOOLInt(int) : i(7) {}
+// CHECK-FIXES: PositiveNotDefaultOOLInt::PositiveNotDefaultOOLInt(int)  {}
+
+struct PositiveNotDefaultOOLInt2 {
+  PositiveNotDefaultOOLInt2(int, int);
+  int i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use default member initializer for 'i'
+  // CHECK-FIXES: int i{7};
+  int j;
+};
+
+PositiveNotDefaultOOLInt2::PositiveNotDefaultOOLInt2(int, int arg) : i(7), j(arg) {}
+// CHECK-FIXES: PositiveNotDefaultOOLInt2::PositiveNotDefaultOOLInt2(int, int arg) :  j(arg) {}
+
 struct PositiveUnaryMinusInt {
   PositiveUnaryMinusInt() : j(-1) {}
   // CHECK-FIXES: PositiveUnaryMinusInt()  {}
@@ 

[PATCH] D114995: clang-tidy: improve the 'modernize-use-default-member-init'

[Oleg Smolsky via Phabricator via cfe-commits]

oleg.smolsky updated this revision to Diff 391494.
oleg.smolsky added a comment.

Ran `clang-format`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114995/new/

https://reviews.llvm.org/D114995

Files:
  clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
@@ -45,6 +45,38 @@
   // CHECK-FIXES: int j{1};
 };
 
+struct PositiveNotDefaultInt
+{
+  PositiveNotDefaultInt(int) : i(7) {}
+  // CHECK-FIXES: PositiveNotDefaultInt(int)  {}
+  int i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use default member initializer 
for 'i'
+  // CHECK-FIXES: int i{7};
+};
+
+struct PositiveNotDefaultOOLInt
+{
+  PositiveNotDefaultOOLInt(int);
+  int i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use default member initializer 
for 'i'
+  // CHECK-FIXES: int i{7};
+};
+
+PositiveNotDefaultOOLInt::PositiveNotDefaultOOLInt(int) : i(7) {}
+// CHECK-FIXES: PositiveNotDefaultOOLInt::PositiveNotDefaultOOLInt(int)  {}
+
+struct PositiveNotDefaultOOLInt2
+{
+  PositiveNotDefaultOOLInt2(int, int);
+  int i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use default member initializer 
for 'i'
+  // CHECK-FIXES: int i{7};
+  int j;
+};
+
+PositiveNotDefaultOOLInt2::PositiveNotDefaultOOLInt2(int, int arg) : i(7), 
j(arg) {}
+// CHECK-FIXES: PositiveNotDefaultOOLInt2::PositiveNotDefaultOOLInt2(int, int 
arg) :  j(arg) {}
+
 struct PositiveUnaryMinusInt {
   PositiveUnaryMinusInt() : j(-1) {}
   // CHECK-FIXES: PositiveUnaryMinusInt()  {}
@@ -234,12 +266,6 @@
   int i : 5;
 };
 
-struct NegativeNotDefaultInt
-{
-  NegativeNotDefaultInt(int) : i(7) {}
-  int i;
-};
-
 struct NegativeDefaultArg
 {
   NegativeDefaultArg(int i = 4) : i(i) {}
Index: clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
@@ -212,17 +212,14 @@
 InitBase);
 
   Finder->addMatcher(
-  cxxConstructorDecl(
-  isDefaultConstructor(),
-  forEachConstructorInitializer(
-  cxxCtorInitializer(
-  forField(unless(anyOf(getLangOpts().CPlusPlus20
-? unless(anything())
-: isBitField(),
-hasInClassInitializer(anything()),
-hasParent(recordDecl(isUnion()),
-  withInitializer(Init))
-  .bind("default"))),
+  cxxConstructorDecl(forEachConstructorInitializer(
+  cxxCtorInitializer(
+  forField(unless(anyOf(
+  getLangOpts().CPlusPlus20 ? unless(anything()) : 
isBitField(),
+  hasInClassInitializer(anything()),
+  hasParent(recordDecl(isUnion()),
+  withInitializer(Init))
+  .bind("default"))),
   this);
 
   Finder->addMatcher(


Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
@@ -45,6 +45,38 @@
   // CHECK-FIXES: int j{1};
 };
 
+struct PositiveNotDefaultInt
+{
+  PositiveNotDefaultInt(int) : i(7) {}
+  // CHECK-FIXES: PositiveNotDefaultInt(int)  {}
+  int i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use default member initializer for 'i'
+  // CHECK-FIXES: int i{7};
+};
+
+struct PositiveNotDefaultOOLInt
+{
+  PositiveNotDefaultOOLInt(int);
+  int i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use default member initializer for 'i'
+  // CHECK-FIXES: int i{7};
+};
+
+PositiveNotDefaultOOLInt::PositiveNotDefaultOOLInt(int) : i(7) {}
+// CHECK-FIXES: PositiveNotDefaultOOLInt::PositiveNotDefaultOOLInt(int)  {}
+
+struct PositiveNotDefaultOOLInt2
+{
+  PositiveNotDefaultOOLInt2(int, int);
+  int i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use default member initializer for 'i'
+  // CHECK-FIXES: int i{7};
+  int j;
+};
+
+PositiveNotDefaultOOLInt2::PositiveNotDefaultOOLInt2(int, int arg) : i(7), j(arg) {}
+// CHECK-FIXES: PositiveNotDefaultOOLInt2::PositiveNotDefaultOOLInt2(int, int arg) :  j(arg) {}
+
 struct PositiveUnaryMinusInt {
   PositiveUnaryMinusInt() : j(-1) {}
   // CHECK-FIXES: PositiveUnaryMinusInt()  {}
@@ -234,12 

[PATCH] D114995: clang-tidy: improve the 'modernize-use-default-member-init'

[Oleg Smolsky via Phabricator via cfe-commits]

oleg.smolsky updated this revision to Diff 391470.
oleg.smolsky added a comment.

Fixing the uploaded diff...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114995/new/

https://reviews.llvm.org/D114995

Files:
  clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
@@ -45,6 +45,38 @@
   // CHECK-FIXES: int j{1};
 };
 
+struct PositiveNotDefaultInt
+{
+  PositiveNotDefaultInt(int) : i(7) {}
+  // CHECK-FIXES: PositiveNotDefaultInt(int)  {}
+  int i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use default member initializer 
for 'i'
+  // CHECK-FIXES: int i{7};
+};
+
+struct PositiveNotDefaultOOLInt
+{
+  PositiveNotDefaultOOLInt(int);
+  int i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use default member initializer 
for 'i'
+  // CHECK-FIXES: int i{7};
+};
+
+PositiveNotDefaultOOLInt::PositiveNotDefaultOOLInt(int) : i(7) {}
+// CHECK-FIXES: PositiveNotDefaultOOLInt::PositiveNotDefaultOOLInt(int)  {}
+
+struct PositiveNotDefaultOOLInt2
+{
+  PositiveNotDefaultOOLInt2(int, int);
+  int i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use default member initializer 
for 'i'
+  // CHECK-FIXES: int i{7};
+  int j;
+};
+
+PositiveNotDefaultOOLInt2::PositiveNotDefaultOOLInt2(int, int arg) : i(7), 
j(arg) {}
+// CHECK-FIXES: PositiveNotDefaultOOLInt2::PositiveNotDefaultOOLInt2(int, int 
arg) :  j(arg) {}
+
 struct PositiveUnaryMinusInt {
   PositiveUnaryMinusInt() : j(-1) {}
   // CHECK-FIXES: PositiveUnaryMinusInt()  {}
@@ -234,12 +266,6 @@
   int i : 5;
 };
 
-struct NegativeNotDefaultInt
-{
-  NegativeNotDefaultInt(int) : i(7) {}
-  int i;
-};
-
 struct NegativeDefaultArg
 {
   NegativeDefaultArg(int i = 4) : i(i) {}
Index: clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
@@ -213,7 +213,6 @@
 
   Finder->addMatcher(
   cxxConstructorDecl(
-  isDefaultConstructor(),
   forEachConstructorInitializer(
   cxxCtorInitializer(
   forField(unless(anyOf(getLangOpts().CPlusPlus20


Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
@@ -45,6 +45,38 @@
   // CHECK-FIXES: int j{1};
 };
 
+struct PositiveNotDefaultInt
+{
+  PositiveNotDefaultInt(int) : i(7) {}
+  // CHECK-FIXES: PositiveNotDefaultInt(int)  {}
+  int i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use default member initializer for 'i'
+  // CHECK-FIXES: int i{7};
+};
+
+struct PositiveNotDefaultOOLInt
+{
+  PositiveNotDefaultOOLInt(int);
+  int i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use default member initializer for 'i'
+  // CHECK-FIXES: int i{7};
+};
+
+PositiveNotDefaultOOLInt::PositiveNotDefaultOOLInt(int) : i(7) {}
+// CHECK-FIXES: PositiveNotDefaultOOLInt::PositiveNotDefaultOOLInt(int)  {}
+
+struct PositiveNotDefaultOOLInt2
+{
+  PositiveNotDefaultOOLInt2(int, int);
+  int i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use default member initializer for 'i'
+  // CHECK-FIXES: int i{7};
+  int j;
+};
+
+PositiveNotDefaultOOLInt2::PositiveNotDefaultOOLInt2(int, int arg) : i(7), j(arg) {}
+// CHECK-FIXES: PositiveNotDefaultOOLInt2::PositiveNotDefaultOOLInt2(int, int arg) :  j(arg) {}
+
 struct PositiveUnaryMinusInt {
   PositiveUnaryMinusInt() : j(-1) {}
   // CHECK-FIXES: PositiveUnaryMinusInt()  {}
@@ -234,12 +266,6 @@
   int i : 5;
 };
 
-struct NegativeNotDefaultInt
-{
-  NegativeNotDefaultInt(int) : i(7) {}
-  int i;
-};
-
 struct NegativeDefaultArg
 {
   NegativeDefaultArg(int i = 4) : i(i) {}
Index: clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
@@ -213,7 +213,6 @@
 
   Finder->addMatcher(
   cxxConstructorDecl(
-  isDefaultConstructor(),
   forEachConstructorInitializer(
   cxxCtorInitializer(
   forField(unless(anyOf(getLangOpts().CPlusPlus20
___
cfe-commits mailing list

[PATCH] D114995: clang-tidy: improve the 'modernize-use-default-member-init'

[Oleg Smolsky via Phabricator via cfe-commits]

oleg.smolsky created this revision.
Herald added a subscriber: carlosgalvezp.
oleg.smolsky requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

- we want to deal with non-default constructors that just happen to contain 
constant initializers

There was already a negative test case, it is now a positive one. We find and 
refactor this case:

  struct PositiveNotDefaultInt {
PositiveNotDefaultInt(int) : i(7) {}
int i;
  };


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114995

Files:
  clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
  test/clang-tidy/checkers/modernize-use-default-member-init.cpp


Index: test/clang-tidy/checkers/modernize-use-default-member-init.cpp
===
--- test/clang-tidy/checkers/modernize-use-default-member-init.cpp
+++ test/clang-tidy/checkers/modernize-use-default-member-init.cpp
@@ -45,6 +45,15 @@
   // CHECK-FIXES: int j{1};
 };
 
+struct PositiveNotDefaultInt
+{
+  PositiveNotDefaultInt(int) : i(7) {}
+  // CHECK-FIXES: PositiveNotDefaultInt(int)  {}
+  int i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use default member initializer 
for 'i'
+  // CHECK-FIXES: int i{7};
+};
+
 struct PositiveUnaryMinusInt {
   PositiveUnaryMinusInt() : j(-1) {}
   // CHECK-FIXES: PositiveUnaryMinusInt()  {}
@@ -234,12 +243,6 @@
   int i : 5;
 };
 
-struct NegativeNotDefaultInt
-{
-  NegativeNotDefaultInt(int) : i(7) {}
-  int i;
-};
-
 struct NegativeDefaultArg
 {
   NegativeDefaultArg(int i = 4) : i(i) {}
Index: clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
@@ -213,7 +213,6 @@
 
   Finder->addMatcher(
   cxxConstructorDecl(
-  isDefaultConstructor(),
   forEachConstructorInitializer(
   cxxCtorInitializer(
   forField(unless(anyOf(getLangOpts().CPlusPlus20


Index: test/clang-tidy/checkers/modernize-use-default-member-init.cpp
===
--- test/clang-tidy/checkers/modernize-use-default-member-init.cpp
+++ test/clang-tidy/checkers/modernize-use-default-member-init.cpp
@@ -45,6 +45,15 @@
   // CHECK-FIXES: int j{1};
 };
 
+struct PositiveNotDefaultInt
+{
+  PositiveNotDefaultInt(int) : i(7) {}
+  // CHECK-FIXES: PositiveNotDefaultInt(int)  {}
+  int i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use default member initializer for 'i'
+  // CHECK-FIXES: int i{7};
+};
+
 struct PositiveUnaryMinusInt {
   PositiveUnaryMinusInt() : j(-1) {}
   // CHECK-FIXES: PositiveUnaryMinusInt()  {}
@@ -234,12 +243,6 @@
   int i : 5;
 };
 
-struct NegativeNotDefaultInt
-{
-  NegativeNotDefaultInt(int) : i(7) {}
-  int i;
-};
-
 struct NegativeDefaultArg
 {
   NegativeDefaultArg(int i = 4) : i(i) {}
Index: clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
@@ -213,7 +213,6 @@
 
   Finder->addMatcher(
   cxxConstructorDecl(
-  isDefaultConstructor(),
   forEachConstructorInitializer(
   cxxCtorInitializer(
   forField(unless(anyOf(getLangOpts().CPlusPlus20
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits