subject:"\[PATCH\] D71199\: \[clang\-tidy\] New check cppcoreguidelines\-prefer\-member\-initializer"

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2021-02-20 Thread Nathan James via Phabricator via cfe-commits

njames93 added inline comments.
Herald added a subscriber: shchenz.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp:184
+bool AddComma = false;
+if (!Ctor->getNumCtorInitializers() && FirstToCtorInits) {
+  SourceLocation BodyPos = Ctor->getBody()->getBeginLoc();

Rechi wrote:
> The following example generates invalid fixes, if 
> modernize-use-default-member-init check isn't enabled, because 
> `Ctor->getNumCtorInitializers()` returns 1.
> 
> ```lang=cpp
> class Example
> {
> public:
>   Example() { a = 0; };
>   int a;
>   std::string string;
> };
> ```
I've done a little work trying to fix some issues I've noticed while running 
this check in the wild. I have a feeling this shortcoming has been addressed in 
there - D97132.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71199

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

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-09-24 Thread Lukas Rechberger via Phabricator via cfe-commits

Rechi added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp:184
+bool AddComma = false;
+if (!Ctor->getNumCtorInitializers() && FirstToCtorInits) {
+  SourceLocation BodyPos = Ctor->getBody()->getBeginLoc();

The following example generates invalid fixes, if 
modernize-use-default-member-init check isn't enabled, because 
`Ctor->getNumCtorInitializers()` returns 1.

```lang=cpp
class Example
{
public:
  Example() { a = 0; };
  int a;
  std::string string;
};
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71199

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

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-09-21 Thread Balogh , Ádám via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4fc0214a1014: [clang-tidy] New check 
cppcoreguidelines-prefer-member-initializer (authored by baloghadamsoftware).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71199

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-modernize-use-default-member-init-assignment.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-modernize-use-default-member-init.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
@@ -0,0 +1,490 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-prefer-member-initializer %t -- -- -fcxx-exceptions
+
+extern void __assert_fail (__const char *__assertion, __const char *__file,
+unsigned int __line, __const char *__function)
+ __attribute__ ((__noreturn__));
+#define assert(expr) \
+  ((expr)  ? (void)(0)  : __assert_fail (#expr, __FILE__, __LINE__, __func__))
+
+class Simple1 {
+  int n;
+  double x;
+
+public:
+  Simple1() {
+// CHECK-FIXES: Simple1() : n(0), x(0.0) {
+n = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+x = 0.0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple1(int nn, double xx) {
+// CHECK-FIXES: Simple1(int nn, double xx) : n(nn), x(xx) {
+n = nn;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+x = xx;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple1() = default;
+};
+
+class Simple2 {
+  int n;
+  double x;
+
+public:
+  Simple2() : n(0) {
+// CHECK-FIXES: Simple2() : n(0), x(0.0) {
+x = 0.0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple2(int nn, double xx) : n(nn) {
+// CHECK-FIXES: Simple2(int nn, double xx) : n(nn), x(xx) {
+x = xx;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple2() = default;
+};
+
+class Simple3 {
+  int n;
+  double x;
+
+public:
+  Simple3() : x(0.0) {
+// CHECK-FIXES: Simple3() : n(0), x(0.0) {
+n = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple3(int nn, double xx) : x(xx) {
+// CHECK-FIXES: Simple3(int nn, double xx) : n(nn), x(xx) {
+n = nn;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple3() = default;
+};
+
+int something_int();
+double something_double();
+
+class Simple4 {
+  int n;
+
+public:
+  Simple4() {
+// CHECK-FIXES: Simple4() : n(something_int()) {
+n = something_int();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple4() = default;
+};
+
+static bool dice();
+
+class Complex1 {
+  int n;
+  int m;
+
+public:
+  Complex1() : n(0) {
+if (dice())
+  m = 1;
+// NO-MESSAGES: initialization of 'm' is nested in a conditional expression
+  }
+
+  ~Complex1() = default;
+};
+
+class Complex2 {
+  int n;
+  int m;
+

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-09-21 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri resigned from this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

In D71199#2285076 , 
@baloghadamsoftware wrote:

> Prerequeisite patch is committed, the check is tested now on the //LLVM 
> Project//. @lebedev.ri, @aaron.ballman can I recommit it?

Thank you! SGTM.


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

https://reviews.llvm.org/D71199

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

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-09-21 Thread Balogh , Ádám via Phabricator via cfe-commits

baloghadamsoftware added a comment.

Prerequeisite patch is committed, the check is tested now on the //LLVM 
Project//. @lebedev.ri, @aaron.ballman can I recommit it?


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

https://reviews.llvm.org/D71199

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

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-09-21 Thread Balogh , Ádám via Phabricator via cfe-commits

baloghadamsoftware updated this revision to Diff 293139.
baloghadamsoftware added a comment.

Updated according to the comments.


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

https://reviews.llvm.org/D71199

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-modernize-use-default-member-init-assignment.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-modernize-use-default-member-init.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
@@ -0,0 +1,490 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-prefer-member-initializer %t -- -- -fcxx-exceptions
+
+extern void __assert_fail (__const char *__assertion, __const char *__file,
+unsigned int __line, __const char *__function)
+ __attribute__ ((__noreturn__));
+#define assert(expr) \
+  ((expr)  ? (void)(0)  : __assert_fail (#expr, __FILE__, __LINE__, __func__))
+
+class Simple1 {
+  int n;
+  double x;
+
+public:
+  Simple1() {
+// CHECK-FIXES: Simple1() : n(0), x(0.0) {
+n = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+x = 0.0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple1(int nn, double xx) {
+// CHECK-FIXES: Simple1(int nn, double xx) : n(nn), x(xx) {
+n = nn;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+x = xx;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple1() = default;
+};
+
+class Simple2 {
+  int n;
+  double x;
+
+public:
+  Simple2() : n(0) {
+// CHECK-FIXES: Simple2() : n(0), x(0.0) {
+x = 0.0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple2(int nn, double xx) : n(nn) {
+// CHECK-FIXES: Simple2(int nn, double xx) : n(nn), x(xx) {
+x = xx;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple2() = default;
+};
+
+class Simple3 {
+  int n;
+  double x;
+
+public:
+  Simple3() : x(0.0) {
+// CHECK-FIXES: Simple3() : n(0), x(0.0) {
+n = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple3(int nn, double xx) : x(xx) {
+// CHECK-FIXES: Simple3(int nn, double xx) : n(nn), x(xx) {
+n = nn;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple3() = default;
+};
+
+int something_int();
+double something_double();
+
+class Simple4 {
+  int n;
+
+public:
+  Simple4() {
+// CHECK-FIXES: Simple4() : n(something_int()) {
+n = something_int();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple4() = default;
+};
+
+static bool dice();
+
+class Complex1 {
+  int n;
+  int m;
+
+public:
+  Complex1() : n(0) {
+if (dice())
+  m = 1;
+// NO-MESSAGES: initialization of 'm' is nested in a conditional expression
+  }
+
+  ~Complex1() = default;
+};
+
+class Complex2 {
+  int n;
+  int m;
+
+public:
+  Complex2() : n(0) {
+if (!dice())
+  return;
+m = 1;
+// NO-MESSAGES: initialization of 'm'

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-09-14 Thread Balogh , Ádám via Phabricator via cfe-commits

baloghadamsoftware updated this revision to Diff 291562.
baloghadamsoftware added a comment.

No crash, no hang, no false positives on the LLVM Project.


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

https://reviews.llvm.org/D71199

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-modernize-use-default-member-init-assignment.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-modernize-use-default-member-init.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
@@ -0,0 +1,474 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-prefer-member-initializer %t -- -- -fcxx-exceptions
+
+extern void __assert_fail (__const char *__assertion, __const char *__file,
+unsigned int __line, __const char *__function)
+ __attribute__ ((__noreturn__));
+#define assert(expr) \
+  ((expr)  ? (void)(0)  : __assert_fail (#expr, __FILE__, __LINE__, __func__))
+
+class Simple1 {
+  int n;
+  double x;
+
+public:
+  Simple1() {
+// CHECK-FIXES: Simple1() : n(0), x(0.0) {
+n = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+x = 0.0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple1(int nn, double xx) {
+// CHECK-FIXES: Simple1(int nn, double xx) : n(nn), x(xx) {
+n = nn;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+x = xx;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple1() = default;
+};
+
+class Simple2 {
+  int n;
+  double x;
+
+public:
+  Simple2() : n(0) {
+// CHECK-FIXES: Simple2() : n(0), x(0.0) {
+x = 0.0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple2(int nn, double xx) : n(nn) {
+// CHECK-FIXES: Simple2(int nn, double xx) : n(nn), x(xx) {
+x = xx;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple2() = default;
+};
+
+class Simple3 {
+  int n;
+  double x;
+
+public:
+  Simple3() : x(0.0) {
+// CHECK-FIXES: Simple3() : n(0), x(0.0) {
+n = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple3(int nn, double xx) : x(xx) {
+// CHECK-FIXES: Simple3(int nn, double xx) : n(nn), x(xx) {
+n = nn;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple3() = default;
+};
+
+int something_int();
+double something_double();
+
+class Simple4 {
+  int n;
+
+public:
+  Simple4() {
+// CHECK-FIXES: Simple4() : n(something_int()) {
+n = something_int();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple4() = default;
+};
+
+static bool dice();
+
+class Complex1 {
+  int n;
+  int m;
+
+public:
+  Complex1() : n(0) {
+if (dice())
+  m = 1;
+// NO-MESSAGES: initialization of 'm' is nested in a conditional expression
+  }
+
+  ~Complex1() = default;
+};
+
+class Complex2 {
+  int n;
+  int m;
+
+public:
+  Complex2() : n(0) {
+if (!dice())
+  return;
+m = 1;
+// NO-MESSAGES:

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-09-14 Thread Whisperity via Phabricator via cfe-commits

whisperity added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp:43
+static bool isLiteral(const Expr *E) {
+  return isa(E) ||
+ isa(E) ||

baloghadamsoftware wrote:
> aaron.ballman wrote:
> > What about other kinds of literals like user-defined literals, or literal 
> > class types? Should this be using `E->getType()->isLiteralType()`?
> `E->getType()->isLiteralType()` does not work since it tells about the type, 
> not the expression itself. Here we should return `true` for literals only, 
> not for any expressions whose type is a literal type. Thus `int` is a literal 
> type, `5` is an `int` literal, but `n` or `return_any_int()` not.
@baloghadamsoftware The original comment by Aaron is left unmarked as "done" 
here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71199

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

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-09-14 Thread Whisperity via Phabricator via cfe-commits

whisperity added a comment.

Minor inline comments.

Could you please add a test case where the constructor is defined out of line? 
So far in every test, the constructor //body// was inside the class. Something 
like:

  struct S {
int i, j;
S();
  };
  
  S::S() {
i = 1;
j = 2;
  }

I'm also particularly interested in the case when the constructor is 
implemented in its "own" translation unit. Where will the initialiser be moved 
in that case? Into the header, where the record is defined, still?




Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp:21-23
+  return isa(S) || isa(S) || isa(S) ||
+ isa(S) || isa(S) || isa(S) ||
+ isa(S) || isa(S) || isa(S);

It was you who figured out in D85728 that `isa` has changed recently and is now 
variadic. Then this function can be simplified with the variadic version.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp:39-41
+  return isa(E) || isa(E) ||
+ isa(E) || isa(E) ||
+ isa(E) || isa(E);

Ditto.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp:126
+
+  for (const auto *S : Body->body()) {
+if (isControlStatement(S))





Comment at: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst:6
+
+Finds member initializations in the constructor body which can be  converted
+into member initializers of the constructor instead. This not only improves




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71199

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

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-09-11 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added a comment.

Thank you for looking into it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71199

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

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-09-11 Thread Balogh , Ádám via Phabricator via cfe-commits

baloghadamsoftware added a comment.

It looks like it was not entirely my fault: D87527 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71199

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

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-09-10 Thread Balogh , Ádám via Phabricator via cfe-commits

baloghadamsoftware added a comment.

OK, I reversed the matcher expression now, it seems to work, but I will check 
it on the //LLVM/Clang// codebase first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71199

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

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-09-10 Thread Balogh , Ádám via Phabricator via cfe-commits

baloghadamsoftware added a comment.

I found the problem: `hasBody()` always returns true if the function has 
//somewhere// a body. This means that also the `hasBody` matcher matches 
forward declarations. However, I only need the definition. This far I could not 
find any method to achieve that: `isDefinition()` also returns true for forward 
declarations, as well as comparing with the `CanonicalDecl`. I am looking 
further...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71199

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

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-09-10 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D71199#2265693 , @lebedev.ri wrote:

> In D71199#2265692 , 
> @baloghadamsoftware wrote:
>
>> In D71199#2265594 , @lebedev.ri 
>> wrote:
>>
>>> So i've just reverted this in rGebf496d805521b53022a351f35854de977fee844 
>>> .
>>>
>>> @aaron.ballman @baloghadamsoftware how's QC going on nowadays here?
>>> Was this evaluated on anything other than it's tests?
>>
>> Surely. After I commit a patch, lots of buildbots verify it. They passed so 
>> far.
>
> @baloghadamsoftware, i think you understand that wasn't the question.

This feedback is a bit terse and not very constructive. FWIW, we don't 
typically ask patch authors to run their patch over a large corpus of code 
unless a reviewer expects there to be a performance concern and asks 
explicitly. Given that this checks constructor bodies, there was no obvious 
reason to ask for that here. Also, I can't recall a time when we expected a 
patch reviewer to do that work. I appreciate that you noticed an issue and 
reverted so we could investigate, but when reporting an issue like this, please 
try to keep in mind that we're all in the same community trying to make a great 
product.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71199

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

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-09-10 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added a comment.

In D71199#2265692 , 
@baloghadamsoftware wrote:

> In D71199#2265594 , @lebedev.ri 
> wrote:
>
>> So i've just reverted this in rGebf496d805521b53022a351f35854de977fee844 
>> .
>>
>> @aaron.ballman @baloghadamsoftware how's QC going on nowadays here?
>> Was this evaluated on anything other than it's tests?
>
> Surely. After I commit a patch, lots of buildbots verify it. They passed so 
> far.

@baloghadamsoftware, i think you understand that wasn't the question.

>> It appears to be either unacceptably slow, or subtly broken, because it 
>> takes at least 100x time more than all of the other clang-tidy checks 
>> enabled, and e.g.
>> `clang-tidy-12 --checks="-*,cppcoreguidelines-prefer-member-initializer" -p 
>> . 
>> /repositories/llvm-project/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp`
>> never finishes (minutes and counting).
>
> I see nothing there that could be slow, thus this is probably some hang. I 
> will investigate it.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71199

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

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-09-10 Thread Balogh , Ádám via Phabricator via cfe-commits

baloghadamsoftware added a comment.

In D71199#2265594 , @lebedev.ri wrote:

> So i've just reverted this in rGebf496d805521b53022a351f35854de977fee844 
> .
>
> @aaron.ballman @baloghadamsoftware how's QC going on nowadays here?
> Was this evaluated on anything other than it's tests?

Surely. After I commit a patch, lots of buildbots verify it. They passed so far.

> It appears to be either unacceptably slow, or subtly broken, because it takes 
> at least 100x time more than all of the other clang-tidy checks enabled, and 
> e.g.
> `clang-tidy-12 --checks="-*,cppcoreguidelines-prefer-member-initializer" -p . 
> /repositories/llvm-project/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp`
> never finishes (minutes and counting).

I see nothing there that could be slow, thus this is probably some hang. I will 
investigate it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71199

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

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-09-10 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri reopened this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

So i've just reverted this.

@aaron.ballman @baloghadamsoftware how's QC going on nowadays here?
Was this evaluated on anything other than it's tests?

It appears to be either unacceptably slow, or subtly broken, because it takes 
at least 100x time more than all of the other clang-tidy checks enabled, and 
e.g.
`clang-tidy-12 --checks="-*,cppcoreguidelines-prefer-member-initializer" -p . 
/repositories/llvm-project/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp`
never finishes (minutes and counting).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71199

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

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-08-31 Thread Balogh , Ádám via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf5fd7486d6c0: [clang-tidy] New check 
readability-prefer-member-initializer (authored by baloghadamsoftware).

Changed prior to commit:
  https://reviews.llvm.org/D71199?vs=285657=288939#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71199

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-modernize-use-default-member-init-assignment.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-modernize-use-default-member-init.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
@@ -0,0 +1,454 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-prefer-member-initializer %t
+
+class Simple1 {
+  int n;
+  double x;
+
+public:
+  Simple1() {
+// CHECK-FIXES: Simple1() : n(0), x(0.0) {
+n = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+x = 0.0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple1(int nn, double xx) {
+// CHECK-FIXES: Simple1(int nn, double xx) : n(nn), x(xx) {
+n = nn;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+x = xx;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple1() = default;
+};
+
+class Simple2 {
+  int n;
+  double x;
+
+public:
+  Simple2() : n(0) {
+// CHECK-FIXES: Simple2() : n(0), x(0.0) {
+x = 0.0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple2(int nn, double xx) : n(nn) {
+// CHECK-FIXES: Simple2(int nn, double xx) : n(nn), x(xx) {
+x = xx;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple2() = default;
+};
+
+class Simple3 {
+  int n;
+  double x;
+
+public:
+  Simple3() : x(0.0) {
+// CHECK-FIXES: Simple3() : n(0), x(0.0) {
+n = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple3(int nn, double xx) : x(xx) {
+// CHECK-FIXES: Simple3(int nn, double xx) : n(nn), x(xx) {
+n = nn;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple3() = default;
+};
+
+int something_int();
+double something_double();
+
+class Simple4 {
+  int n;
+
+public:
+  Simple4() {
+// CHECK-FIXES: Simple4() : n(something_int()) {
+n = something_int();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple4() = default;
+};
+
+static bool dice();
+
+class Complex1 {
+  int n;
+  int m;
+
+public:
+  Complex1() : n(0) {
+if (dice())
+  m = 1;
+// NO-MESSAGES: initialization of 'm' is nested in a conditional expression
+  }
+
+  ~Complex1() = default;
+};
+
+class Complex2 {
+  int n;
+  int m;
+
+public:
+  Complex2() : n(0) {
+if (!dice())
+  return;
+m = 1;
+// NO-MESSAGES: initialization of 'm' follows a conditional expression
+  }
+
+

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-08-14 Thread Balogh , Ádám via Phabricator via cfe-commits

baloghadamsoftware updated this revision to Diff 285657.
baloghadamsoftware added a comment.

Updated according to a comment.


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

https://reviews.llvm.org/D71199

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-modernize-use-default-member-init-assignment.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-modernize-use-default-member-init.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
@@ -0,0 +1,454 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-prefer-member-initializer %t
+
+class Simple1 {
+  int n;
+  double x;
+
+public:
+  Simple1() {
+// CHECK-FIXES: Simple1() : n(0), x(0.0) {
+n = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+x = 0.0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple1(int nn, double xx) {
+// CHECK-FIXES: Simple1(int nn, double xx) : n(nn), x(xx) {
+n = nn;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+x = xx;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple1() = default;
+};
+
+class Simple2 {
+  int n;
+  double x;
+
+public:
+  Simple2() : n(0) {
+// CHECK-FIXES: Simple2() : n(0), x(0.0) {
+x = 0.0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple2(int nn, double xx) : n(nn) {
+// CHECK-FIXES: Simple2(int nn, double xx) : n(nn), x(xx) {
+x = xx;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple2() = default;
+};
+
+class Simple3 {
+  int n;
+  double x;
+
+public:
+  Simple3() : x(0.0) {
+// CHECK-FIXES: Simple3() : n(0), x(0.0) {
+n = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple3(int nn, double xx) : x(xx) {
+// CHECK-FIXES: Simple3(int nn, double xx) : n(nn), x(xx) {
+n = nn;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple3() = default;
+};
+
+int something_int();
+double something_double();
+
+class Simple4 {
+  int n;
+
+public:
+  Simple4() {
+// CHECK-FIXES: Simple4() : n(something_int()) {
+n = something_int();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple4() = default;
+};
+
+static bool dice();
+
+class Complex1 {
+  int n;
+  int m;
+
+public:
+  Complex1() : n(0) {
+if (dice())
+  m = 1;
+// NO-MESSAGES: initialization of 'm' is nested in a conditional expression
+  }
+
+  ~Complex1() = default;
+};
+
+class Complex2 {
+  int n;
+  int m;
+
+public:
+  Complex2() : n(0) {
+if (!dice())
+  return;
+m = 1;
+// NO-MESSAGES: initialization of 'm' follows a conditional expression
+  }
+
+  ~Complex2() = default;
+};
+
+class Complex3 {
+  int n;
+  int m;
+
+public:
+  Complex3() : n(0) {
+while (dice())
+  m = 1;
+// NO-MESSAGES: initialization of 'm' is nested in a conditional

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-08-13 Thread Nathan James via Phabricator via cfe-commits

njames93 added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp:103
+Context->getOptions().CheckOptions)
+.get("UseAssignment", 0) != 0) {}
+

`OptionsView::get(...)` is specialized for bools, so you can just pass `false` 
as the second argument and negate the need to compare against zero


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

https://reviews.llvm.org/D71199

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

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-08-13 Thread 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.

In D71199#2215719 , 
@baloghadamsoftware wrote:

> Since no better idea cam to my mind, in this version I check whether 
> `modernize-use-default-member-init` is enabled. If it is, then we issue a 
> warning and suggest a fix that uses default member initializer. We also take 
> into account the option for that checker whether we should use brackets or 
> assignment. This checker has no options now. If the other checker is not 
> enabled, we always warn and suggest fix to use constructor member initializer.

While I don't feel awesome about this approach, I don't have a better idea 
either. Unless @alexfh, @gribozavr2, or one of the other reviewers has 
concerns, this LGTM. Thank you for your patience while we thought our way 
through this!

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

https://reviews.llvm.org/D71199

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

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-08-13 Thread Balogh , Ádám via Phabricator via cfe-commits

baloghadamsoftware updated this revision to Diff 285366.
baloghadamsoftware added a comment.

Since no better idea cam to my mind, in this version I check whether 
`modernize-use-default-member-init` is enabled. If it is, then we issue a 
warning and suggest a fix that uses default member initializer. We also take 
into account the option for that checker whether we should use brackets or 
assignment. This checker has no options now. If the other checker is not 
enabled, we always warn and suggest fix to use constructor member initializer.


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

https://reviews.llvm.org/D71199

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
@@ -0,0 +1,454 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-prefer-member-initializer %t
+
+class Simple1 {
+  int n;
+  double x;
+
+public:
+  Simple1() {
+// CHECK-FIXES: Simple1() : n(0), x(0.0) {
+n = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+x = 0.0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple1(int nn, double xx) {
+// CHECK-FIXES: Simple1(int nn, double xx) : n(nn), x(xx) {
+n = nn;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+x = xx;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple1() = default;
+};
+
+class Simple2 {
+  int n;
+  double x;
+
+public:
+  Simple2() : n(0) {
+// CHECK-FIXES: Simple2() : n(0), x(0.0) {
+x = 0.0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple2(int nn, double xx) : n(nn) {
+// CHECK-FIXES: Simple2(int nn, double xx) : n(nn), x(xx) {
+x = xx;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple2() = default;
+};
+
+class Simple3 {
+  int n;
+  double x;
+
+public:
+  Simple3() : x(0.0) {
+// CHECK-FIXES: Simple3() : n(0), x(0.0) {
+n = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple3(int nn, double xx) : x(xx) {
+// CHECK-FIXES: Simple3(int nn, double xx) : n(nn), x(xx) {
+n = nn;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple3() = default;
+};
+
+int something_int();
+double something_double();
+
+class Simple4 {
+  int n;
+
+public:
+  Simple4() {
+// CHECK-FIXES: Simple4() : n(something_int()) {
+n = something_int();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple4() = default;
+};
+
+static bool dice();
+
+class Complex1 {
+  int n;
+  int m;
+
+public:
+  Complex1() : n(0) {
+if (dice())
+  m = 1;
+// NO-MESSAGES: initialization of 'm' is nested in a conditional expression
+  }
+
+  ~Complex1() = default;
+};
+
+class Complex2 {
+  int n;
+  int m;
+
+public:
+  Complex2() : n(0) {
+if (!dice())
+  return;
+m = 1;
+// NO-MESSAGES: initialization of 'm' follows a conditional expression
+  }
+
+  ~Complex2() = default;
+};
+
+class Complex3 {
+

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-07-02 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-assignment.cpp:29
+public:
+  Simple2() : n (0) {
+x = 0.0;

baloghadamsoftware wrote:
> aaron.ballman wrote:
> > baloghadamsoftware wrote:
> > > aaron.ballman wrote:
> > > > baloghadamsoftware wrote:
> > > > > aaron.ballman wrote:
> > > > > > By my reading of the core guideline 
> > > > > > (https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c48-prefer-in-class-initializers-to-member-initializers-in-constructors-for-constant-initializers),
> > > > > >  it looks like `n` should also be diagnosed because all of the 
> > > > > > constructors in the class initialize the member to the same 
> > > > > > constant value. Is there a reason to deviate from the rule (or have 
> > > > > > I missed something)?
> > > > > > 
> > > > > > Also, I'd like to see a test case like:
> > > > > > ```
> > > > > > class C {
> > > > > >   int n;
> > > > > > public:
> > > > > >   C() { n = 0; }
> > > > > >   explicit C(int) { n = 12; }
> > > > > > };
> > > > > > ```
> > > > > This check only cares for initializations inside the body (rule 
> > > > > `C.49`, but if the proper fix is to convert them to default member 
> > > > > initializer according to rule `C.48` then we follow that rule in the 
> > > > > fix). For initializations implemented as constructor member 
> > > > > initializers but according to `C.48` they should have been 
> > > > > implemented as default member initializers we already have check 
> > > > > `modernize-use-default-member-init`.
> > > > Thank you for the explanation. I have the feeling that these two rules 
> > > > are going to have some really weird interactions together. For 
> > > > instance, the example you added at my request shows behavior I can't 
> > > > easily explain as a user:
> > > > ```
> > > > class Complex19 {
> > > >   int n;
> > > >   // CHECK-FIXES: int n{0};
> > > > public:
> > > >   Complex19() {
> > > > n = 0;
> > > > // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be 
> > > > initialized in an in-class default member initializer 
> > > > [cppcoreguidelines-prefer-member-initializer]
> > > > // CHECK-FIXES: {{^\ *$}}
> > > >   }
> > > > 
> > > >   explicit Complex19(int) {
> > > > // CHECK-FIXES: Complex19(int) : n(12) {
> > > > n = 12;
> > > > // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be 
> > > > initialized in a member initializer of the constructor 
> > > > [cppcoreguidelines-prefer-member-initializer]
> > > > // CHECK-FIXES: {{^\ *$}}
> > > >   }
> > > > 
> > > >   ~Complex19() = default;
> > > > };
> > > > ```
> > > > Despite both constructors having the assignment expression `n = 
> > > > ;` one gets one diagnostic + fixit and the other gets a 
> > > > different diagnostic + fixit.
> > > > 
> > > > Also, from reading C.49, I don't see anything about using in-class 
> > > > initialization. I see C.49 as being about avoiding the situation where 
> > > > the ctor runs a constructor for a data member only to then immediately 
> > > > in the ctor body make an assignment to that member. You don't need to 
> > > > initialize then reinitialize when you can use a member init list to 
> > > > construct the object properly initially.
> > > Thank you for your comment. While I can agree with you, I think that this 
> > > check should be fully consistent with 
> > > `modernize-use-default-member-init`. Thus I tried it on the following 
> > > example:
> > > ```
> > > class C {
> > >   int n;
> > > public:
> > >   C() : n(1) {}
> > >   C(int nn) : n(nn) {}
> > > 
> > >   int get_n() { return n; }
> > > };
> > > ```
> > > I got the following message from Clang-Tidy:
> > > ```
> > > warning: use default member initializer for 'n' 
> > > [modernize-use-default-member-init]
> > >   int n;
> > >   ^
> > >{1}
> > > ```
> > > This is no surprise, however. If you take a look on the example in [[ 
> > > https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c48-prefer-in-class-initializers-to-member-initializers-in-constructors-for-constant-initializers
> > >  | C.48: Prefer in-class initializers to member initializers in 
> > > constructors for constant initializers ]] you can see that our code is 
> > > almost the same as the example considered bad there. So rule C.49 does 
> > > not speak anything about in-class initialization, but C.48 does, our 
> > > check enforcing C.49 must not suggest a fixit the the check enforcing 
> > > C.48 fixes again. We should not force the user to run Clang-Tidy in 
> > > multiple passes. Maybe we can mention the numbers of the rules in the 
> > > error messages to make them clearer.
> > > This is no surprise, however.
> > 
> > Agreed, but it's also a different example from the one I posted where the 
> > behavior is mysterious.
> > 
> > > We should not force the user to run Clang-Tidy in

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-07-02 Thread Balogh , Ádám via Phabricator via cfe-commits

baloghadamsoftware marked an inline comment as done.
baloghadamsoftware added a comment.

In D71199#2125602 , @njames93 wrote:

> Just my 2 cents, but would it not be wise to introduce a test case that runs 
> the 2 aforementioned checks together demonstrating how they interact with 
> each other. This will also force compliance if either check is updated down 
> the line.


I can do that, and I will do it. However, all checks work on the original 
files, thus no interaction is expected.




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-assignment.cpp:29
+public:
+  Simple2() : n (0) {
+x = 0.0;

aaron.ballman wrote:
> baloghadamsoftware wrote:
> > aaron.ballman wrote:
> > > baloghadamsoftware wrote:
> > > > aaron.ballman wrote:
> > > > > By my reading of the core guideline 
> > > > > (https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c48-prefer-in-class-initializers-to-member-initializers-in-constructors-for-constant-initializers),
> > > > >  it looks like `n` should also be diagnosed because all of the 
> > > > > constructors in the class initialize the member to the same constant 
> > > > > value. Is there a reason to deviate from the rule (or have I missed 
> > > > > something)?
> > > > > 
> > > > > Also, I'd like to see a test case like:
> > > > > ```
> > > > > class C {
> > > > >   int n;
> > > > > public:
> > > > >   C() { n = 0; }
> > > > >   explicit C(int) { n = 12; }
> > > > > };
> > > > > ```
> > > > This check only cares for initializations inside the body (rule `C.49`, 
> > > > but if the proper fix is to convert them to default member initializer 
> > > > according to rule `C.48` then we follow that rule in the fix). For 
> > > > initializations implemented as constructor member initializers but 
> > > > according to `C.48` they should have been implemented as default member 
> > > > initializers we already have check `modernize-use-default-member-init`.
> > > Thank you for the explanation. I have the feeling that these two rules 
> > > are going to have some really weird interactions together. For instance, 
> > > the example you added at my request shows behavior I can't easily explain 
> > > as a user:
> > > ```
> > > class Complex19 {
> > >   int n;
> > >   // CHECK-FIXES: int n{0};
> > > public:
> > >   Complex19() {
> > > n = 0;
> > > // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized 
> > > in an in-class default member initializer 
> > > [cppcoreguidelines-prefer-member-initializer]
> > > // CHECK-FIXES: {{^\ *$}}
> > >   }
> > > 
> > >   explicit Complex19(int) {
> > > // CHECK-FIXES: Complex19(int) : n(12) {
> > > n = 12;
> > > // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized 
> > > in a member initializer of the constructor 
> > > [cppcoreguidelines-prefer-member-initializer]
> > > // CHECK-FIXES: {{^\ *$}}
> > >   }
> > > 
> > >   ~Complex19() = default;
> > > };
> > > ```
> > > Despite both constructors having the assignment expression `n = 
> > > ;` one gets one diagnostic + fixit and the other gets a 
> > > different diagnostic + fixit.
> > > 
> > > Also, from reading C.49, I don't see anything about using in-class 
> > > initialization. I see C.49 as being about avoiding the situation where 
> > > the ctor runs a constructor for a data member only to then immediately in 
> > > the ctor body make an assignment to that member. You don't need to 
> > > initialize then reinitialize when you can use a member init list to 
> > > construct the object properly initially.
> > Thank you for your comment. While I can agree with you, I think that this 
> > check should be fully consistent with `modernize-use-default-member-init`. 
> > Thus I tried it on the following example:
> > ```
> > class C {
> >   int n;
> > public:
> >   C() : n(1) {}
> >   C(int nn) : n(nn) {}
> > 
> >   int get_n() { return n; }
> > };
> > ```
> > I got the following message from Clang-Tidy:
> > ```
> > warning: use default member initializer for 'n' 
> > [modernize-use-default-member-init]
> >   int n;
> >   ^
> >{1}
> > ```
> > This is no surprise, however. If you take a look on the example in [[ 
> > https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c48-prefer-in-class-initializers-to-member-initializers-in-constructors-for-constant-initializers
> >  | C.48: Prefer in-class initializers to member initializers in 
> > constructors for constant initializers ]] you can see that our code is 
> > almost the same as the example considered bad there. So rule C.49 does not 
> > speak anything about in-class initialization, but C.48 does, our check 
> > enforcing C.49 must not suggest a fixit the the check enforcing C.48 fixes 
> > again. We should not force the user to run Clang-Tidy in multiple passes. 
> > Maybe we can mention the numbers of the rules in

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-07-01 Thread Nathan James via Phabricator via cfe-commits

njames93 added a comment.

Just my 2 cents, but would it not be wise to introduce a test case that runs 
the 2 aforementioned checks together demonstrating how they interact with each 
other. This will also force compliance if either check is updated down the line.


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

https://reviews.llvm.org/D71199



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

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-07-01 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-assignment.cpp:29
+public:
+  Simple2() : n (0) {
+x = 0.0;

baloghadamsoftware wrote:
> aaron.ballman wrote:
> > baloghadamsoftware wrote:
> > > aaron.ballman wrote:
> > > > By my reading of the core guideline 
> > > > (https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c48-prefer-in-class-initializers-to-member-initializers-in-constructors-for-constant-initializers),
> > > >  it looks like `n` should also be diagnosed because all of the 
> > > > constructors in the class initialize the member to the same constant 
> > > > value. Is there a reason to deviate from the rule (or have I missed 
> > > > something)?
> > > > 
> > > > Also, I'd like to see a test case like:
> > > > ```
> > > > class C {
> > > >   int n;
> > > > public:
> > > >   C() { n = 0; }
> > > >   explicit C(int) { n = 12; }
> > > > };
> > > > ```
> > > This check only cares for initializations inside the body (rule `C.49`, 
> > > but if the proper fix is to convert them to default member initializer 
> > > according to rule `C.48` then we follow that rule in the fix). For 
> > > initializations implemented as constructor member initializers but 
> > > according to `C.48` they should have been implemented as default member 
> > > initializers we already have check `modernize-use-default-member-init`.
> > Thank you for the explanation. I have the feeling that these two rules are 
> > going to have some really weird interactions together. For instance, the 
> > example you added at my request shows behavior I can't easily explain as a 
> > user:
> > ```
> > class Complex19 {
> >   int n;
> >   // CHECK-FIXES: int n{0};
> > public:
> >   Complex19() {
> > n = 0;
> > // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized 
> > in an in-class default member initializer 
> > [cppcoreguidelines-prefer-member-initializer]
> > // CHECK-FIXES: {{^\ *$}}
> >   }
> > 
> >   explicit Complex19(int) {
> > // CHECK-FIXES: Complex19(int) : n(12) {
> > n = 12;
> > // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized 
> > in a member initializer of the constructor 
> > [cppcoreguidelines-prefer-member-initializer]
> > // CHECK-FIXES: {{^\ *$}}
> >   }
> > 
> >   ~Complex19() = default;
> > };
> > ```
> > Despite both constructors having the assignment expression `n = ;` 
> > one gets one diagnostic + fixit and the other gets a different diagnostic + 
> > fixit.
> > 
> > Also, from reading C.49, I don't see anything about using in-class 
> > initialization. I see C.49 as being about avoiding the situation where the 
> > ctor runs a constructor for a data member only to then immediately in the 
> > ctor body make an assignment to that member. You don't need to initialize 
> > then reinitialize when you can use a member init list to construct the 
> > object properly initially.
> Thank you for your comment. While I can agree with you, I think that this 
> check should be fully consistent with `modernize-use-default-member-init`. 
> Thus I tried it on the following example:
> ```
> class C {
>   int n;
> public:
>   C() : n(1) {}
>   C(int nn) : n(nn) {}
> 
>   int get_n() { return n; }
> };
> ```
> I got the following message from Clang-Tidy:
> ```
> warning: use default member initializer for 'n' 
> [modernize-use-default-member-init]
>   int n;
>   ^
>{1}
> ```
> This is no surprise, however. If you take a look on the example in [[ 
> https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c48-prefer-in-class-initializers-to-member-initializers-in-constructors-for-constant-initializers
>  | C.48: Prefer in-class initializers to member initializers in constructors 
> for constant initializers ]] you can see that our code is almost the same as 
> the example considered bad there. So rule C.49 does not speak anything about 
> in-class initialization, but C.48 does, our check enforcing C.49 must not 
> suggest a fixit the the check enforcing C.48 fixes again. We should not force 
> the user to run Clang-Tidy in multiple passes. Maybe we can mention the 
> numbers of the rules in the error messages to make them clearer.
> This is no surprise, however.

Agreed, but it's also a different example from the one I posted where the 
behavior is mysterious.

> We should not force the user to run Clang-Tidy in multiple passes.

Agreed, but the checks are also independent of one another (e.g., I can run one 
but not the other) and that use case needs to be considered as well as the 
combined use case.

I personally don't use the C++ Core Guidelines and so I'm not as well-versed in 
their nuances -- I'm happy to let someone more familiar with the standard sway 
my opinion, but this smells a bit like the two rules are less orthogonal than 
the rule authors may believe. Both rules say to

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-06-24 Thread Balogh , Ádám via Phabricator via cfe-commits

baloghadamsoftware marked 2 inline comments as done.
baloghadamsoftware added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-assignment.cpp:29
+public:
+  Simple2() : n (0) {
+x = 0.0;

aaron.ballman wrote:
> baloghadamsoftware wrote:
> > aaron.ballman wrote:
> > > By my reading of the core guideline 
> > > (https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c48-prefer-in-class-initializers-to-member-initializers-in-constructors-for-constant-initializers),
> > >  it looks like `n` should also be diagnosed because all of the 
> > > constructors in the class initialize the member to the same constant 
> > > value. Is there a reason to deviate from the rule (or have I missed 
> > > something)?
> > > 
> > > Also, I'd like to see a test case like:
> > > ```
> > > class C {
> > >   int n;
> > > public:
> > >   C() { n = 0; }
> > >   explicit C(int) { n = 12; }
> > > };
> > > ```
> > This check only cares for initializations inside the body (rule `C.49`, but 
> > if the proper fix is to convert them to default member initializer 
> > according to rule `C.48` then we follow that rule in the fix). For 
> > initializations implemented as constructor member initializers but 
> > according to `C.48` they should have been implemented as default member 
> > initializers we already have check `modernize-use-default-member-init`.
> Thank you for the explanation. I have the feeling that these two rules are 
> going to have some really weird interactions together. For instance, the 
> example you added at my request shows behavior I can't easily explain as a 
> user:
> ```
> class Complex19 {
>   int n;
>   // CHECK-FIXES: int n{0};
> public:
>   Complex19() {
> n = 0;
> // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in 
> an in-class default member initializer 
> [cppcoreguidelines-prefer-member-initializer]
> // CHECK-FIXES: {{^\ *$}}
>   }
> 
>   explicit Complex19(int) {
> // CHECK-FIXES: Complex19(int) : n(12) {
> n = 12;
> // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in 
> a member initializer of the constructor 
> [cppcoreguidelines-prefer-member-initializer]
> // CHECK-FIXES: {{^\ *$}}
>   }
> 
>   ~Complex19() = default;
> };
> ```
> Despite both constructors having the assignment expression `n = ;` 
> one gets one diagnostic + fixit and the other gets a different diagnostic + 
> fixit.
> 
> Also, from reading C.49, I don't see anything about using in-class 
> initialization. I see C.49 as being about avoiding the situation where the 
> ctor runs a constructor for a data member only to then immediately in the 
> ctor body make an assignment to that member. You don't need to initialize 
> then reinitialize when you can use a member init list to construct the object 
> properly initially.
Thank you for your comment. While I can agree with you, I think that this check 
should be fully consistent with `modernize-use-default-member-init`. Thus I 
tried it on the following example:
```
class C {
  int n;
public:
  C() : n(1) {}
  C(int nn) : n(nn) {}

  int get_n() { return n; }
};
```
I got the following message from Clang-Tidy:
```
warning: use default member initializer for 'n' 
[modernize-use-default-member-init]
  int n;
  ^
   {1}
```
This is no surprise, however. If you take a look on the example in [[ 
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c48-prefer-in-class-initializers-to-member-initializers-in-constructors-for-constant-initializers
 | C.48: Prefer in-class initializers to member initializers in constructors 
for constant initializers ]] you can see that our code is almost the same as 
the example considered bad there. So rule C.49 does not speak anything about 
in-class initialization, but C.48 does, our check enforcing C.49 must not 
suggest a fixit the the check enforcing C.48 fixes again. We should not force 
the user to run Clang-Tidy in multiple passes. Maybe we can mention the numbers 
of the rules in the error messages to make them clearer.


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

https://reviews.llvm.org/D71199



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

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-06-16 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-assignment.cpp:29
+public:
+  Simple2() : n (0) {
+x = 0.0;

baloghadamsoftware wrote:
> aaron.ballman wrote:
> > By my reading of the core guideline 
> > (https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c48-prefer-in-class-initializers-to-member-initializers-in-constructors-for-constant-initializers),
> >  it looks like `n` should also be diagnosed because all of the constructors 
> > in the class initialize the member to the same constant value. Is there a 
> > reason to deviate from the rule (or have I missed something)?
> > 
> > Also, I'd like to see a test case like:
> > ```
> > class C {
> >   int n;
> > public:
> >   C() { n = 0; }
> >   explicit C(int) { n = 12; }
> > };
> > ```
> This check only cares for initializations inside the body (rule `C.49`, but 
> if the proper fix is to convert them to default member initializer according 
> to rule `C.48` then we follow that rule in the fix). For initializations 
> implemented as constructor member initializers but according to `C.48` they 
> should have been implemented as default member initializers we already have 
> check `modernize-use-default-member-init`.
Thank you for the explanation. I have the feeling that these two rules are 
going to have some really weird interactions together. For instance, the 
example you added at my request shows behavior I can't easily explain as a user:
```
class Complex19 {
  int n;
  // CHECK-FIXES: int n{0};
public:
  Complex19() {
n = 0;
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in an 
in-class default member initializer 
[cppcoreguidelines-prefer-member-initializer]
// CHECK-FIXES: {{^\ *$}}
  }

  explicit Complex19(int) {
// CHECK-FIXES: Complex19(int) : n(12) {
n = 12;
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a 
member initializer of the constructor 
[cppcoreguidelines-prefer-member-initializer]
// CHECK-FIXES: {{^\ *$}}
  }

  ~Complex19() = default;
};
```
Despite both constructors having the assignment expression `n = ;` one 
gets one diagnostic + fixit and the other gets a different diagnostic + fixit.

Also, from reading C.49, I don't see anything about using in-class 
initialization. I see C.49 as being about avoiding the situation where the ctor 
runs a constructor for a data member only to then immediately in the ctor body 
make an assignment to that member. You don't need to initialize then 
reinitialize when you can use a member init list to construct the object 
properly initially.


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

https://reviews.llvm.org/D71199



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

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-06-02 Thread Balogh , Ádám via Phabricator via cfe-commits

baloghadamsoftware marked 4 inline comments as done.
baloghadamsoftware added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-assignment.cpp:29
+public:
+  Simple2() : n (0) {
+x = 0.0;

aaron.ballman wrote:
> By my reading of the core guideline 
> (https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c48-prefer-in-class-initializers-to-member-initializers-in-constructors-for-constant-initializers),
>  it looks like `n` should also be diagnosed because all of the constructors 
> in the class initialize the member to the same constant value. Is there a 
> reason to deviate from the rule (or have I missed something)?
> 
> Also, I'd like to see a test case like:
> ```
> class C {
>   int n;
> public:
>   C() { n = 0; }
>   explicit C(int) { n = 12; }
> };
> ```
This check only cares for initializations inside the body (rule `C.49`, but if 
the proper fix is to convert them to default member initializer according to 
rule `C.48` then we follow that rule in the fix). For initializations 
implemented as constructor member initializers but according to `C.48` they 
should have been implemented as default member initializers we already have 
check `modernize-use-default-member-init`.


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

https://reviews.llvm.org/D71199



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

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-06-02 Thread Balogh , Ádám via Phabricator via cfe-commits

baloghadamsoftware updated this revision to Diff 267833.
baloghadamsoftware added a comment.

Updated according to the comments.


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

https://reviews.llvm.org/D71199

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-assignment.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
@@ -0,0 +1,455 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-prefer-member-initializer %t
+
+class Simple1 {
+  int n;
+  // CHECK-FIXES: int n{0};
+  double x;
+  // CHECK-FIXES: double x{0.0};
+
+public:
+  Simple1() {
+n = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+x = 0.0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple1(int nn, double xx) {
+// CHECK-FIXES: Simple1(int nn, double xx) : n(nn), x(xx) {
+n = nn;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+x = xx;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple1() = default;
+};
+
+class Simple2 {
+  int n;
+  double x;
+  // CHECK-FIXES: double x{0.0};
+
+public:
+  Simple2() : n (0) {
+x = 0.0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple2(int nn, double xx) : n(nn) {
+// CHECK-FIXES: Simple2(int nn, double xx) : n(nn), x(xx) {
+x = xx;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple2() = default;
+};
+
+class Simple3 {
+  int n;
+  // CHECK-FIXES: int n{0};
+  double x;
+
+public:
+  Simple3() : x (0.0) {
+n = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple3(int nn, double xx) : x(xx) {
+// CHECK-FIXES: Simple3(int nn, double xx) : n(nn), x(xx) {
+n = nn;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple3() = default;
+};
+
+int something_int();
+double something_double();
+
+class Simple4 {
+  int n;
+
+public:
+  Simple4() {
+// CHECK-FIXES: Simple4() : n(something_int()) {
+n = something_int();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple4() = default;
+};
+
+static bool dice();
+
+class Complex1 {
+  int n;
+  int m;
+
+public:
+  Complex1() : n(0) {
+if (dice())
+  m = 1;
+// NO-MESSAGES: initialization of 'm' is nested in a conditional expression
+  }
+
+  ~Complex1() = default;
+};
+
+class Complex2 {
+  int n;
+  int m;
+
+public:
+  Complex2() : n(0) {
+if (!dice())
+  return;
+m = 1;
+// NO-MESSAGES: initialization of 'm' follows a conditional expression
+  }
+
+  ~Complex2() = default;
+};
+
+class Complex3 {
+  int n;
+  int m;
+
+public:
+  Complex3() : n(0) {
+while (dice())
+  m = 1;
+// NO-MESSAGES: initialization of 'm' is nested in a conditional loop
+  }
+
+  ~Complex3() = default;
+};
+
+class Complex4 {
+  int n;
+  int m;
+
+public:
+  Complex4() : n(0) {
+while (!dice())
+  return;
+m = 1;
+//

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-05-30 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp:106-107
+void PreferMemberInitializerCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+return;
+

This should now be done by overriding `bool isLanguageVersionSupported(const 
LangOptions ) const override;`, see 
bugprone/ThrowKeywordMissingCheck.h for an example.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst:3
+
+cppcoreguidelines-prefer-member-initializer
+===

You should probably link to the rule from somewhere in these docs.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-assignment.cpp:29
+public:
+  Simple2() : n (0) {
+x = 0.0;

By my reading of the core guideline 
(https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c48-prefer-in-class-initializers-to-member-initializers-in-constructors-for-constant-initializers),
 it looks like `n` should also be diagnosed because all of the constructors in 
the class initialize the member to the same constant value. Is there a reason 
to deviate from the rule (or have I missed something)?

Also, I'd like to see a test case like:
```
class C {
  int n;
public:
  C() { n = 0; }
  explicit C(int) { n = 12; }
};
```


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

https://reviews.llvm.org/D71199



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

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-05-26 Thread Balogh , Ádám via Phabricator via cfe-commits

baloghadamsoftware updated this revision to Diff 266277.
baloghadamsoftware added a comment.

Code reformatted.


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

https://reviews.llvm.org/D71199

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-assignment.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
@@ -0,0 +1,435 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-prefer-member-initializer %t
+
+class Simple1 {
+  int n;
+  // CHECK-FIXES: int n{0};
+  double x;
+  // CHECK-FIXES: double x{0.0};
+
+public:
+  Simple1() {
+n = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+x = 0.0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple1(int nn, double xx) {
+// CHECK-FIXES: Simple1(int nn, double xx) : n(nn), x(xx) {
+n = nn;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+x = xx;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple1() = default;
+};
+
+class Simple2 {
+  int n;
+  double x;
+  // CHECK-FIXES: double x{0.0};
+
+public:
+  Simple2() : n (0) {
+x = 0.0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple2(int nn, double xx) : n(nn) {
+// CHECK-FIXES: Simple2(int nn, double xx) : n(nn), x(xx) {
+x = xx;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple2() = default;
+};
+
+class Simple3 {
+  int n;
+  // CHECK-FIXES: int n{0};
+  double x;
+
+public:
+  Simple3() : x (0.0) {
+n = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple3(int nn, double xx) : x(xx) {
+// CHECK-FIXES: Simple3(int nn, double xx) : n(nn), x(xx) {
+n = nn;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple3() = default;
+};
+
+int something_int();
+double something_double();
+
+class Simple4 {
+  int n;
+
+public:
+  Simple4() {
+// CHECK-FIXES: Simple4() : n(something_int()) {
+n = something_int();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple4() = default;
+};
+
+static bool dice();
+
+class Complex1 {
+  int n;
+  int m;
+
+public:
+  Complex1() : n(0) {
+if (dice())
+  m = 1;
+// NO-MESSAGES: initialization of 'm' is nested in a conditional expression
+  }
+
+  ~Complex1() = default;
+};
+
+class Complex2 {
+  int n;
+  int m;
+
+public:
+  Complex2() : n(0) {
+if (!dice())
+  return;
+m = 1;
+// NO-MESSAGES: initialization of 'm' follows a conditional expression
+  }
+
+  ~Complex2() = default;
+};
+
+class Complex3 {
+  int n;
+  int m;
+
+public:
+  Complex3() : n(0) {
+while (dice())
+  m = 1;
+// NO-MESSAGES: initialization of 'm' is nested in a conditional loop
+  }
+
+  ~Complex3() = default;
+};
+
+class Complex4 {
+  int n;
+  int m;
+
+public:
+  Complex4() : n(0) {
+while (!dice())
+  return;
+m = 1;
+// NO-MESSAGES: initialization

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-05-26 Thread Balogh , Ádám via Phabricator via cfe-commits

baloghadamsoftware updated this revision to Diff 266151.
baloghadamsoftware marked an inline comment as done.
baloghadamsoftware added a comment.

Fix from `CPlusPlus2a` to `CPlusPlus20` after rebase.


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

https://reviews.llvm.org/D71199

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-assignment.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
@@ -0,0 +1,435 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-prefer-member-initializer %t
+
+class Simple1 {
+  int n;
+  // CHECK-FIXES: int n{0};
+  double x;
+  // CHECK-FIXES: double x{0.0};
+
+public:
+  Simple1() {
+n = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+x = 0.0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple1(int nn, double xx) {
+// CHECK-FIXES: Simple1(int nn, double xx) : n(nn), x(xx) {
+n = nn;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+x = xx;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple1() = default;
+};
+
+class Simple2 {
+  int n;
+  double x;
+  // CHECK-FIXES: double x{0.0};
+
+public:
+  Simple2() : n (0) {
+x = 0.0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple2(int nn, double xx) : n(nn) {
+// CHECK-FIXES: Simple2(int nn, double xx) : n(nn), x(xx) {
+x = xx;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple2() = default;
+};
+
+class Simple3 {
+  int n;
+  // CHECK-FIXES: int n{0};
+  double x;
+
+public:
+  Simple3() : x (0.0) {
+n = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple3(int nn, double xx) : x(xx) {
+// CHECK-FIXES: Simple3(int nn, double xx) : n(nn), x(xx) {
+n = nn;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple3() = default;
+};
+
+int something_int();
+double something_double();
+
+class Simple4 {
+  int n;
+
+public:
+  Simple4() {
+// CHECK-FIXES: Simple4() : n(something_int()) {
+n = something_int();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple4() = default;
+};
+
+static bool dice();
+
+class Complex1 {
+  int n;
+  int m;
+
+public:
+  Complex1() : n(0) {
+if (dice())
+  m = 1;
+// NO-MESSAGES: initialization of 'm' is nested in a conditional expression
+  }
+
+  ~Complex1() = default;
+};
+
+class Complex2 {
+  int n;
+  int m;
+
+public:
+  Complex2() : n(0) {
+if (!dice())
+  return;
+m = 1;
+// NO-MESSAGES: initialization of 'm' follows a conditional expression
+  }
+
+  ~Complex2() = default;
+};
+
+class Complex3 {
+  int n;
+  int m;
+
+public:
+  Complex3() : n(0) {
+while (dice())
+  m = 1;
+// NO-MESSAGES: initialization of 'm' is nested in a conditional loop
+  }
+
+  ~Complex3() = default;
+};
+
+class Complex4 {
+  int n;
+  int m;
+
+public:
+  Complex4() :

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-05-26 Thread Balogh , Ádám via Phabricator via cfe-commits

baloghadamsoftware updated this revision to Diff 266117.
baloghadamsoftware added a comment.

Updated according to the comments.


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

https://reviews.llvm.org/D71199

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-assignment.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
@@ -0,0 +1,435 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-prefer-member-initializer %t
+
+class Simple1 {
+  int n;
+  // CHECK-FIXES: int n{0};
+  double x;
+  // CHECK-FIXES: double x{0.0};
+
+public:
+  Simple1() {
+n = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+x = 0.0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple1(int nn, double xx) {
+// CHECK-FIXES: Simple1(int nn, double xx) : n(nn), x(xx) {
+n = nn;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+x = xx;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple1() = default;
+};
+
+class Simple2 {
+  int n;
+  double x;
+  // CHECK-FIXES: double x{0.0};
+
+public:
+  Simple2() : n (0) {
+x = 0.0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple2(int nn, double xx) : n(nn) {
+// CHECK-FIXES: Simple2(int nn, double xx) : n(nn), x(xx) {
+x = xx;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple2() = default;
+};
+
+class Simple3 {
+  int n;
+  // CHECK-FIXES: int n{0};
+  double x;
+
+public:
+  Simple3() : x (0.0) {
+n = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple3(int nn, double xx) : x(xx) {
+// CHECK-FIXES: Simple3(int nn, double xx) : n(nn), x(xx) {
+n = nn;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple3() = default;
+};
+
+int something_int();
+double something_double();
+
+class Simple4 {
+  int n;
+
+public:
+  Simple4() {
+// CHECK-FIXES: Simple4() : n(something_int()) {
+n = something_int();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple4() = default;
+};
+
+static bool dice();
+
+class Complex1 {
+  int n;
+  int m;
+
+public:
+  Complex1() : n(0) {
+if (dice())
+  m = 1;
+// NO-MESSAGES: initialization of 'm' is nested in a conditional expression
+  }
+
+  ~Complex1() = default;
+};
+
+class Complex2 {
+  int n;
+  int m;
+
+public:
+  Complex2() : n(0) {
+if (!dice())
+  return;
+m = 1;
+// NO-MESSAGES: initialization of 'm' follows a conditional expression
+  }
+
+  ~Complex2() = default;
+};
+
+class Complex3 {
+  int n;
+  int m;
+
+public:
+  Complex3() : n(0) {
+while (dice())
+  m = 1;
+// NO-MESSAGES: initialization of 'm' is nested in a conditional loop
+  }
+
+  ~Complex3() = default;
+};
+
+class Complex4 {
+  int n;
+  int m;
+
+public:
+  Complex4() : n(0) {
+while (!dice())
+  return;
+m = 1;
+//

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-05-26 Thread Balogh , Ádám via Phabricator via cfe-commits

baloghadamsoftware marked 7 inline comments as done.
baloghadamsoftware added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp:126-128
+  const auto *Body = dyn_cast_or_null(Ctor->getBody());
+  if (!Body)
+return;

aaron.ballman wrote:
> This can be hoisted into the matcher with `hasBody(anything())` I believe. 
> One interesting test case that's somewhat related are constructors that use a 
> function-try-block instead of a compound statement. e.g.,
> ```
> class C {
>   int i;
> 
> public:
>   C() try {
> i = 12;
>   } catch (...) {
>   }
> };
> ```
I think we should skip these cases similarly to initialization inside a try 
block in a body that is compound statement. Maybe the initialization throws 
thus it is inappropriate to convert it to member initializer. I also added a 
test case for this.


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

https://reviews.llvm.org/D71199



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

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-02-28 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp:25
+ isa(S) ||
+ isa(S) ||
+ isa(S) ||

How about `throw` expressions?



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp:61-65
+  const auto *DRE = dyn_cast(Value);
+  if (!DRE)
+return false;
+
+  return isa(DRE->getDecl());

```
if (const auto *DRE = dyn_cast(Value))
  return isa(DRE->getDecl())
return false;
```



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp:126-128
+  const auto *Body = dyn_cast_or_null(Ctor->getBody());
+  if (!Body)
+return;

This can be hoisted into the matcher with `hasBody(anything())` I believe. One 
interesting test case that's somewhat related are constructors that use a 
function-try-block instead of a compound statement. e.g.,
```
class C {
  int i;

public:
  C() try {
i = 12;
  } catch (...) {
  }
};
```



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst:6
+
+Finds member initializations in the constructor body which can be placed to
+the member initializers of the constructor instead. This does not only improves

placed to the -> converted into



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst:7
+Finds member initializations in the constructor body which can be placed to
+the member initializers of the constructor instead. This does not only improves
+the readability of the code but also positively affects its performance.

does not -> not


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

https://reviews.llvm.org/D71199



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

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-02-25 Thread Balogh, Ádám via Phabricator via cfe-commits

baloghadamsoftware marked 2 inline comments as done.
baloghadamsoftware added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp:43
+static bool isLiteral(const Expr *E) {
+  return isa(E) ||
+ isa(E) ||

aaron.ballman wrote:
> What about other kinds of literals like user-defined literals, or literal 
> class types? Should this be using `E->getType()->isLiteralType()`?
`E->getType()->isLiteralType()` does not work since it tells about the type, 
not the expression itself. Here we should return `true` for literals only, not 
for any expressions whose type is a literal type. Thus `int` is a literal type, 
`5` is an `int` literal, but `n` or `return_any_int()` not.


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

https://reviews.llvm.org/D71199



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

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-02-25 Thread Balogh, Ádám via Phabricator via cfe-commits

baloghadamsoftware updated this revision to Diff 246425.
baloghadamsoftware added a comment.
Herald added subscribers: martong, steakhal.

Minor update.


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

https://reviews.llvm.org/D71199

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-assignment.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
@@ -0,0 +1,407 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-prefer-member-initializer %t
+
+class Simple1 {
+  int n;
+  // CHECK-FIXES: int n{0};
+  double x;
+  // CHECK-FIXES: double x{0.0};
+
+public:
+  Simple1() {
+n = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+x = 0.0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple1(int nn, double xx) {
+// CHECK-FIXES: Simple1(int nn, double xx) : n(nn), x(xx) {
+n = nn;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+x = xx;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple1() = default;
+};
+
+class Simple2 {
+  int n;
+  double x;
+  // CHECK-FIXES: double x{0.0};
+
+public:
+  Simple2() : n (0) {
+x = 0.0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple2(int nn, double xx) : n(nn) {
+// CHECK-FIXES: Simple2(int nn, double xx) : n(nn), x(xx) {
+x = xx;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple2() = default;
+};
+
+class Simple3 {
+  int n;
+  // CHECK-FIXES: int n{0};
+  double x;
+
+public:
+  Simple3() : x (0.0) {
+n = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple3(int nn, double xx) : x(xx) {
+// CHECK-FIXES: Simple3(int nn, double xx) : n(nn), x(xx) {
+n = nn;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple3() = default;
+};
+
+int something_int();
+double something_double();
+
+class Simple4 {
+  int n;
+
+public:
+  Simple4() {
+// CHECK-FIXES: Simple4() : n(something_int()) {
+n = something_int();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple4() = default;
+};
+
+static bool dice();
+
+class Complex1 {
+  int n;
+  int m;
+
+public:
+  Complex1() : n(0) {
+if (dice())
+  m = 1;
+// NO-MESSAGES: initialization of 'm' is nested in a conditional expression
+  }
+
+  ~Complex1() = default;
+};
+
+class Complex2 {
+  int n;
+  int m;
+
+public:
+  Complex2() : n(0) {
+if (!dice())
+  return;
+m = 1;
+// NO-MESSAGES: initialization of 'm' follows a conditional expression
+  }
+
+  ~Complex2() = default;
+};
+
+class Complex3 {
+  int n;
+  int m;
+
+public:
+  Complex3() : n(0) {
+while (dice())
+  m = 1;
+// NO-MESSAGES: initialization of 'm' is nested in a conditional loop
+  }
+
+  ~Complex3() = default;
+};
+
+class Complex4 {
+  int n;
+  int m;
+
+public:
+  Complex4() : n(0) {
+while (!dice())
+  return;
+m

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-02-01 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp:47
+CheckFactories.registerCheck(
+"cppcoreguidelines-prefer-member-initializer");
 CheckFactories.registerCheck(

Please keep this list sorted alphabetically.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp:43
+static bool isLiteral(const Expr *E) {
+  return isa(E) ||
+ isa(E) ||

What about other kinds of literals like user-defined literals, or literal class 
types? Should this be using `E->getType()->isLiteralType()`?


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

https://reviews.llvm.org/D71199



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

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-01-16 Thread Balogh, Ádám via Phabricator via cfe-commits

baloghadamsoftware updated this revision to Diff 238527.
baloghadamsoftware retitled this revision from "[clang-tidy] New check 
cppcoreguidelines-prefer-initialization-list" to "[clang-tidy] New check 
cppcoreguidelines-prefer-initializer".
baloghadamsoftware added a comment.

Indentation of the code fixed in the documentation.


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

https://reviews.llvm.org/D71199

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-assignment.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
@@ -0,0 +1,407 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-prefer-member-initializer %t
+
+class Simple1 {
+  int n;
+  // CHECK-FIXES: int n{0};
+  double x;
+  // CHECK-FIXES: double x{0.0};
+
+public:
+  Simple1() {
+n = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+x = 0.0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple1(int nn, double xx) {
+// CHECK-FIXES: Simple1(int nn, double xx) : n(nn), x(xx) {
+n = nn;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+x = xx;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple1() = default;
+};
+
+class Simple2 {
+  int n;
+  double x;
+  // CHECK-FIXES: double x{0.0};
+
+public:
+  Simple2() : n (0) {
+x = 0.0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple2(int nn, double xx) : n(nn) {
+// CHECK-FIXES: Simple2(int nn, double xx) : n(nn), x(xx) {
+x = xx;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple2() = default;
+};
+
+class Simple3 {
+  int n;
+  // CHECK-FIXES: int n{0};
+  double x;
+
+public:
+  Simple3() : x (0.0) {
+n = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple3(int nn, double xx) : x(xx) {
+// CHECK-FIXES: Simple3(int nn, double xx) : n(nn), x(xx) {
+n = nn;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple3() = default;
+};
+
+int something_int();
+double something_double();
+
+class Simple4 {
+  int n;
+
+public:
+  Simple4() {
+// CHECK-FIXES: Simple4() : n(something_int()) {
+n = something_int();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple4() = default;
+};
+
+static bool dice();
+
+class Complex1 {
+  int n;
+  int m;
+
+public:
+  Complex1() : n(0) {
+if (dice())
+  m = 1;
+// NO-MESSAGES: initialization of 'm' is nested in a conditional expression
+  }
+
+  ~Complex1() = default;
+};
+
+class Complex2 {
+  int n;
+  int m;
+
+public:
+  Complex2() : n(0) {
+if (!dice())
+  return;
+m = 1;
+// NO-MESSAGES: initialization of 'm' follows a conditional expression
+  }
+
+  ~Complex2() = default;
+};
+
+class Complex3 {
+  int n;
+  int m;
+
+public:
+  Complex3() : n(0) {
+while (dice())
+  m = 1;
+// NO-MESSAGES: initialization of 'm' is

40 matches

Mail list logo