[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 167437.
JonasToth added a comment.

- write user-facing documentation


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/IsolateDeclCheck.cpp
  clang-tidy/readability/IsolateDeclCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/utils/LexerUtils.cpp
  clang-tidy/utils/LexerUtils.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-isolate-decl.rst
  test/clang-tidy/readability-isolate-decl-cxx17.cpp
  test/clang-tidy/readability-isolate-decl-fixing.cpp
  test/clang-tidy/readability-isolate-decl.cpp

Index: test/clang-tidy/readability-isolate-decl.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-isolate-decl.cpp
@@ -0,0 +1,356 @@
+// RUN: %check_clang_tidy %s readability-isolate-decl %t
+
+void f() {
+  int i;
+}
+
+void f2() {
+  int i, j, *k, lala = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int j;
+  // CHECK-FIXES: {{^  }}int *k;
+  // CHECK-FIXES: {{^  }}int lala = 42;
+
+  int normal, weird = /* comment */ 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int normal;
+  // CHECK-FIXES: {{^  }}int weird = /* comment */ 42;
+  //
+  int /* here is a comment */ v1,
+  // another comment
+  v2 = 42 // Ok, more comments
+  ;
+  // CHECK-MESSAGES: [[@LINE-4]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int /* here is a comment */ v1;
+  // CHECK-FIXES: {{^  }}int /* here is a comment */ // another comment
+  // CHECK-FIXES: {{^  }}v2 = 42 // Ok, more comments
+  // CHECK-FIXES: {{^  }};
+}
+
+void f3() {
+  int i, *pointer1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int *pointer1;
+  //
+  int *pointer2 = nullptr, *pointer3 = 
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int *pointer2 = nullptr;
+  // CHECK-FIXES: {{^  }}int *pointer3 = 
+}
+
+void f4() {
+  double d = 42. /* foo */, z = 43., /* hi */ y, c /* */ /*  */, l = 2.;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: double d = 42. /* foo */;
+  // CHECK-FIXES: {{^  }}double z = 43.;
+  // CHECK-FIXES: {{^  }}double /* hi */ y;
+  // CHECK-FIXES: {{^  }}double c /* */ /*  */;
+  // CHECK-FIXES: {{^  }}double l = 2.;
+}
+
+struct SomeClass {
+  SomeClass() = default;
+  SomeClass(int value);
+};
+
+class Point {
+  double x;
+  double y;
+
+public:
+  Point(double x, double y) : x(x), y(y) {}
+};
+
+class Rectangle {
+  Point TopLeft;
+  Point BottomRight;
+
+public:
+  Rectangle(Point TopLeft, Point BottomRight) : TopLeft(TopLeft), BottomRight(BottomRight) {}
+};
+
+void f5() {
+  SomeClass v1, v2(42), v3{42}, v4(42.5);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: SomeClass v1;
+  // CHECK-FIXES: {{^  }}SomeClass v2(42);
+  // CHECK-FIXES: {{^  }}SomeClass v3{42};
+  // CHECK-FIXES: {{^  }}SomeClass v4(42.5);
+
+  SomeClass v5 = 42, *p1 = nullptr;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: SomeClass v5 = 42;
+  // CHECK-FIXES: {{^  }}SomeClass *p1 = nullptr;
+
+  Point P1(0., 2.), P2{2., 0.};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: Point P1(0., 2.);
+  // CHECK-FIXES: {{^  }}Point P2{2., 0.};
+
+  Rectangle R1({0., 0.}, {1., -2.}), R2{{0., 1.}, {1., 0.}}, R3(P1, P2), R4{P1, P2};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: Rectangle R1({0., 0.}, {1., -2.});
+  // CHECK-FIXES: {{^  }}Rectangle R2{{[{][{]}}0., 1.}, {1., 0.{{[}][}]}};
+  // CHECK-FIXES: {{^  }}Rectangle R3(P1, P2);
+  // CHECK-FIXES: {{^  }}Rectangle R4{P1, P2};
+}
+
+void f6() {
+  int array1[] = {1, 2, 3, 4}, array2[] = {1, 2, 3}, value1, value2 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int array1[] = {1, 2, 3, 4};
+  // CHECK-FIXES: {{^  }}int array2[] = {1, 2, 3};
+  // CHECK-FIXES: {{^  }}int value1;
+  // CHECK-FIXES: {{^  }}int value2 = 42;
+}
+
+template 
+struct TemplatedType {
+  TemplatedType() = default;
+  TemplatedType(T value);
+};
+
+void f7() {
+  TemplatedType TT1(42), TT2{42}, TT3;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: 

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 167400.
JonasToth added a comment.

- last cleanup for today, testing on clang/lib looks good, but is already very 
clean


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/IsolateDeclCheck.cpp
  clang-tidy/readability/IsolateDeclCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/utils/LexerUtils.cpp
  clang-tidy/utils/LexerUtils.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-isolate-decl.rst
  test/clang-tidy/readability-isolate-decl-cxx17.cpp
  test/clang-tidy/readability-isolate-decl-fixing.cpp
  test/clang-tidy/readability-isolate-decl.cpp

Index: test/clang-tidy/readability-isolate-decl.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-isolate-decl.cpp
@@ -0,0 +1,356 @@
+// RUN: %check_clang_tidy %s readability-isolate-decl %t
+
+void f() {
+  int i;
+}
+
+void f2() {
+  int i, j, *k, lala = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int j;
+  // CHECK-FIXES: {{^  }}int *k;
+  // CHECK-FIXES: {{^  }}int lala = 42;
+
+  int normal, weird = /* comment */ 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int normal;
+  // CHECK-FIXES: {{^  }}int weird = /* comment */ 42;
+  //
+  int /* here is a comment */ v1,
+  // another comment
+  v2 = 42 // Ok, more comments
+  ;
+  // CHECK-MESSAGES: [[@LINE-4]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int /* here is a comment */ v1;
+  // CHECK-FIXES: {{^  }}int /* here is a comment */ // another comment
+  // CHECK-FIXES: {{^  }}v2 = 42 // Ok, more comments
+  // CHECK-FIXES: {{^  }};
+}
+
+void f3() {
+  int i, *pointer1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int *pointer1;
+  //
+  int *pointer2 = nullptr, *pointer3 = 
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int *pointer2 = nullptr;
+  // CHECK-FIXES: {{^  }}int *pointer3 = 
+}
+
+void f4() {
+  double d = 42. /* foo */, z = 43., /* hi */ y, c /* */ /*  */, l = 2.;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: double d = 42. /* foo */;
+  // CHECK-FIXES: {{^  }}double z = 43.;
+  // CHECK-FIXES: {{^  }}double /* hi */ y;
+  // CHECK-FIXES: {{^  }}double c /* */ /*  */;
+  // CHECK-FIXES: {{^  }}double l = 2.;
+}
+
+struct SomeClass {
+  SomeClass() = default;
+  SomeClass(int value);
+};
+
+class Point {
+  double x;
+  double y;
+
+public:
+  Point(double x, double y) : x(x), y(y) {}
+};
+
+class Rectangle {
+  Point TopLeft;
+  Point BottomRight;
+
+public:
+  Rectangle(Point TopLeft, Point BottomRight) : TopLeft(TopLeft), BottomRight(BottomRight) {}
+};
+
+void f5() {
+  SomeClass v1, v2(42), v3{42}, v4(42.5);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: SomeClass v1;
+  // CHECK-FIXES: {{^  }}SomeClass v2(42);
+  // CHECK-FIXES: {{^  }}SomeClass v3{42};
+  // CHECK-FIXES: {{^  }}SomeClass v4(42.5);
+
+  SomeClass v5 = 42, *p1 = nullptr;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: SomeClass v5 = 42;
+  // CHECK-FIXES: {{^  }}SomeClass *p1 = nullptr;
+
+  Point P1(0., 2.), P2{2., 0.};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: Point P1(0., 2.);
+  // CHECK-FIXES: {{^  }}Point P2{2., 0.};
+
+  Rectangle R1({0., 0.}, {1., -2.}), R2{{0., 1.}, {1., 0.}}, R3(P1, P2), R4{P1, P2};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: Rectangle R1({0., 0.}, {1., -2.});
+  // CHECK-FIXES: {{^  }}Rectangle R2{{[{][{]}}0., 1.}, {1., 0.{{[}][}]}};
+  // CHECK-FIXES: {{^  }}Rectangle R3(P1, P2);
+  // CHECK-FIXES: {{^  }}Rectangle R4{P1, P2};
+}
+
+void f6() {
+  int array1[] = {1, 2, 3, 4}, array2[] = {1, 2, 3}, value1, value2 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int array1[] = {1, 2, 3, 4};
+  // CHECK-FIXES: {{^  }}int array2[] = {1, 2, 3};
+  // CHECK-FIXES: {{^  }}int value1;
+  // CHECK-FIXES: {{^  }}int value2 = 42;
+}
+
+template 
+struct TemplatedType {
+  TemplatedType() = default;
+  TemplatedType(T value);
+};
+
+void f7() {
+  TemplatedType TT1(42), 

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 167397.
JonasToth added a comment.

- fix doc typo


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/IsolateDeclCheck.cpp
  clang-tidy/readability/IsolateDeclCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/utils/LexerUtils.cpp
  clang-tidy/utils/LexerUtils.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-isolate-decl.rst
  test/clang-tidy/readability-isolate-decl-cxx17.cpp
  test/clang-tidy/readability-isolate-decl-fixing.cpp
  test/clang-tidy/readability-isolate-decl.cpp

Index: test/clang-tidy/readability-isolate-decl.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-isolate-decl.cpp
@@ -0,0 +1,356 @@
+// RUN: %check_clang_tidy %s readability-isolate-decl %t
+
+void f() {
+  int i;
+}
+
+void f2() {
+  int i, j, *k, lala = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int j;
+  // CHECK-FIXES: {{^  }}int *k;
+  // CHECK-FIXES: {{^  }}int lala = 42;
+
+  int normal, weird = /* comment */ 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int normal;
+  // CHECK-FIXES: {{^  }}int weird = /* comment */ 42;
+  //
+  int /* here is a comment */ v1,
+  // another comment
+  v2 = 42 // Ok, more comments
+  ;
+  // CHECK-MESSAGES: [[@LINE-4]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int /* here is a comment */ v1;
+  // CHECK-FIXES: {{^  }}int /* here is a comment */ // another comment
+  // CHECK-FIXES: {{^  }}v2 = 42 // Ok, more comments
+  // CHECK-FIXES: {{^  }};
+}
+
+void f3() {
+  int i, *pointer1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int *pointer1;
+  //
+  int *pointer2 = nullptr, *pointer3 = 
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int *pointer2 = nullptr;
+  // CHECK-FIXES: {{^  }}int *pointer3 = 
+}
+
+void f4() {
+  double d = 42. /* foo */, z = 43., /* hi */ y, c /* */ /*  */, l = 2.;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: double d = 42. /* foo */;
+  // CHECK-FIXES: {{^  }}double z = 43.;
+  // CHECK-FIXES: {{^  }}double /* hi */ y;
+  // CHECK-FIXES: {{^  }}double c /* */ /*  */;
+  // CHECK-FIXES: {{^  }}double l = 2.;
+}
+
+struct SomeClass {
+  SomeClass() = default;
+  SomeClass(int value);
+};
+
+class Point {
+  double x;
+  double y;
+
+public:
+  Point(double x, double y) : x(x), y(y) {}
+};
+
+class Rectangle {
+  Point TopLeft;
+  Point BottomRight;
+
+public:
+  Rectangle(Point TopLeft, Point BottomRight) : TopLeft(TopLeft), BottomRight(BottomRight) {}
+};
+
+void f5() {
+  SomeClass v1, v2(42), v3{42}, v4(42.5);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: SomeClass v1;
+  // CHECK-FIXES: {{^  }}SomeClass v2(42);
+  // CHECK-FIXES: {{^  }}SomeClass v3{42};
+  // CHECK-FIXES: {{^  }}SomeClass v4(42.5);
+
+  SomeClass v5 = 42, *p1 = nullptr;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: SomeClass v5 = 42;
+  // CHECK-FIXES: {{^  }}SomeClass *p1 = nullptr;
+
+  Point P1(0., 2.), P2{2., 0.};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: Point P1(0., 2.);
+  // CHECK-FIXES: {{^  }}Point P2{2., 0.};
+
+  Rectangle R1({0., 0.}, {1., -2.}), R2{{0., 1.}, {1., 0.}}, R3(P1, P2), R4{P1, P2};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: Rectangle R1({0., 0.}, {1., -2.});
+  // CHECK-FIXES: {{^  }}Rectangle R2{{[{][{]}}0., 1.}, {1., 0.{{[}][}]}};
+  // CHECK-FIXES: {{^  }}Rectangle R3(P1, P2);
+  // CHECK-FIXES: {{^  }}Rectangle R4{P1, P2};
+}
+
+void f6() {
+  int array1[] = {1, 2, 3, 4}, array2[] = {1, 2, 3}, value1, value2 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int array1[] = {1, 2, 3, 4};
+  // CHECK-FIXES: {{^  }}int array2[] = {1, 2, 3};
+  // CHECK-FIXES: {{^  }}int value1;
+  // CHECK-FIXES: {{^  }}int value2 = 42;
+}
+
+template 
+struct TemplatedType {
+  TemplatedType() = default;
+  TemplatedType(T value);
+};
+
+void f7() {
+  TemplatedType TT1(42), TT2{42}, TT3;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple 

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 167389.
JonasToth added a comment.

- bail out in transformation if there are PP directives in the source range of 
the DeclStmt


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/IsolateDeclCheck.cpp
  clang-tidy/readability/IsolateDeclCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/utils/LexerUtils.cpp
  clang-tidy/utils/LexerUtils.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-isolate-decl.rst
  test/clang-tidy/readability-isolate-decl-cxx17.cpp
  test/clang-tidy/readability-isolate-decl-fixing.cpp
  test/clang-tidy/readability-isolate-decl.cpp

Index: test/clang-tidy/readability-isolate-decl.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-isolate-decl.cpp
@@ -0,0 +1,356 @@
+// RUN: %check_clang_tidy %s readability-isolate-decl %t
+
+void f() {
+  int i;
+}
+
+void f2() {
+  int i, j, *k, lala = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int j;
+  // CHECK-FIXES: {{^  }}int *k;
+  // CHECK-FIXES: {{^  }}int lala = 42;
+
+  int normal, weird = /* comment */ 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int normal;
+  // CHECK-FIXES: {{^  }}int weird = /* comment */ 42;
+  //
+  int /* here is a comment */ v1,
+  // another comment
+  v2 = 42 // Ok, more comments
+  ;
+  // CHECK-MESSAGES: [[@LINE-4]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int /* here is a comment */ v1;
+  // CHECK-FIXES: {{^  }}int /* here is a comment */ // another comment
+  // CHECK-FIXES: {{^  }}v2 = 42 // Ok, more comments
+  // CHECK-FIXES: {{^  }};
+}
+
+void f3() {
+  int i, *pointer1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int *pointer1;
+  //
+  int *pointer2 = nullptr, *pointer3 = 
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int *pointer2 = nullptr;
+  // CHECK-FIXES: {{^  }}int *pointer3 = 
+}
+
+void f4() {
+  double d = 42. /* foo */, z = 43., /* hi */ y, c /* */ /*  */, l = 2.;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: double d = 42. /* foo */;
+  // CHECK-FIXES: {{^  }}double z = 43.;
+  // CHECK-FIXES: {{^  }}double /* hi */ y;
+  // CHECK-FIXES: {{^  }}double c /* */ /*  */;
+  // CHECK-FIXES: {{^  }}double l = 2.;
+}
+
+struct SomeClass {
+  SomeClass() = default;
+  SomeClass(int value);
+};
+
+class Point {
+  double x;
+  double y;
+
+public:
+  Point(double x, double y) : x(x), y(y) {}
+};
+
+class Rectangle {
+  Point TopLeft;
+  Point BottomRight;
+
+public:
+  Rectangle(Point TopLeft, Point BottomRight) : TopLeft(TopLeft), BottomRight(BottomRight) {}
+};
+
+void f5() {
+  SomeClass v1, v2(42), v3{42}, v4(42.5);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: SomeClass v1;
+  // CHECK-FIXES: {{^  }}SomeClass v2(42);
+  // CHECK-FIXES: {{^  }}SomeClass v3{42};
+  // CHECK-FIXES: {{^  }}SomeClass v4(42.5);
+
+  SomeClass v5 = 42, *p1 = nullptr;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: SomeClass v5 = 42;
+  // CHECK-FIXES: {{^  }}SomeClass *p1 = nullptr;
+
+  Point P1(0., 2.), P2{2., 0.};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: Point P1(0., 2.);
+  // CHECK-FIXES: {{^  }}Point P2{2., 0.};
+
+  Rectangle R1({0., 0.}, {1., -2.}), R2{{0., 1.}, {1., 0.}}, R3(P1, P2), R4{P1, P2};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: Rectangle R1({0., 0.}, {1., -2.});
+  // CHECK-FIXES: {{^  }}Rectangle R2{{[{][{]}}0., 1.}, {1., 0.{{[}][}]}};
+  // CHECK-FIXES: {{^  }}Rectangle R3(P1, P2);
+  // CHECK-FIXES: {{^  }}Rectangle R4{P1, P2};
+}
+
+void f6() {
+  int array1[] = {1, 2, 3, 4}, array2[] = {1, 2, 3}, value1, value2 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int array1[] = {1, 2, 3, 4};
+  // CHECK-FIXES: {{^  }}int array2[] = {1, 2, 3};
+  // CHECK-FIXES: {{^  }}int value1;
+  // CHECK-FIXES: {{^  }}int value2 = 42;
+}
+
+template 
+struct TemplatedType {
+  TemplatedType() = default;
+  TemplatedType(T value);
+};
+
+void f7() {
+  TemplatedType 

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

JonasToth marked 9 inline comments as done.
JonasToth added inline comments.



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:211
+  std::vector Snippets;
+  Snippets.reserve(Ranges.size());
+

kbobyrev wrote:
> nit: It would be probably easier to have `Snippets(Ranges.size()` and then 
> insert each `Snippet` to the correct position. That would require sacrificing 
> for-range loop and having a counter, but I think that it would make this 
> piece slightly better. Up to you, I don't have strong opinion about this part.
I prefer this one :)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 167381.
JonasToth marked 9 inline comments as done.
JonasToth added a comment.

- simplification on FIXME:/TODO:


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/IsolateDeclCheck.cpp
  clang-tidy/readability/IsolateDeclCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/utils/LexerUtils.cpp
  clang-tidy/utils/LexerUtils.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-isolate-decl.rst
  test/clang-tidy/readability-isolate-decl-cxx17.cpp
  test/clang-tidy/readability-isolate-decl-fixing.cpp
  test/clang-tidy/readability-isolate-decl.cpp

Index: test/clang-tidy/readability-isolate-decl.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-isolate-decl.cpp
@@ -0,0 +1,347 @@
+// RUN: %check_clang_tidy %s readability-isolate-decl %t
+
+void f() {
+  int i;
+}
+
+void f2() {
+  int i, j, *k, lala = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int j;
+  // CHECK-FIXES: {{^  }}int *k;
+  // CHECK-FIXES: {{^  }}int lala = 42;
+
+  int normal, weird = /* comment */ 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int normal;
+  // CHECK-FIXES: {{^  }}int weird = /* comment */ 42;
+  //
+  int /* here is a comment */ v1,
+  // another comment
+  v2 = 42 // Ok, more comments
+  ;
+  // CHECK-MESSAGES: [[@LINE-4]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int /* here is a comment */ v1;
+  // CHECK-FIXES: {{^  }}int /* here is a comment */ // another comment
+  // CHECK-FIXES: {{^  }}v2 = 42 // Ok, more comments
+  // CHECK-FIXES: {{^  }};
+}
+
+void f3() {
+  int i, *pointer1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int *pointer1;
+  //
+  int *pointer2 = nullptr, *pointer3 = 
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int *pointer2 = nullptr;
+  // CHECK-FIXES: {{^  }}int *pointer3 = 
+}
+
+void f4() {
+  double d = 42. /* foo */, z = 43., /* hi */ y, c /* */ /*  */, l = 2.;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: double d = 42. /* foo */;
+  // CHECK-FIXES: {{^  }}double z = 43.;
+  // CHECK-FIXES: {{^  }}double /* hi */ y;
+  // CHECK-FIXES: {{^  }}double c /* */ /*  */;
+  // CHECK-FIXES: {{^  }}double l = 2.;
+}
+
+struct SomeClass {
+  SomeClass() = default;
+  SomeClass(int value);
+};
+
+class Point {
+  double x;
+  double y;
+
+public:
+  Point(double x, double y) : x(x), y(y) {}
+};
+
+class Rectangle {
+  Point TopLeft;
+  Point BottomRight;
+
+public:
+  Rectangle(Point TopLeft, Point BottomRight) : TopLeft(TopLeft), BottomRight(BottomRight) {}
+};
+
+void f5() {
+  SomeClass v1, v2(42), v3{42}, v4(42.5);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: SomeClass v1;
+  // CHECK-FIXES: {{^  }}SomeClass v2(42);
+  // CHECK-FIXES: {{^  }}SomeClass v3{42};
+  // CHECK-FIXES: {{^  }}SomeClass v4(42.5);
+
+  SomeClass v5 = 42, *p1 = nullptr;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: SomeClass v5 = 42;
+  // CHECK-FIXES: {{^  }}SomeClass *p1 = nullptr;
+
+  Point P1(0., 2.), P2{2., 0.};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: Point P1(0., 2.);
+  // CHECK-FIXES: {{^  }}Point P2{2., 0.};
+
+  Rectangle R1({0., 0.}, {1., -2.}), R2{{0., 1.}, {1., 0.}}, R3(P1, P2), R4{P1, P2};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: Rectangle R1({0., 0.}, {1., -2.});
+  // CHECK-FIXES: {{^  }}Rectangle R2{{[{][{]}}0., 1.}, {1., 0.{{[}][}]}};
+  // CHECK-FIXES: {{^  }}Rectangle R3(P1, P2);
+  // CHECK-FIXES: {{^  }}Rectangle R4{P1, P2};
+}
+
+void f6() {
+  int array1[] = {1, 2, 3, 4}, array2[] = {1, 2, 3}, value1, value2 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int array1[] = {1, 2, 3, 4};
+  // CHECK-FIXES: {{^  }}int array2[] = {1, 2, 3};
+  // CHECK-FIXES: {{^  }}int value1;
+  // CHECK-FIXES: {{^  }}int value2 = 42;
+}
+
+template 
+struct TemplatedType {
+  TemplatedType() = default;
+  TemplatedType(T value);
+};
+
+void f7() {
+  TemplatedType TT1(42), TT2{42}, TT3;

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:343
+  auto Diag =
+  diag(WholeDecl->getBeginLoc(), "this statement declares %0 variables")
+  << static_cast(

aaron.ballman wrote:
> JonasToth wrote:
> > kbobyrev wrote:
> > > aaron.ballman wrote:
> > > > lebedev.ri wrote:
> > > > > kbobyrev wrote:
> > > > > > JonasToth wrote:
> > > > > > > kbobyrev wrote:
> > > > > > > > How about `multiple declarations within a single statement 
> > > > > > > > hurts readability`?
> > > > > > > s/hurts/reduces/? hurts sound a bit weird i think.
> > > > > > > 
> > > > > > > Lebedev wanted the number of decls in the diagnostic, would you 
> > > > > > > include it or rather now?
> > > > > > "decreases" is also fine. "hurts" is probably too strong, I agree.
> > > > > > 
> > > > > > Up to you. Personally, I don't see any value in having the 
> > > > > > diagnostic message saying "hey, you have 2 declarations within one 
> > > > > > statement, that's really bad!" or "hey, you have 5 declarations 
> > > > > > within one statement..." - in both cases the point is that there 
> > > > > > are *multiple* declarations. I also don't think it would make 
> > > > > > debugging easier because you also check the formatting, so you 
> > > > > > already imply that the correct number of declarations was detected.
> > > > > > 
> > > > > > I'm interested to know what @lebedev.ri thinks.
> > > > > > I'm interested to know what @lebedev.ri thinks.
> > > > > 
> > > > > "This translation unit has an error. Can not continue" is also a 
> > > > > diagnostic message.
> > > > > Why are we not ok with that one, and want compiler to be a bit more 
> > > > > specific?
> > > > > 
> > > > > Similarly here, why just point out that this code is bad as per the 
> > > > > check,
> > > > > without giving a little bit more info, that you already have?
> > > > > "This translation unit has an error. Can not continue" is also a 
> > > > > diagnostic message.
> > > > >Why are we not ok with that one, and want compiler to be a bit more 
> > > > >specific?
> > > > >
> > > > > Similarly here, why just point out that this code is bad as per the 
> > > > > check, without giving a little bit more info, that you already have?
> > > > 
> > > > More information doesn't always equate into more understanding, 
> > > > especially when that information causes a distraction. For instance, 
> > > > you could argue that the type of the declared variables is also 
> > > > information we already have, but what purpose would it serve to tell it 
> > > > to the user?
> > > > 
> > > > Can you give an example where the specific number of declarations 
> > > > involved would help you to correct the diagnostic? I can't come up with 
> > > > one, so it feels to me like having the count is more of a distraction; 
> > > > especially given that there's no configurable threshold for "now you 
> > > > have too many declarations". I'd feel differently if there was a config 
> > > > option, because then the count is truly useful to know.
> > > Oh, but that's different: "This translation unit has an error. Can not 
> > > continue" does not provide enough information for users to fix the issue, 
> > > pointing out that there are *multiple* declarations per statement is 
> > > definitely enough.
> > I am personally against having the number in the diagnostic as well, it 
> > would only add value if the declarations are expanded from a macro.
> > 
> > @aaron.ballman Configuration of this check would be intersting but i would 
> > rather postpone that and have a basic working check first. Given that this 
> > aims to be utility-like to evaluate `const-correctness` and/or to be usable 
> > with other checks doing type transformations.
> Yeah, I wasn't suggesting a threshold config option for this patch so much as 
> pointing out why I'm opposed to putting the count in the diagnostic.
I used a different wording without the variable count. Adding configuration, 
more advanced diagnostic can be done in a follow up or in a different check, 
e.g. `readability-function-size`, as it does counting (might not be the perfect 
fit though)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 167377.
JonasToth added a comment.

- address review comments, most nits solved
- fix typedefs and function pointers with comments as distraction
- make memberpointer detection more accurate
- move functioning member pointer test
- clean debug output
- clean lexer utils as well
- adjust diagnostic message


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/IsolateDeclCheck.cpp
  clang-tidy/readability/IsolateDeclCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/utils/LexerUtils.cpp
  clang-tidy/utils/LexerUtils.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-isolate-decl.rst
  test/clang-tidy/readability-isolate-decl-cxx17.cpp
  test/clang-tidy/readability-isolate-decl-fixing.cpp
  test/clang-tidy/readability-isolate-decl.cpp

Index: test/clang-tidy/readability-isolate-decl.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-isolate-decl.cpp
@@ -0,0 +1,347 @@
+// RUN: %check_clang_tidy %s readability-isolate-decl %t
+
+void f() {
+  int i;
+}
+
+void f2() {
+  int i, j, *k, lala = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int j;
+  // CHECK-FIXES: {{^  }}int *k;
+  // CHECK-FIXES: {{^  }}int lala = 42;
+
+  int normal, weird = /* comment */ 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int normal;
+  // CHECK-FIXES: {{^  }}int weird = /* comment */ 42;
+  //
+  int /* here is a comment */ v1,
+  // another comment
+  v2 = 42 // Ok, more comments
+  ;
+  // CHECK-MESSAGES: [[@LINE-4]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int /* here is a comment */ v1;
+  // CHECK-FIXES: {{^  }}int /* here is a comment */ // another comment
+  // CHECK-FIXES: {{^  }}v2 = 42 // Ok, more comments
+  // CHECK-FIXES: {{^  }};
+}
+
+void f3() {
+  int i, *pointer1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int *pointer1;
+  //
+  int *pointer2 = nullptr, *pointer3 = 
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int *pointer2 = nullptr;
+  // CHECK-FIXES: {{^  }}int *pointer3 = 
+}
+
+void f4() {
+  double d = 42. /* foo */, z = 43., /* hi */ y, c /* */ /*  */, l = 2.;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: double d = 42. /* foo */;
+  // CHECK-FIXES: {{^  }}double z = 43.;
+  // CHECK-FIXES: {{^  }}double /* hi */ y;
+  // CHECK-FIXES: {{^  }}double c /* */ /*  */;
+  // CHECK-FIXES: {{^  }}double l = 2.;
+}
+
+struct SomeClass {
+  SomeClass() = default;
+  SomeClass(int value);
+};
+
+class Point {
+  double x;
+  double y;
+
+public:
+  Point(double x, double y) : x(x), y(y) {}
+};
+
+class Rectangle {
+  Point TopLeft;
+  Point BottomRight;
+
+public:
+  Rectangle(Point TopLeft, Point BottomRight) : TopLeft(TopLeft), BottomRight(BottomRight) {}
+};
+
+void f5() {
+  SomeClass v1, v2(42), v3{42}, v4(42.5);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: SomeClass v1;
+  // CHECK-FIXES: {{^  }}SomeClass v2(42);
+  // CHECK-FIXES: {{^  }}SomeClass v3{42};
+  // CHECK-FIXES: {{^  }}SomeClass v4(42.5);
+
+  SomeClass v5 = 42, *p1 = nullptr;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: SomeClass v5 = 42;
+  // CHECK-FIXES: {{^  }}SomeClass *p1 = nullptr;
+
+  Point P1(0., 2.), P2{2., 0.};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: Point P1(0., 2.);
+  // CHECK-FIXES: {{^  }}Point P2{2., 0.};
+
+  Rectangle R1({0., 0.}, {1., -2.}), R2{{0., 1.}, {1., 0.}}, R3(P1, P2), R4{P1, P2};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: Rectangle R1({0., 0.}, {1., -2.});
+  // CHECK-FIXES: {{^  }}Rectangle R2{{[{][{]}}0., 1.}, {1., 0.{{[}][}]}};
+  // CHECK-FIXES: {{^  }}Rectangle R3(P1, P2);
+  // CHECK-FIXES: {{^  }}Rectangle R4{P1, P2};
+}
+
+void f6() {
+  int array1[] = {1, 2, 3, 4}, array2[] = {1, 2, 3}, value1, value2 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int array1[] = {1, 2, 3, 4};
+  // CHECK-FIXES: {{^  }}int array2[] = {1, 2, 3};
+  // CHECK-FIXES: {{^  }}int value1;
+  // 

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

JonasToth marked 11 inline comments as done.
JonasToth added inline comments.



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:68
+if (Start.isInvalid() || Start.isMacroID())
+  return SourceLocation();
+  }

kbobyrev wrote:
> Also, I don't think this should happen with the correct behavior. 
> `llvm::Expected` looks like a better alternative for error handling here.
It currently happens for `typedef int * IntPtr; IntPtr p;`.  Regarding 
`Expected` see other comment.



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:278
+  if (!PotentialSnippets)
+return;
+

kbobyrev wrote:
> Both for `PotentialSlices` and `PotentialSnippets`: is there any case where 
> any can not be determined and this is not an error? They both are 
> `llvm::Optional`s and the `check(Result)` just returns if any of them are 
> not set, is there any case when any of the variables is actually not set, but 
> this is the correct behavior? If not, IMO it would be better to use 
> `llvm::Expected`: then, if the check fails, either print `.takeError()` 
> message or propagate it "upstairs" (although I'm not really sure what would 
> be better here).
Currently the `Optional` and checking for invalid and so on everywhere is 
result of the WIP and the bugs I encountered and try to fix. In general the 
operations, like re-lexing can fail judging from the interface, same for 
retrieving SourceLocations, so I took the very conservative approach to check 
everything.
In my opinion it is more user-friendly to just emit the warning, but without 
transformation instead of an error that the transformation fails for some 
reason. Running this check over multiple bigger code-bases will give better 
data to make an actual judgement on this.

In general there are code-constructs where transformation _could_ be dangerous 
or unexpted, all I can think of includes macros.

```
int SomeConfig = 42,
#if BUILD_WITH_X
 AnotherConfig = 43;
#else 
AnotherConfig = 44;
#endif
```

This example is not unreasonable and currently transformed incorrectly. I 
wanted to have an mechanism to just bail if something is fishy. I tried 
`llvm::Expected` but thought that using `llvm::Optional` was just simpler for 
prototyping.
Semantically `Expected` would fit better, because the transformation is 
actually expected to work. Emitting notes that contain information from the 
errors might be user-friendly, but having a `note: Can not automatically 
transform this declaration statement` is probably similarly  informative.
For now I'd stick with `Optional` and it might be even the better fit in 
general, considering that more declaration kinds should be implemented in the 
future (similar to the other, currently staled check).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:343
+  auto Diag =
+  diag(WholeDecl->getBeginLoc(), "this statement declares %0 variables")
+  << static_cast(

JonasToth wrote:
> kbobyrev wrote:
> > aaron.ballman wrote:
> > > lebedev.ri wrote:
> > > > kbobyrev wrote:
> > > > > JonasToth wrote:
> > > > > > kbobyrev wrote:
> > > > > > > How about `multiple declarations within a single statement hurts 
> > > > > > > readability`?
> > > > > > s/hurts/reduces/? hurts sound a bit weird i think.
> > > > > > 
> > > > > > Lebedev wanted the number of decls in the diagnostic, would you 
> > > > > > include it or rather now?
> > > > > "decreases" is also fine. "hurts" is probably too strong, I agree.
> > > > > 
> > > > > Up to you. Personally, I don't see any value in having the diagnostic 
> > > > > message saying "hey, you have 2 declarations within one statement, 
> > > > > that's really bad!" or "hey, you have 5 declarations within one 
> > > > > statement..." - in both cases the point is that there are *multiple* 
> > > > > declarations. I also don't think it would make debugging easier 
> > > > > because you also check the formatting, so you already imply that the 
> > > > > correct number of declarations was detected.
> > > > > 
> > > > > I'm interested to know what @lebedev.ri thinks.
> > > > > I'm interested to know what @lebedev.ri thinks.
> > > > 
> > > > "This translation unit has an error. Can not continue" is also a 
> > > > diagnostic message.
> > > > Why are we not ok with that one, and want compiler to be a bit more 
> > > > specific?
> > > > 
> > > > Similarly here, why just point out that this code is bad as per the 
> > > > check,
> > > > without giving a little bit more info, that you already have?
> > > > "This translation unit has an error. Can not continue" is also a 
> > > > diagnostic message.
> > > >Why are we not ok with that one, and want compiler to be a bit more 
> > > >specific?
> > > >
> > > > Similarly here, why just point out that this code is bad as per the 
> > > > check, without giving a little bit more info, that you already have?
> > > 
> > > More information doesn't always equate into more understanding, 
> > > especially when that information causes a distraction. For instance, you 
> > > could argue that the type of the declared variables is also information 
> > > we already have, but what purpose would it serve to tell it to the user?
> > > 
> > > Can you give an example where the specific number of declarations 
> > > involved would help you to correct the diagnostic? I can't come up with 
> > > one, so it feels to me like having the count is more of a distraction; 
> > > especially given that there's no configurable threshold for "now you have 
> > > too many declarations". I'd feel differently if there was a config 
> > > option, because then the count is truly useful to know.
> > Oh, but that's different: "This translation unit has an error. Can not 
> > continue" does not provide enough information for users to fix the issue, 
> > pointing out that there are *multiple* declarations per statement is 
> > definitely enough.
> I am personally against having the number in the diagnostic as well, it would 
> only add value if the declarations are expanded from a macro.
> 
> @aaron.ballman Configuration of this check would be intersting but i would 
> rather postpone that and have a basic working check first. Given that this 
> aims to be utility-like to evaluate `const-correctness` and/or to be usable 
> with other checks doing type transformations.
Yeah, I wasn't suggesting a threshold config option for this patch so much as 
pointing out why I'm opposed to putting the count in the diagnostic.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:343
+  auto Diag =
+  diag(WholeDecl->getBeginLoc(), "this statement declares %0 variables")
+  << static_cast(

kbobyrev wrote:
> aaron.ballman wrote:
> > lebedev.ri wrote:
> > > kbobyrev wrote:
> > > > JonasToth wrote:
> > > > > kbobyrev wrote:
> > > > > > How about `multiple declarations within a single statement hurts 
> > > > > > readability`?
> > > > > s/hurts/reduces/? hurts sound a bit weird i think.
> > > > > 
> > > > > Lebedev wanted the number of decls in the diagnostic, would you 
> > > > > include it or rather now?
> > > > "decreases" is also fine. "hurts" is probably too strong, I agree.
> > > > 
> > > > Up to you. Personally, I don't see any value in having the diagnostic 
> > > > message saying "hey, you have 2 declarations within one statement, 
> > > > that's really bad!" or "hey, you have 5 declarations within one 
> > > > statement..." - in both cases the point is that there are *multiple* 
> > > > declarations. I also don't think it would make debugging easier because 
> > > > you also check the formatting, so you already imply that the correct 
> > > > number of declarations was detected.
> > > > 
> > > > I'm interested to know what @lebedev.ri thinks.
> > > > I'm interested to know what @lebedev.ri thinks.
> > > 
> > > "This translation unit has an error. Can not continue" is also a 
> > > diagnostic message.
> > > Why are we not ok with that one, and want compiler to be a bit more 
> > > specific?
> > > 
> > > Similarly here, why just point out that this code is bad as per the check,
> > > without giving a little bit more info, that you already have?
> > > "This translation unit has an error. Can not continue" is also a 
> > > diagnostic message.
> > >Why are we not ok with that one, and want compiler to be a bit more 
> > >specific?
> > >
> > > Similarly here, why just point out that this code is bad as per the 
> > > check, without giving a little bit more info, that you already have?
> > 
> > More information doesn't always equate into more understanding, especially 
> > when that information causes a distraction. For instance, you could argue 
> > that the type of the declared variables is also information we already 
> > have, but what purpose would it serve to tell it to the user?
> > 
> > Can you give an example where the specific number of declarations involved 
> > would help you to correct the diagnostic? I can't come up with one, so it 
> > feels to me like having the count is more of a distraction; especially 
> > given that there's no configurable threshold for "now you have too many 
> > declarations". I'd feel differently if there was a config option, because 
> > then the count is truly useful to know.
> Oh, but that's different: "This translation unit has an error. Can not 
> continue" does not provide enough information for users to fix the issue, 
> pointing out that there are *multiple* declarations per statement is 
> definitely enough.
I am personally against having the number in the diagnostic as well, it would 
only add value if the declarations are expanded from a macro.

@aaron.ballman Configuration of this check would be intersting but i would 
rather postpone that and have a basic working check first. Given that this aims 
to be utility-like to evaluate `const-correctness` and/or to be usable with 
other checks doing type transformations.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

There are testcases with macro around line 100 in the default tests. I
am not sure yet if `#define I_DECLS int i1, i2, i3;` should be
diagnosed, but the other cases should.

Am 27.09.2018 um 16:28 schrieb Kirill Bobyrev via Phabricator:

> kbobyrev added a comment.
> 
> Also, regarding error handling and `llvm::Option` vs `llvm::Expected`: I 
> think the case where the check most likely wouldn't be able to provide useful 
> diagnostics and perform enough analysis is when there are macro expansions 
> within inspected statement `SourceRange`. It might make sense to completely 
> disable it in this case both because it's hard to do anything about the range 
> transformation and because it seems to be hard to analyze this case. E.g. if 
> the whole statement is expanded from a single macro and then the check would 
> report on every macro usage (while the actual problem is in the macro 
> itself). I don't know whether the check should support macros at all, it 
> might make sense to mention this in the documentation and add few tests if we 
> decide to go this way.
> 
> Repository:
> 
>   rCTE Clang Tools Extra
> 
> https://reviews.llvm.org/D51949


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:343
+  auto Diag =
+  diag(WholeDecl->getBeginLoc(), "this statement declares %0 variables")
+  << static_cast(

aaron.ballman wrote:
> lebedev.ri wrote:
> > kbobyrev wrote:
> > > JonasToth wrote:
> > > > kbobyrev wrote:
> > > > > How about `multiple declarations within a single statement hurts 
> > > > > readability`?
> > > > s/hurts/reduces/? hurts sound a bit weird i think.
> > > > 
> > > > Lebedev wanted the number of decls in the diagnostic, would you include 
> > > > it or rather now?
> > > "decreases" is also fine. "hurts" is probably too strong, I agree.
> > > 
> > > Up to you. Personally, I don't see any value in having the diagnostic 
> > > message saying "hey, you have 2 declarations within one statement, that's 
> > > really bad!" or "hey, you have 5 declarations within one statement..." - 
> > > in both cases the point is that there are *multiple* declarations. I also 
> > > don't think it would make debugging easier because you also check the 
> > > formatting, so you already imply that the correct number of declarations 
> > > was detected.
> > > 
> > > I'm interested to know what @lebedev.ri thinks.
> > > I'm interested to know what @lebedev.ri thinks.
> > 
> > "This translation unit has an error. Can not continue" is also a diagnostic 
> > message.
> > Why are we not ok with that one, and want compiler to be a bit more 
> > specific?
> > 
> > Similarly here, why just point out that this code is bad as per the check,
> > without giving a little bit more info, that you already have?
> > "This translation unit has an error. Can not continue" is also a diagnostic 
> > message.
> >Why are we not ok with that one, and want compiler to be a bit more specific?
> >
> > Similarly here, why just point out that this code is bad as per the check, 
> > without giving a little bit more info, that you already have?
> 
> More information doesn't always equate into more understanding, especially 
> when that information causes a distraction. For instance, you could argue 
> that the type of the declared variables is also information we already have, 
> but what purpose would it serve to tell it to the user?
> 
> Can you give an example where the specific number of declarations involved 
> would help you to correct the diagnostic? I can't come up with one, so it 
> feels to me like having the count is more of a distraction; especially given 
> that there's no configurable threshold for "now you have too many 
> declarations". I'd feel differently if there was a config option, because 
> then the count is truly useful to know.
Oh, but that's different: "This translation unit has an error. Can not 
continue" does not provide enough information for users to fix the issue, 
pointing out that there are *multiple* declarations per statement is definitely 
enough.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:343
+  auto Diag =
+  diag(WholeDecl->getBeginLoc(), "this statement declares %0 variables")
+  << static_cast(

lebedev.ri wrote:
> kbobyrev wrote:
> > JonasToth wrote:
> > > kbobyrev wrote:
> > > > How about `multiple declarations within a single statement hurts 
> > > > readability`?
> > > s/hurts/reduces/? hurts sound a bit weird i think.
> > > 
> > > Lebedev wanted the number of decls in the diagnostic, would you include 
> > > it or rather now?
> > "decreases" is also fine. "hurts" is probably too strong, I agree.
> > 
> > Up to you. Personally, I don't see any value in having the diagnostic 
> > message saying "hey, you have 2 declarations within one statement, that's 
> > really bad!" or "hey, you have 5 declarations within one statement..." - in 
> > both cases the point is that there are *multiple* declarations. I also 
> > don't think it would make debugging easier because you also check the 
> > formatting, so you already imply that the correct number of declarations 
> > was detected.
> > 
> > I'm interested to know what @lebedev.ri thinks.
> > I'm interested to know what @lebedev.ri thinks.
> 
> "This translation unit has an error. Can not continue" is also a diagnostic 
> message.
> Why are we not ok with that one, and want compiler to be a bit more specific?
> 
> Similarly here, why just point out that this code is bad as per the check,
> without giving a little bit more info, that you already have?
> "This translation unit has an error. Can not continue" is also a diagnostic 
> message.
>Why are we not ok with that one, and want compiler to be a bit more specific?
>
> Similarly here, why just point out that this code is bad as per the check, 
> without giving a little bit more info, that you already have?

More information doesn't always equate into more understanding, especially when 
that information causes a distraction. For instance, you could argue that the 
type of the declared variables is also information we already have, but what 
purpose would it serve to tell it to the user?

Can you give an example where the specific number of declarations involved 
would help you to correct the diagnostic? I can't come up with one, so it feels 
to me like having the count is more of a distraction; especially given that 
there's no configurable threshold for "now you have too many declarations". I'd 
feel differently if there was a config option, because then the count is truly 
useful to know.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:343
+  auto Diag =
+  diag(WholeDecl->getBeginLoc(), "this statement declares %0 variables")
+  << static_cast(

kbobyrev wrote:
> JonasToth wrote:
> > kbobyrev wrote:
> > > How about `multiple declarations within a single statement hurts 
> > > readability`?
> > s/hurts/reduces/? hurts sound a bit weird i think.
> > 
> > Lebedev wanted the number of decls in the diagnostic, would you include it 
> > or rather now?
> "decreases" is also fine. "hurts" is probably too strong, I agree.
> 
> Up to you. Personally, I don't see any value in having the diagnostic message 
> saying "hey, you have 2 declarations within one statement, that's really 
> bad!" or "hey, you have 5 declarations within one statement..." - in both 
> cases the point is that there are *multiple* declarations. I also don't think 
> it would make debugging easier because you also check the formatting, so you 
> already imply that the correct number of declarations was detected.
> 
> I'm interested to know what @lebedev.ri thinks.
> I'm interested to know what @lebedev.ri thinks.

"This translation unit has an error. Can not continue" is also a diagnostic 
message.
Why are we not ok with that one, and want compiler to be a bit more specific?

Similarly here, why just point out that this code is bad as per the check,
without giving a little bit more info, that you already have?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

Also, regarding error handling and `llvm::Option` vs `llvm::Expected`: I think 
the case where the check most likely wouldn't be able to provide useful 
diagnostics and perform enough analysis is when there are macro expansions 
within inspected statement `SourceRange`. It might make sense to completely 
disable it in this case both because it's hard to do anything about the range 
transformation and because it seems to be hard to analyze this case. E.g. if 
the whole statement is expanded from a single macro and then the check would 
report on every macro usage (while the actual problem is in the macro itself). 
I don't know whether the check should support macros at all, it might make 
sense to mention this in the documentation and add few tests if we decide to go 
this way.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

A lot of good improvements and many test cases, thank you!

The comments are mostly nits.




Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:343
+  auto Diag =
+  diag(WholeDecl->getBeginLoc(), "this statement declares %0 variables")
+  << static_cast(

JonasToth wrote:
> kbobyrev wrote:
> > How about `multiple declarations within a single statement hurts 
> > readability`?
> s/hurts/reduces/? hurts sound a bit weird i think.
> 
> Lebedev wanted the number of decls in the diagnostic, would you include it or 
> rather now?
"decreases" is also fine. "hurts" is probably too strong, I agree.

Up to you. Personally, I don't see any value in having the diagnostic message 
saying "hey, you have 2 declarations within one statement, that's really bad!" 
or "hey, you have 5 declarations within one statement..." - in both cases the 
point is that there are *multiple* declarations. I also don't think it would 
make debugging easier because you also check the formatting, so you already 
imply that the correct number of declarations was detected.

I'm interested to know what @lebedev.ri thinks.



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:39
+  const Stmt *const InitStmt = Node.getInit();
+  if (InitStmt)
+return InnerMatcher.matches(*InitStmt, Finder, Builder);

nit: `return InitStmt ? InnerMatcher... : false;`



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:65
+
+  for (int i = Indirections; i > 0; --i) {
+Start = findPreviousAnyTokenKind(Start, SM, LangOpts, tok::star, tok::amp);

`i`-> `I`

or even better `while (--Indirections)` since it's not used anywhere afterwards.



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:68
+if (Start.isInvalid() || Start.isMacroID())
+  return SourceLocation();
+  }

Also, I don't think this should happen with the correct behavior. 
`llvm::Expected` looks like a better alternative for error handling here.



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:78
+static int countIndirections(const Type *T, int Indirections = 0) {
+  assert(T && "Require non-null");
+  if (T->isPointerType() || T->isReferenceType())

nit: "Type has to be set for *countIndirections()*"? The wording is not 
optimal, but currently the assertion message doesn't say much about the error.



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:93
+static bool IsMemberPointer(const VarDecl *D) {
+  if (D->getType()->isMemberPointerType() ||
+  D->getType()->isMemberDataPointerType() ||

just `return (Condition0 || Condition1 || Condition2);`



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:102
+static Optional>
+DeclSlice(const DeclStmt *DS, const SourceManager ,
+  const LangOptions ) {

nit: naming (lowercase + more descriptive name/documentation about the returned 
value).



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:166
+VarDecl *CurrentDecl = dyn_cast(*It);
+assert(CurrentDecl && "Expect only VarDecls here");
+

This assertion message doesn't really give much information, too. Before you 
inspect the code closely (e.g. to understand what "here" refers to), it would 
be hard for other developers to understand what the issue might just by looking 
at the failed assertion message unless they are very familiar with the code.



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:168
+
+// FIXME Memberpointer are not transformed correctly right now, thats
+// why they are treated as problematic here.

nit `FIXME:`. Also, ideally there would be an XFAIL test on that.



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:173
+
+SourceLocation DeclEnd;
+if (CurrentDecl->hasInit())

nit: `const SourceLocation DeclEnd = findNextTerimator(CurrentDecl->hasInit() ? 
CurrentDecl->getINit()->getEndLoc() : CurrentDecl->getEndLoc(), SM, LagOpts);` 
(formatted with Clang-Format, of course, my snippet is messy :)

Up to you, though, I just think it would be slightly more readable.



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:208
+static Optional>
+SourceFromRanges(const std::vector ,
+ const SourceManager , const LangOptions ) {

nit: `const std::vector &` -> `llvm::ArrayRef`

Also, probably `SourceFromRanges` -> `collectSourceText(Ranges, ...)`? This 
would probably be easier to read (otherwise it's easier to confuse with 
something like "get `SourceLocations` from `SourceRanges` when not looking at 
the function body).



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:211
+  std::vector Snippets;
+  Snippets.reserve(Ranges.size());
+

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 167282.
JonasToth marked 3 inline comments as done.
JonasToth added a comment.

- add space after clang-tidy in banner
- refactor slightly, add better in-code comments to explain whats happening, 
add more TODOs and FIXMEs


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/IsolateDeclCheck.cpp
  clang-tidy/readability/IsolateDeclCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/utils/LexerUtils.cpp
  clang-tidy/utils/LexerUtils.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-isolate-decl.rst
  test/clang-tidy/readability-isolate-decl-cxx17.cpp
  test/clang-tidy/readability-isolate-decl.cpp

Index: test/clang-tidy/readability-isolate-decl.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-isolate-decl.cpp
@@ -0,0 +1,356 @@
+// RUN: %check_clang_tidy %s readability-isolate-decl %t
+
+void f() {
+  int i;
+}
+
+void f2() {
+  int i, j, *k, lala = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int j;
+  // CHECK-FIXES: {{^  }}int *k;
+  // CHECK-FIXES: {{^  }}int lala = 42;
+
+  int normal, weird = /* comment */ 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int normal;
+  // CHECK-FIXES: {{^  }}int weird = /* comment */ 42;
+  //
+  int /* here is a comment */ v1,
+  // another comment
+  v2 = 42 // Ok, more comments
+  ;
+  // CHECK-MESSAGES: [[@LINE-4]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int /* here is a comment */ v1;
+  // CHECK-FIXES: {{^  }}int /* here is a comment */ // another comment
+  // CHECK-FIXES: {{^  }}v2 = 42 // Ok, more comments
+  // CHECK-FIXES: {{^  }};
+}
+
+void f3() {
+  int i, *pointer1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int *pointer1;
+  //
+  int *pointer2 = nullptr, *pointer3 = 
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int *pointer2 = nullptr;
+  // CHECK-FIXES: {{^  }}int *pointer3 = 
+}
+
+void f4() {
+  double d = 42. /* foo */, z = 43., /* hi */ y, c /* */ /*  */, l = 2.;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 5 variables
+  // CHECK-FIXES: double d = 42. /* foo */;
+  // CHECK-FIXES: {{^  }}double z = 43.;
+  // CHECK-FIXES: {{^  }}double /* hi */ y;
+  // CHECK-FIXES: {{^  }}double c /* */ /*  */;
+  // CHECK-FIXES: {{^  }}double l = 2.;
+}
+
+struct SomeClass {
+  SomeClass() = default;
+  SomeClass(int value);
+};
+void f5() {
+  SomeClass v1, v2(42), v3{42}, v4(42.5);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // CHECK-FIXES: SomeClass v1;
+  // CHECK-FIXES: {{^  }}SomeClass v2(42);
+  // CHECK-FIXES: {{^  }}SomeClass v3{42};
+  // CHECK-FIXES: {{^  }}SomeClass v4(42.5);
+
+  SomeClass v5 = 42, *p1 = nullptr;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: SomeClass v5 = 42;
+  // CHECK-FIXES: {{^  }}SomeClass *p1 = nullptr;
+}
+
+void f6() {
+  int array1[] = {1, 2, 3, 4}, array2[] = {1, 2, 3}, value1, value2 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // CHECK-FIXES: int array1[] = {1, 2, 3, 4};
+  // CHECK-FIXES: {{^  }}int array2[] = {1, 2, 3};
+  // CHECK-FIXES: {{^  }}int value1;
+  // CHECK-FIXES: {{^  }}int value2 = 42;
+}
+
+template 
+struct TemplatedType {
+  TemplatedType() = default;
+  TemplatedType(T value);
+};
+
+void f7() {
+  TemplatedType TT1(42), TT2{42}, TT3;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 3 variables
+  // CHECK-FIXES: TemplatedType TT1(42);
+  // CHECK-FIXES: {{^  }}TemplatedType TT2{42};
+  // CHECK-FIXES: {{^  }}TemplatedType TT3;
+  //
+  TemplatedType *TT4(nullptr), TT5, **TT6 = nullptr, *const *const TT7{nullptr};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // CHECK-FIXES: TemplatedType *TT4(nullptr);
+  // CHECK-FIXES: {{^  }}TemplatedType TT5;
+  // CHECK-FIXES: {{^  }}TemplatedType **TT6 = nullptr;
+  // CHECK-FIXES: {{^  }}TemplatedType *const *const TT7{nullptr};
+
+  TemplatedType **TT8(nullptr), *TT9;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: TemplatedType **T8(nullptr);
+  // CHECK-FIXES: {{^  }}TemplatedType *TT9;
+}
+
+void forbidden_transformations() {
+  for (int i = 0, j = 42; i < j; ++i)
+;
+}
+
+#define NULL 0
+#define MY_NICE_TYPE int **
+#define VAR_NAME(name) name##__LINE__
+#define A_BUNCH_OF_VARIABLES int m1 = 42, m2 = 43, m3 = 44;
+
+void macros() {
+  int *p1 = NULL, *p2 = NULL;
+  // 

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 167270.
JonasToth marked 6 inline comments as done.
JonasToth added a comment.

- move lexer stuff into LexerUtils
- remove custom string handling function
- address review comments


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/IsolateDeclCheck.cpp
  clang-tidy/readability/IsolateDeclCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/utils/LexerUtils.cpp
  clang-tidy/utils/LexerUtils.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-isolate-decl.rst
  test/clang-tidy/readability-isolate-decl-cxx17.cpp
  test/clang-tidy/readability-isolate-decl.cpp

Index: test/clang-tidy/readability-isolate-decl.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-isolate-decl.cpp
@@ -0,0 +1,354 @@
+// RUN: %check_clang_tidy %s readability-isolate-decl %t
+
+void f() {
+  int i;
+}
+
+void f2() {
+  int i, j, *k, lala = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int j;
+  // CHECK-FIXES: {{^  }}int *k;
+  // CHECK-FIXES: {{^  }}int lala = 42;
+
+  int normal, weird = /* comment */ 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int normal;
+  // CHECK-FIXES: {{^  }}int weird = /* comment */ 42;
+  //
+  int /* here is a comment */ v1,
+  // another comment
+  v2 = 42 // Ok, more comments
+  ;
+  // CHECK-MESSAGES: [[@LINE-4]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int /* here is a comment */ v1;
+  // CHECK-FIXES: {{^  }}int /* here is a comment */ // another comment
+  // CHECK-FIXES: {{^  }}v2 = 42 // Ok, more comments
+  // CHECK-FIXES: {{^  }};
+}
+
+void f3() {
+  int i, *pointer1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int *pointer1;
+  //
+  int *pointer2 = nullptr, *pointer3 = 
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int *pointer2 = nullptr;
+  // CHECK-FIXES: {{^  }}int *pointer3 = 
+}
+
+void f4() {
+  double d = 42. /* foo */, z = 43., /* hi */ y, c /* */ /*  */, l = 2.;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 5 variables
+  // CHECK-FIXES: double d = 42. /* foo */;
+  // CHECK-FIXES: {{^  }}double z = 43.;
+  // CHECK-FIXES: {{^  }}double /* hi */ y;
+  // CHECK-FIXES: {{^  }}double c /* */ /*  */;
+  // CHECK-FIXES: {{^  }}double l = 2.;
+}
+
+struct SomeClass {
+  SomeClass() = default;
+  SomeClass(int value);
+};
+void f5() {
+  SomeClass v1, v2(42), v3{42}, v4(42.5);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // CHECK-FIXES: SomeClass v1;
+  // CHECK-FIXES: {{^  }}SomeClass v2(42);
+  // CHECK-FIXES: {{^  }}SomeClass v3{42};
+  // CHECK-FIXES: {{^  }}SomeClass v4(42.5);
+
+  SomeClass v5 = 42, *p1 = nullptr;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: SomeClass v5 = 42;
+  // CHECK-FIXES: {{^  }}SomeClass *p1 = nullptr;
+}
+
+void f6() {
+  int array1[] = {1, 2, 3, 4}, array2[] = {1, 2, 3}, value1, value2 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // CHECK-FIXES: int array1[] = {1, 2, 3, 4};
+  // CHECK-FIXES: {{^  }}int array2[] = {1, 2, 3};
+  // CHECK-FIXES: {{^  }}int value1;
+  // CHECK-FIXES: {{^  }}int value2 = 42;
+}
+
+template 
+struct TemplatedType {
+  TemplatedType() = default;
+  TemplatedType(T value);
+};
+
+void f7() {
+  TemplatedType TT1(42), TT2{42}, TT3;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 3 variables
+  // CHECK-FIXES: TemplatedType TT1(42);
+  // CHECK-FIXES: {{^  }}TemplatedType TT2{42};
+  // CHECK-FIXES: {{^  }}TemplatedType TT3;
+  //
+  TemplatedType *TT4(nullptr), TT5, **TT6 = nullptr, *const *const TT7{nullptr};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // TODO Fix this
+  // CHECK FIXES: TemplatedType *TT4(nullptr);
+  // CHECK FIXES: {{^  }}TemplatedType TT5;
+  // CHECK FIXES: {{^  }}TemplatedType **TT6 = nullptr;
+  // CHECK FIXES: {{^  }}TemplatedType *const *const TT7{nullptr};
+
+  TemplatedType **TT8(nullptr), TT9;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // TODO Fix this
+}
+
+void forbidden_transformations() {
+  for (int i = 0, j = 42; i < j; ++i)
+;
+}
+
+#define NULL 0
+#define MY_NICE_TYPE int **
+#define VAR_NAME(name) name##__LINE__
+#define A_BUNCH_OF_VARIABLES int m1 = 42, m2 = 43, m3 = 44;
+
+void macros() {
+  int *p1 = NULL, *p2 = NULL;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int *p1 

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:21
+
+#define PRINT_DEBUG 1
+

kbobyrev wrote:
> Do you plan to submit the patch with debugging messages or are you just using 
> these for better local debugging experience?
> 
> If you plan to upload the patch with the messages, please use `LLVM_DEBUG` 
> (see `readability/IdentifierNamingCheck.cpp` for reference) and 
> `llvm::dbgs()` ([[ 
> https://llvm.org/docs/CodingStandards.html#include-iostream-is-forbidden | 
> `` traits shouldn't be used ]] and should be replaced with their 
> LLVM counterparts: `llvm::outs()`, `llvm::errs()`, etc). Also, I believe the 
> messages should be more informative if you think debug info is really 
> important here.
No, they will be removed, just for the current prototyping. using `llvm::outs` 
and `llvm::errs` would have been better though ;)



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:323
+
+static std::string (std::string ) {
+  auto it1 = std::find_if(str.rbegin(), str.rend(), [](char ch) {

kbobyrev wrote:
> `llvm::StringRef::rtrim()`?
> 
> Also, naming `str` should be at least `Str`, `it1` -> `It`, `ch` -> `Ch` (or 
> better `Symbol`) ...
`rtrim()` is a better fit, but introduces another copy. Do you think I should 
rater use `llvm::SmallString`?



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:331
+
+static std::string Concat(const std::vector ,
+  StringRef Indent) {

kbobyrev wrote:
> Use `llvm::join` with `";\n" + Indent` as the `Separator`?
Yup, didn't know that one yet, thanks :)



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:343
+  auto Diag =
+  diag(WholeDecl->getBeginLoc(), "this statement declares %0 variables")
+  << static_cast(

kbobyrev wrote:
> How about `multiple declarations within a single statement hurts readability`?
s/hurts/reduces/? hurts sound a bit weird i think.

Lebedev wanted the number of decls in the diagnostic, would you include it or 
rather now?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

Sorry, I didn't get time to review the patch properly, these are few stylistic 
comments. Hopefully, I'll be able to give more feedback when I get more time.




Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:21
+
+#define PRINT_DEBUG 1
+

Do you plan to submit the patch with debugging messages or are you just using 
these for better local debugging experience?

If you plan to upload the patch with the messages, please use `LLVM_DEBUG` (see 
`readability/IdentifierNamingCheck.cpp` for reference) and `llvm::dbgs()` ([[ 
https://llvm.org/docs/CodingStandards.html#include-iostream-is-forbidden | 
`` traits shouldn't be used ]] and should be replaced with their LLVM 
counterparts: `llvm::outs()`, `llvm::errs()`, etc). Also, I believe the 
messages should be more informative if you think debug info is really important 
here.



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:63
+
+if (!CurrentToken.hasValue())
+  return SourceLocation();

nit: `if (!CurrentToken)`



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:323
+
+static std::string (std::string ) {
+  auto it1 = std::find_if(str.rbegin(), str.rend(), [](char ch) {

`llvm::StringRef::rtrim()`?

Also, naming `str` should be at least `Str`, `it1` -> `It`, `ch` -> `Ch` (or 
better `Symbol`) ...



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:331
+
+static std::string Concat(const std::vector ,
+  StringRef Indent) {

Use `llvm::join` with `";\n" + Indent` as the `Separator`?



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:333
+  StringRef Indent) {
+  std::string R;
+  for (const StringRef  : Decls)

`Range`?



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:343
+  auto Diag =
+  diag(WholeDecl->getBeginLoc(), "this statement declares %0 variables")
+  << static_cast(

How about `multiple declarations within a single statement hurts readability`?



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:51
+
+return TypeAndName + " = " + Initializer + ";";
+  }

JonasToth wrote:
> kbobyrev wrote:
> > JonasToth wrote:
> > > kbobyrev wrote:
> > > > This seems to replace `int x = 5, y = 42;` with `int x = 5;int y = 42`. 
> > > > I don't think that it becomes cleaner (in fact, without spaces in 
> > > > between it looks cryptic). Consider formatting it or simply applying 
> > > > newlines (if there were no newlines inbetween before).
> > > I do not plan to do a lot of formatting here (maybe space or newline), 
> > > because that clang-format area.
> > While Clang-Tidy can apply Clang-Format on top of the Fix-Its, it will 
> > still look weird in the Fix-Its previews. While supporting proper 
> > formatting, in general, might be hard, it totally makes sense to do some 
> > basic formatting so that editor integration warnings would look better, for 
> > example.
> The current version adds a new line for each decl and keeps the indendation 
> (as the other check does).
> 
> Because it does the slicing on commas the manual/custom formatting of the 
> original code will stay. That might result in weird looking output for exotic 
> variable declarations. I would like to ignore these cases, what do you think 
> @kbobyrev ?
Yes, sure, it's hard (and probably impossible) to support the generic case. 
This approach sounds good to me, thanks!



Comment at: clang-tidy/readability/IsolateDeclCheck.h:1
+//===--- IsolateDeclCheck.h - clang-tidy-*- C++ 
-*-===//
+//

JonasToth wrote:
> kbobyrev wrote:
> > nit: space between clang-tidy (also, in .cpp file)
> That comes from the template `add_new_check.py`, do you want me to fix it 
> there?
Ah, okay, I was thinking whether it was a template issue (since I thought it's 
unlikely that one would edit the header template). Yes, it looks like a typo in 
the `clang-tidy` check adding tool implementation. It seems to be fixed in 
rL342601.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 166044.
JonasToth added a comment.

- minor adjustments, add fixmes


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/IsolateDeclCheck.cpp
  clang-tidy/readability/IsolateDeclCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-isolate-decl.rst
  test/clang-tidy/readability-isolate-decl-cxx17.cpp
  test/clang-tidy/readability-isolate-decl.cpp

Index: test/clang-tidy/readability-isolate-decl.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-isolate-decl.cpp
@@ -0,0 +1,354 @@
+// RUN: %check_clang_tidy %s readability-isolate-decl %t
+
+void f() {
+  int i;
+}
+
+void f2() {
+  int i, j, *k, lala = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int j;
+  // CHECK-FIXES: {{^  }}int *k;
+  // CHECK-FIXES: {{^  }}int lala = 42;
+
+  int normal, weird = /* comment */ 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int normal;
+  // CHECK-FIXES: {{^  }}int weird = /* comment */ 42;
+  //
+  int /* here is a comment */ v1,
+  // another comment
+  v2 = 42 // Ok, more comments
+  ;
+  // CHECK-MESSAGES: [[@LINE-4]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int /* here is a comment */ v1;
+  // CHECK-FIXES: {{^  }}int /* here is a comment */ // another comment
+  // CHECK-FIXES: {{^  }}v2 = 42 // Ok, more comments
+  // CHECK-FIXES: {{^  }};
+}
+
+void f3() {
+  int i, *pointer1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int *pointer1;
+  //
+  int *pointer2 = nullptr, *pointer3 = 
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int *pointer2 = nullptr;
+  // CHECK-FIXES: {{^  }}int *pointer3 = 
+}
+
+void f4() {
+  double d = 42. /* foo */, z = 43., /* hi */ y, c /* */ /*  */, l = 2.;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 5 variables
+  // CHECK-FIXES: double d = 42. /* foo */;
+  // CHECK-FIXES: {{^  }}double z = 43.;
+  // CHECK-FIXES: {{^  }}double /* hi */ y;
+  // CHECK-FIXES: {{^  }}double c /* */ /*  */;
+  // CHECK-FIXES: {{^  }}double l = 2.;
+}
+
+struct SomeClass {
+  SomeClass() = default;
+  SomeClass(int value);
+};
+void f5() {
+  SomeClass v1, v2(42), v3{42}, v4(42.5);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // CHECK-FIXES: SomeClass v1;
+  // CHECK-FIXES: {{^  }}SomeClass v2(42);
+  // CHECK-FIXES: {{^  }}SomeClass v3{42};
+  // CHECK-FIXES: {{^  }}SomeClass v4(42.5);
+
+  SomeClass v5 = 42, *p1 = nullptr;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: SomeClass v5 = 42;
+  // CHECK-FIXES: {{^  }}SomeClass *p1 = nullptr;
+}
+
+void f6() {
+  int array1[] = {1, 2, 3, 4}, array2[] = {1, 2, 3}, value1, value2 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // CHECK-FIXES: int array1[] = {1, 2, 3, 4};
+  // CHECK-FIXES: {{^  }}int array2[] = {1, 2, 3};
+  // CHECK-FIXES: {{^  }}int value1;
+  // CHECK-FIXES: {{^  }}int value2 = 42;
+}
+
+template 
+struct TemplatedType {
+  TemplatedType() = default;
+  TemplatedType(T value);
+};
+
+void f7() {
+  TemplatedType TT1(42), TT2{42}, TT3;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 3 variables
+  // CHECK-FIXES: TemplatedType TT1(42);
+  // CHECK-FIXES: {{^  }}TemplatedType TT2{42};
+  // CHECK-FIXES: {{^  }}TemplatedType TT3;
+  //
+  TemplatedType *TT4(nullptr), TT5, **TT6 = nullptr, *const *const TT7{nullptr};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // TODO Fix this
+  // CHECK FIXES: TemplatedType *TT4(nullptr);
+  // CHECK FIXES: {{^  }}TemplatedType TT5;
+  // CHECK FIXES: {{^  }}TemplatedType **TT6 = nullptr;
+  // CHECK FIXES: {{^  }}TemplatedType *const *const TT7{nullptr};
+
+  TemplatedType **TT8(nullptr), TT9;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // TODO Fix this
+}
+
+void forbidden_transformations() {
+  for (int i = 0, j = 42; i < j; ++i)
+;
+}
+
+#define NULL 0
+#define MY_NICE_TYPE int **
+#define VAR_NAME(name) name##__LINE__
+#define A_BUNCH_OF_VARIABLES int m1 = 42, m2 = 43, m3 = 44;
+
+void macros() {
+  int *p1 = NULL, *p2 = NULL;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int *p1 = NULL;
+  // CHECK-FIXES: {{^  }}int *p2 = NULL;
+
+  // Macros are involved, so there will be no transformation
+  MY_NICE_TYPE p3, v1, v2;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: 

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 166041.
JonasToth added a comment.

- Merge branch 'master' into experiment_isolate_decl
- further work on isolate decl, fix tokenizing bug with templates
- include big test-suite and make them run


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/IsolateDeclCheck.cpp
  clang-tidy/readability/IsolateDeclCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-isolate-decl.rst
  test/clang-tidy/readability-isolate-decl-cxx17.cpp
  test/clang-tidy/readability-isolate-decl.cpp

Index: test/clang-tidy/readability-isolate-decl.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-isolate-decl.cpp
@@ -0,0 +1,365 @@
+// RUN: %check_clang_tidy %s readability-isolate-decl %t
+
+void f() {
+  int i;
+}
+
+void f2() {
+  int i, j, *k, lala = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int j;
+  // CHECK-FIXES: {{^  }}int *k;
+  // CHECK-FIXES: {{^  }}int lala = 42;
+
+  int normal, weird = /* comment */ 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int normal;
+  // CHECK-FIXES: {{^  }}int weird = /* comment */ 42;
+  //
+  int /* here is a comment */ v1,
+  // another comment
+  v2 = 42 // Ok, more comments
+  ;
+  // CHECK-MESSAGES: [[@LINE-4]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int /* here is a comment */ v1;
+  // CHECK-FIXES: {{^  }}int /* here is a comment */ // another comment
+  // CHECK-FIXES: {{^  }}v2 = 42 // Ok, more comments
+  // CHECK-FIXES: {{^  }};
+}
+
+void f3() {
+  int i, *pointer1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int *pointer1;
+  //
+  int *pointer2 = nullptr, *pointer3 = 
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int *pointer2 = nullptr;
+  // CHECK-FIXES: {{^  }}int *pointer3 = 
+}
+
+void f4() {
+  double d = 42. /* foo */, z = 43., /* hi */ y, c /* */ /*  */, l = 2.;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 5 variables
+  // CHECK-FIXES: double d = 42. /* foo */;
+  // CHECK-FIXES: {{^  }}double z = 43.;
+  // CHECK-FIXES: {{^  }}double /* hi */ y;
+  // CHECK-FIXES: {{^  }}double c /* */ /*  */;
+  // CHECK-FIXES: {{^  }}double l = 2.;
+}
+
+struct SomeClass {
+  SomeClass() = default;
+  SomeClass(int value);
+};
+void f5() {
+  SomeClass v1, v2(42), v3{42}, v4(42.5);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // CHECK-FIXES: SomeClass v1;
+  // CHECK-FIXES: {{^  }}SomeClass v2(42);
+  // CHECK-FIXES: {{^  }}SomeClass v3{42};
+  // CHECK-FIXES: {{^  }}SomeClass v4(42.5);
+
+  SomeClass v5 = 42, *p1 = nullptr;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: SomeClass v5 = 42;
+  // CHECK-FIXES: {{^  }}SomeClass *p1 = nullptr;
+}
+
+void f6() {
+  int array1[] = {1, 2, 3, 4}, array2[] = {1, 2, 3}, value1, value2 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // CHECK-FIXES: int array1[] = {1, 2, 3, 4};
+  // CHECK-FIXES: {{^  }}int array2[] = {1, 2, 3};
+  // CHECK-FIXES: {{^  }}int value1;
+  // CHECK-FIXES: {{^  }}int value2 = 42;
+}
+
+template 
+struct TemplatedType {
+  TemplatedType() = default;
+  TemplatedType(T value);
+};
+
+void f7() {
+  TemplatedType TT1(42), TT2{42}, TT3;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 3 variables
+  // CHECK-FIXES: TemplatedType TT1(42);
+  // CHECK-FIXES: {{^  }}TemplatedType TT2{42};
+  // CHECK-FIXES: {{^  }}TemplatedType TT3;
+  //
+  TemplatedType *TT4(nullptr), TT5, **TT6 = nullptr, *const *const TT7{nullptr};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // CHECK FIXES: TemplatedType *TT4(nullptr);
+  // CHECK FIXES: {{^  }}TemplatedType TT5;
+  // CHECK FIXES: {{^  }}TemplatedType **TT6 = nullptr;
+  // CHECK FIXES: {{^  }}TemplatedType *const *const TT7{nullptr};
+  //
+  TemplatedType **TT8(nullptr), TT9;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+}
+
+void forbidden_transformations() {
+  for (int i = 0, j = 42; i < j; ++i)
+;
+}
+
+#define NULL 0
+#define MY_NICE_TYPE int **
+#define VAR_NAME(name) name##__LINE__
+#define A_BUNCH_OF_VARIABLES int m1 = 42, m2 = 43, m3 = 44;
+
+void macros() {
+  int *p1 = NULL, *p2 = NULL;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int *p1 = NULL;
+  // CHECK-FIXES: {{^  }}int *p2 = NULL;
+
+  // Macros are involved, so 

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 165752.
JonasToth marked an inline comment as done.
JonasToth added a comment.

- use static functions instead anonymous namespace


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/IsolateDeclCheck.cpp
  clang-tidy/readability/IsolateDeclCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-isolate-decl.rst
  test/clang-tidy/readability-isolate-decl-cxx17.cpp
  test/clang-tidy/readability-isolate-decl.cpp

Index: test/clang-tidy/readability-isolate-decl.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-isolate-decl.cpp
@@ -0,0 +1,121 @@
+// RUN: %check_clang_tidy %s readability-isolate-decl %t
+
+void f() {
+  int i;
+}
+
+void f2() {
+  int i, j, *k, lala = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int j;
+  // CHECK-FIXES: {{^  }}int *k;
+  // CHECK-FIXES: {{^  }}int lala = 42;
+
+  int normal, weird = /* comment */ 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int normal;
+  // CHECK-FIXES: {{^  }}int weird = /* comment */ 42;
+  //
+  int /* here is a comment */ v1,
+  // another comment
+  v2 = 42 // Ok, more comments
+  ;
+  // CHECK-MESSAGES: [[@LINE-4]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int /* here is a comment */ v1;
+  // CHECK-FIXES: {{^  }}int /* here is a comment */ // another comment
+  // CHECK-FIXES: {{^  }}v2 = 42 // Ok, more comments
+  // CHECK-FIXES: {{^  }};
+}
+
+void f3() {
+  int i, *pointer1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int *pointer1;
+  //
+  int *pointer2 = nullptr, *pointer3 = 
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int *pointer2 = nullptr;
+  // CHECK-FIXES: {{^  }}int *pointer3 = 
+}
+
+void f4() {
+  double d = 42. /* foo */, z = 43., /* hi */ y, c /* */ /*  */, l = 2.;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 5 variables
+  // CHECK-FIXES: double d = 42. /* foo */;
+  // CHECK-FIXES: {{^  }}double z = 43.;
+  // CHECK-FIXES: {{^  }}double /* hi */ y;
+  // CHECK-FIXES: {{^  }}double c /* */ /*  */;
+  // CHECK-FIXES: {{^  }}double l = 2.;
+}
+
+struct SomeClass {
+  SomeClass() = default;
+  SomeClass(int value);
+};
+void f5() {
+  SomeClass v1, v2(42), v3{42}, v4(42.5);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // CHECK-FIXES: SomeClass v1;
+  // CHECK-FIXES: {{^  }}SomeClass v2(42);
+  // CHECK-FIXES: {{^  }}SomeClass v3{42};
+  // CHECK-FIXES: {{^  }}SomeClass v4(42.5);
+
+  SomeClass v5 = 42, *p1 = nullptr;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: SomeClass v5 = 42;
+  // CHECK-FIXES: {{^  }}SomeClass *p1 = nullptr;
+}
+
+void f6() {
+  int array1[] = {1, 2, 3, 4}, array2[] = {1, 2, 3}, value1, value2 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // CHECK-FIXES: int array1[] = {1, 2, 3, 4};
+  // CHECK-FIXES: {{^  }}int array2[] = {1, 2, 3};
+  // CHECK-FIXES: {{^  }}int value1;
+  // CHECK-FIXES: {{^  }}int value2 = 42;
+}
+
+template 
+struct TemplatedType {
+  TemplatedType() = default;
+  TemplatedType(T value);
+};
+
+void f7() {
+  TemplatedType TT1(42), TT2{42}, TT3;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 3 variables
+  // CHECK-FIXES: TemplatedType TT1(42);
+  // CHECK-FIXES: {{^  }}TemplatedType TT2{42};
+  // CHECK-FIXES: {{^  }}TemplatedType TT3;
+}
+
+void forbidden_transformations() {
+  for (int i = 0, j = 42; i < j; ++i)
+;
+}
+
+#define NULL 0
+#define MY_NICE_TYPE int **
+#define VAR_NAME(name) name##__LINE__
+#define A_BUNCH_OF_VARIABLES int m1 = 42, m2 = 43, m3 = 44;
+
+void macros() {
+  int *p1 = NULL, *p2 = NULL;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int *p1 = NULL;
+  // CHECK-FIXES: {{^  }}int *p2 = NULL;
+
+  // Macros are involved, so there will be no transformation
+  MY_NICE_TYPE p3, v1, v2;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 3 variables
+
+  int VAR_NAME(v3),
+  VAR_NAME(v4),
+  VAR_NAME(v5);
+  // CHECK-MESSAGES: [[@LINE-3]]:3: warning: this statement declares 3 variables
+
+  A_BUNCH_OF_VARIABLES
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 3 variables
+}
Index: test/clang-tidy/readability-isolate-decl-cxx17.cpp
===
--- /dev/null
+++ 

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

JonasToth marked 5 inline comments as done.
JonasToth added inline comments.



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:71
+  auto Diag =
+  diag(WholeDecl->getBeginLoc(), "make only one declaration per 
statement");
+

kbobyrev wrote:
> Maybe it's just me: this doesn't seem like a very clear diagnostic message, 
> probably needs better wording (can't think of anything in particular ATM, 
> though).
I updated the diagnostic to include the number of declared variables. I can 
reword again, for me its hard to think of something better.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 165749.
JonasToth marked an inline comment as done.
JonasToth added a comment.

- improve diagnostic to include number of declared variables


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/IsolateDeclCheck.cpp
  clang-tidy/readability/IsolateDeclCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-isolate-decl.rst
  test/clang-tidy/readability-isolate-decl-cxx17.cpp
  test/clang-tidy/readability-isolate-decl.cpp

Index: test/clang-tidy/readability-isolate-decl.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-isolate-decl.cpp
@@ -0,0 +1,121 @@
+// RUN: %check_clang_tidy %s readability-isolate-decl %t
+
+void f() {
+  int i;
+}
+
+void f2() {
+  int i, j, *k, lala = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int j;
+  // CHECK-FIXES: {{^  }}int *k;
+  // CHECK-FIXES: {{^  }}int lala = 42;
+
+  int normal, weird = /* comment */ 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int normal;
+  // CHECK-FIXES: {{^  }}int weird = /* comment */ 42;
+  //
+  int /* here is a comment */ v1,
+  // another comment
+  v2 = 42 // Ok, more comments
+  ;
+  // CHECK-MESSAGES: [[@LINE-4]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int /* here is a comment */ v1;
+  // CHECK-FIXES: {{^  }}int /* here is a comment */ // another comment
+  // CHECK-FIXES: {{^  }}v2 = 42 // Ok, more comments
+  // CHECK-FIXES: {{^  }};
+}
+
+void f3() {
+  int i, *pointer1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int *pointer1;
+  //
+  int *pointer2 = nullptr, *pointer3 = 
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int *pointer2 = nullptr;
+  // CHECK-FIXES: {{^  }}int *pointer3 = 
+}
+
+void f4() {
+  double d = 42. /* foo */, z = 43., /* hi */ y, c /* */ /*  */, l = 2.;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 5 variables
+  // CHECK-FIXES: double d = 42. /* foo */;
+  // CHECK-FIXES: {{^  }}double z = 43.;
+  // CHECK-FIXES: {{^  }}double /* hi */ y;
+  // CHECK-FIXES: {{^  }}double c /* */ /*  */;
+  // CHECK-FIXES: {{^  }}double l = 2.;
+}
+
+struct SomeClass {
+  SomeClass() = default;
+  SomeClass(int value);
+};
+void f5() {
+  SomeClass v1, v2(42), v3{42}, v4(42.5);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // CHECK-FIXES: SomeClass v1;
+  // CHECK-FIXES: {{^  }}SomeClass v2(42);
+  // CHECK-FIXES: {{^  }}SomeClass v3{42};
+  // CHECK-FIXES: {{^  }}SomeClass v4(42.5);
+
+  SomeClass v5 = 42, *p1 = nullptr;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: SomeClass v5 = 42;
+  // CHECK-FIXES: {{^  }}SomeClass *p1 = nullptr;
+}
+
+void f6() {
+  int array1[] = {1, 2, 3, 4}, array2[] = {1, 2, 3}, value1, value2 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // CHECK-FIXES: int array1[] = {1, 2, 3, 4};
+  // CHECK-FIXES: {{^  }}int array2[] = {1, 2, 3};
+  // CHECK-FIXES: {{^  }}int value1;
+  // CHECK-FIXES: {{^  }}int value2 = 42;
+}
+
+template 
+struct TemplatedType {
+  TemplatedType() = default;
+  TemplatedType(T value);
+};
+
+void f7() {
+  TemplatedType TT1(42), TT2{42}, TT3;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 3 variables
+  // CHECK-FIXES: TemplatedType TT1(42);
+  // CHECK-FIXES: {{^  }}TemplatedType TT2{42};
+  // CHECK-FIXES: {{^  }}TemplatedType TT3;
+}
+
+void forbidden_transformations() {
+  for (int i = 0, j = 42; i < j; ++i)
+;
+}
+
+#define NULL 0
+#define MY_NICE_TYPE int **
+#define VAR_NAME(name) name##__LINE__
+#define A_BUNCH_OF_VARIABLES int m1 = 42, m2 = 43, m3 = 44;
+
+void macros() {
+  int *p1 = NULL, *p2 = NULL;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int *p1 = NULL;
+  // CHECK-FIXES: {{^  }}int *p2 = NULL;
+
+  // Macros are involved, so there will be no transformation
+  MY_NICE_TYPE p3, v1, v2;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 3 variables
+
+  int VAR_NAME(v3),
+  VAR_NAME(v4),
+  VAR_NAME(v5);
+  // CHECK-MESSAGES: [[@LINE-3]]:3: warning: this statement declares 3 variables
+
+  A_BUNCH_OF_VARIABLES
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 3 variables
+}
Index: test/clang-tidy/readability-isolate-decl-cxx17.cpp
===
--- /dev/null
+++ 

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

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



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:51
+
+return TypeAndName + " = " + Initializer + ";";
+  }

kbobyrev wrote:
> JonasToth wrote:
> > kbobyrev wrote:
> > > This seems to replace `int x = 5, y = 42;` with `int x = 5;int y = 42`. I 
> > > don't think that it becomes cleaner (in fact, without spaces in between 
> > > it looks cryptic). Consider formatting it or simply applying newlines (if 
> > > there were no newlines inbetween before).
> > I do not plan to do a lot of formatting here (maybe space or newline), 
> > because that clang-format area.
> While Clang-Tidy can apply Clang-Format on top of the Fix-Its, it will still 
> look weird in the Fix-Its previews. While supporting proper formatting, in 
> general, might be hard, it totally makes sense to do some basic formatting so 
> that editor integration warnings would look better, for example.
The current version adds a new line for each decl and keeps the indendation (as 
the other check does).

Because it does the slicing on commas the manual/custom formatting of the 
original code will stay. That might result in weird looking output for exotic 
variable declarations. I would like to ignore these cases, what do you think 
@kbobyrev ?



Comment at: clang-tidy/readability/IsolateDeclCheck.h:1
+//===--- IsolateDeclCheck.h - clang-tidy-*- C++ 
-*-===//
+//

kbobyrev wrote:
> nit: space between clang-tidy (also, in .cpp file)
That comes from the template `add_new_check.py`, do you want me to fix it there?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 165742.
JonasToth added a comment.

- [Feature] initial commit for extract decl experiment
- [Feature] implement the last-element extraction version kinda ok
- [Misc] work further on the experiment
- do a full transformation of the multi decl stmt
- [Misc] add new test case for for init
- ignore some special cases, add c++17 tests
- refactor token finding unified
- make error handling more solid, broke previously working transformations for 
some reason
- fix bug for 2 decl stmts, more debug output when the transformation bails
- add tests for macros


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/IsolateDeclCheck.cpp
  clang-tidy/readability/IsolateDeclCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-isolate-decl.rst
  test/clang-tidy/readability-isolate-decl-cxx17.cpp
  test/clang-tidy/readability-isolate-decl.cpp

Index: test/clang-tidy/readability-isolate-decl.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-isolate-decl.cpp
@@ -0,0 +1,112 @@
+// RUN: %check_clang_tidy %s readability-isolate-decl %t
+
+void f() {
+  int i;
+}
+
+void f2() {
+  int i, j, *k, lala = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: declaring multiple variables in one statement
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int j;
+  // CHECK-FIXES: {{^  }}int *k;
+  // CHECK-FIXES: {{^  }}int lala = 42;
+
+  int normal, weird = /* comment */ 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: declaring multiple variables in one statement
+  // CHECK-FIXES: int normal;
+  // CHECK-FIXES: {{^  }}int weird = /* comment */ 42;
+}
+
+void f3() {
+  int i, *pointer1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: declaring multiple variables in one statement
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int *pointer1;
+  //
+  int *pointer2 = nullptr, *pointer3 = 
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: declaring multiple variables in one statement
+  // CHECK-FIXES: int *pointer2 = nullptr;
+  // CHECK-FIXES: {{^  }}int *pointer3 = 
+}
+
+void f4() {
+  double d = 42. /* foo */, z = 43., /* hi */ y, c /* */ /*  */, l = 2.;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: declaring multiple variables in one statement
+  // CHECK-FIXES: double d = 42. /* foo */;
+  // CHECK-FIXES: {{^  }}double z = 43.;
+  // CHECK-FIXES: {{^  }}double /* hi */ y;
+  // CHECK-FIXES: {{^  }}double c /* */ /*  */;
+  // CHECK-FIXES: {{^  }}double l = 2.;
+}
+
+struct SomeClass {
+  SomeClass() = default;
+  SomeClass(int value);
+};
+void f5() {
+  SomeClass v1, v2(42), v3{42}, v4(42.5);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: declaring multiple variables in one statement
+  // CHECK-FIXES: SomeClass v1;
+  // CHECK-FIXES: {{^  }}SomeClass v2(42);
+  // CHECK-FIXES: {{^  }}SomeClass v3{42};
+  // CHECK-FIXES: {{^  }}SomeClass v4(42.5);
+
+  SomeClass v5 = 42, *p1 = nullptr;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: declaring multiple variables in one statement
+  // CHECK-FIXES: SomeClass v5 = 42;
+  // CHECK-FIXES: {{^  }}SomeClass *p1 = nullptr;
+}
+
+void f6() {
+  int array1[] = {1, 2, 3, 4}, array2[] = {1, 2, 3}, value1, value2 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: declaring multiple variables in one statement
+  // CHECK-FIXES: int array1[] = {1, 2, 3, 4};
+  // CHECK-FIXES: {{^  }}int array2[] = {1, 2, 3};
+  // CHECK-FIXES: {{^  }}int value1;
+  // CHECK-FIXES: {{^  }}int value2 = 42;
+}
+
+template 
+struct TemplatedType {
+  TemplatedType() = default;
+  TemplatedType(T value);
+};
+
+void f7() {
+  TemplatedType TT1(42), TT2{42}, TT3;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: declaring multiple variables in one statement
+  // CHECK-FIXES: TemplatedType TT1(42);
+  // CHECK-FIXES: {{^  }}TemplatedType TT2{42};
+  // CHECK-FIXES: {{^  }}TemplatedType TT3;
+}
+
+void forbidden_transformations() {
+  for (int i = 0, j = 42; i < j; ++i)
+;
+}
+
+#define NULL 0
+#define MY_NICE_TYPE int **
+
+#define VAR_NAME(name) name##__LINE__
+
+#define A_BUNCH_OF_VARIABLES int m1 = 42, m2 = 43, m3 = 44;
+void macros() {
+  int *p1 = NULL, *p2 = NULL;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: declaring multiple variables in one statement
+  // CHECK-FIXES: int *p1 = NULL;
+  // CHECK-FIXES: {{^  }}int *p2 = NULL;
+
+  // Macros are involved, so there will be no transformation
+  MY_NICE_TYPE p3, v1, v2;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: declaring multiple variables in one statement
+
+  int VAR_NAME(v3),
+  VAR_NAME(v4),
+  VAR_NAME(v5);
+  // CHECK-MESSAGES: [[@LINE-3]]:3: warning: declaring multiple variables in one statement
+
+  A_BUNCH_OF_VARIABLES
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: declaring multiple variables in one statement
+}
Index: 

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D51949#1233951, @JonasToth wrote:

> Yes, do you think it should be included in the diag?


Yes, please :) Else, the message seems a bit too empty.
I **don't** think it should point (via `NOTE:`) at the each decl though.

> Am 13.09.2018 um 22:09 schrieb Roman Lebedev via Phabricator:
> 
>> lebedev.ri added inline comments.
>> 
>> 
>>  Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:200
>>  +
>>  +  diag(WholeDecl->getBeginLoc(), "make only one declaration per statement")
>>  +  << FixItHint::CreateReplacement(WholeDecl->getSourceRange(), 
>> Replacement);
>> 
>>  
>> 
>> I think you can get the count of declarations via 
>> `std::distance(WholeDecl->decl_begin(), WholeDecl->decl_end())`.
>> 
>> Repository:
>> 
>>   rCTE Clang Tools Extra
>> 
>> https://reviews.llvm.org/D51949




Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Yes, do you think it should be included in the diag?

Am 13.09.2018 um 22:09 schrieb Roman Lebedev via Phabricator:

> lebedev.ri added inline comments.
> 
> 
>  Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:200
>  +
>  +  diag(WholeDecl->getBeginLoc(), "make only one declaration per statement")
>  +  << FixItHint::CreateReplacement(WholeDecl->getSourceRange(), 
> Replacement);
> 
>  
> 
> I think you can get the count of declarations via 
> `std::distance(WholeDecl->decl_begin(), WholeDecl->decl_end())`.
> 
> Repository:
> 
>   rCTE Clang Tools Extra
> 
> https://reviews.llvm.org/D51949


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:200
+
+  diag(WholeDecl->getBeginLoc(), "make only one declaration per statement")
+  << FixItHint::CreateReplacement(WholeDecl->getSourceRange(), 
Replacement);

I think you can get the count of declarations via 
`std::distance(WholeDecl->decl_begin(), WholeDecl->decl_end())`.



Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:35
+namespace {
+SourceLocation findNextTerminator(SourceLocation Start, const SourceManager 
,
+  const LangOptions ) {

Remove duplication, make it a variadic template and use 
`Token.isOneOf(TemplateArgs...)`



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:74
+/// This function assumes, that \p DS only contains VarDecl elements.
+std::vector DeclSlice(const DeclStmt *DS, const SourceManager ,
+   const LangOptions ) {

Make this an `llvm::Expected` and bail on all macro occurences or any other 
invalid sourcelocations.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 165321.
JonasToth added a comment.

This is the newer and better working code to slice and separate multiple
vardecls in a multi-decl stmt.
I want this version only for now, as the multiple variable definitions in one
statement are by far the most common variant of the single-decl-per-stmt rule.

All other cases are supposed to be diagnosed only.
The next steps will be:

- add the testsuite of the other abondended check
- dont fixit for places like `for(int start = 0, end = 42; ...)`

This patch includes:

- do proper transformations for normal vardecls
- finalize current tests and trim right whitespace of last generated replacement


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/IsolateDeclCheck.cpp
  clang-tidy/readability/IsolateDeclCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-isolate-decl.rst
  test/clang-tidy/readability-isolate-decl.cpp

Index: test/clang-tidy/readability-isolate-decl.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-isolate-decl.cpp
@@ -0,0 +1,82 @@
+// RUN: %check_clang_tidy %s readability-isolate-decl %t
+
+void f() {
+  int i;
+}
+
+void f2() {
+  int i, j, *k, lala = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: make only one declaration per statement
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int j;
+  // CHECK-FIXES: {{^  }}int *k;
+  // CHECK-FIXES: {{^  }}int lala = 42;
+
+  int normal, weird = /* comment */ 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: make only one declaration per statement
+  // CHECK-FIXES: int normal;
+  // CHECK-FIXES: {{^  }}int weird = /* comment */ 42;
+}
+
+void f3() {
+  int i, *pointer1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: make only one declaration per statement
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int *pointer1;
+  //
+  int *pointer2 = nullptr, *pointer3 = 
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: make only one declaration per statement
+  // CHECK-FIXES: int *pointer2 = nullptr;
+  // CHECK-FIXES: {{^  }}int *pointer3 = 
+}
+
+void f4() {
+  double d = 42. /* foo */, z = 43., /* hi */ y, c /* */ /*  */, l = 2.;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: make only one declaration per statement
+  // CHECK-FIXES: double d = 42. /* foo */;
+  // CHECK-FIXES: {{^  }}double z = 43.;
+  // CHECK-FIXES: {{^  }}double /* hi */ y;
+  // CHECK-FIXES: {{^  }}double c /* */ /*  */;
+  // CHECK-FIXES: {{^  }}double l = 2.;
+}
+
+struct SomeClass {
+  SomeClass() = default;
+  SomeClass(int value);
+};
+void f5() {
+  SomeClass v1, v2(42), v3{42}, v4(42.5);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: make only one declaration per statement
+  // CHECK-FIXES: SomeClass v1;
+  // CHECK-FIXES: {{^  }}SomeClass v2(42);
+  // CHECK-FIXES: {{^  }}SomeClass v3{42};
+  // CHECK-FIXES: {{^  }}SomeClass v4(42.5);
+
+  SomeClass v5 = 42, *p1 = nullptr;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: make only one declaration per statement
+  // CHECK-FIXES: SomeClass v5 = 42;
+  // CHECK-FIXES: {{^  }}SomeClass *p1 = nullptr;
+}
+
+void f6() {
+  int array1[] = {1, 2, 3, 4}, array2[] = {1, 2, 3}, value1, value2 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: make only one declaration per statement
+  // CHECK-FIXES: int array1[] = {1, 2, 3, 4};
+  // CHECK-FIXES: {{^  }}int array2[] = {1, 2, 3};
+  // CHECK-FIXES: {{^  }}int value1;
+  // CHECK-FIXES: {{^  }}int value2 = 42;
+}
+
+template 
+struct TemplatedType {
+  TemplatedType() = default;
+  TemplatedType(T value);
+};
+
+void f7() {
+  TemplatedType TT1(42), TT2{42}, TT3;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: make only one declaration per statement
+  // CHECK-FIXES: TemplatedType TT1(42);
+  // CHECK-FIXES: {{^  }}TemplatedType TT2{42};
+  // CHECK-FIXES: {{^  }}TemplatedType TT3;
+}
Index: docs/clang-tidy/checks/readability-isolate-decl.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-isolate-decl.rst
@@ -0,0 +1,6 @@
+.. title:: clang-tidy - readability-isolate-decl
+
+readability-isolate-decl
+
+
+FIXME: Describe what patterns does the check detect and why. Give examples.
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -225,6 +225,7 @@
readability-identifier-naming
readability-implicit-bool-conversion
readability-inconsistent-declaration-parameter-name
+   readability-isolate-decl
readability-magic-numbers
readability-misleading-indentation
readability-misplaced-array-index
Index: docs/ReleaseNotes.rst
===
--- 

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In https://reviews.llvm.org/D51949#1232443, @kbobyrev wrote:

> I've been thinking about corner-cases (e.g. don't split `DeclStmt`s within 
> init-statement declaration) a while and it seems that there might be many of 
> them.


Yes, things I am currently thinnking of:

- `if (condition) int i, j, k = function(i,j);` is not allowed to be 
transformed, as it will change semantics.
- `for(int i, j; ..)` similar case
- the new C++17 if-init
- Everything related with Macros

I want the transformation and run it over with good test-suites and see what 
happens :)

> I didn't notice https://reviews.llvm.org/D27621 in the first place, it seems 
> to have a solid test suite (and it also does some basic formatting, which is 
> nice). It might be worthy to pick it up where it was left. Alternatively, you 
> might want to pull the test cases from that patch if you don't want to reuse 
> the code.

Totally, the test-suite looks very good. Its missing some VarDecl-FunctionDecl 
mixing and cases like this, but I plan to at least use the tests. You right 
with the basic formatting, at least indendation and new lines (as the other 
check does) are worth it.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D51949#1232443, @kbobyrev wrote:

> I've been thinking about corner-cases (e.g. don't split `DeclStmt`s within 
> init-statement declaration) a while and it seems that there might be many of 
> them.


It should at least diagnose these cases.
This check is really needed.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

I've been thinking about corner-cases (e.g. don't split `DeclStmt`s within 
init-statement declaration) a while and it seems that there might be many of 
them.

I didn't notice https://reviews.llvm.org/D27621 in the first place, it seems to 
have a solid test suite (and it also does some basic formatting, which is 
nice). It might be worthy to pick it up where it was left. Alternatively, you 
might want to pull the test cases from that patch if you don't want to reuse 
the code.




Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:51
+
+return TypeAndName + " = " + Initializer + ";";
+  }

JonasToth wrote:
> kbobyrev wrote:
> > This seems to replace `int x = 5, y = 42;` with `int x = 5;int y = 42`. I 
> > don't think that it becomes cleaner (in fact, without spaces in between it 
> > looks cryptic). Consider formatting it or simply applying newlines (if 
> > there were no newlines inbetween before).
> I do not plan to do a lot of formatting here (maybe space or newline), 
> because that clang-format area.
While Clang-Tidy can apply Clang-Format on top of the Fix-Its, it will still 
look weird in the Fix-Its previews. While supporting proper formatting, in 
general, might be hard, it totally makes sense to do some basic formatting so 
that editor integration warnings would look better, for example.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:35
+
+  std::string TypeAndName =
+  VarType.getAsString(TypePrinter) + " " + D->getNameAsString();

kbobyrev wrote:
> `llvm::Twine` here? Seems like this one is used a lot for concatenation, so 
> just `.str()` when returning.
the logic already changed (but i did not update the diff, because its still 
super prototype).
Maybe i go to `Twine`, but i want it do be more a utility function that can be 
used in other checks that want to do type transformations.



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:51
+
+return TypeAndName + " = " + Initializer + ";";
+  }

kbobyrev wrote:
> This seems to replace `int x = 5, y = 42;` with `int x = 5;int y = 42`. I 
> don't think that it becomes cleaner (in fact, without spaces in between it 
> looks cryptic). Consider formatting it or simply applying newlines (if there 
> were no newlines inbetween before).
I do not plan to do a lot of formatting here (maybe space or newline), because 
that clang-format area.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:35
+
+  std::string TypeAndName =
+  VarType.getAsString(TypePrinter) + " " + D->getNameAsString();

`llvm::Twine` here? Seems like this one is used a lot for concatenation, so 
just `.str()` when returning.



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:51
+
+return TypeAndName + " = " + Initializer + ";";
+  }

This seems to replace `int x = 5, y = 42;` with `int x = 5;int y = 42`. I don't 
think that it becomes cleaner (in fact, without spaces in between it looks 
cryptic). Consider formatting it or simply applying newlines (if there were no 
newlines inbetween before).



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:71
+  auto Diag =
+  diag(WholeDecl->getBeginLoc(), "make only one declaration per 
statement");
+

Maybe it's just me: this doesn't seem like a very clear diagnostic message, 
probably needs better wording (can't think of anything in particular ATM, 
though).



Comment at: clang-tidy/readability/IsolateDeclCheck.h:1
+//===--- IsolateDeclCheck.h - clang-tidy-*- C++ 
-*-===//
+//

nit: space between clang-tidy (also, in .cpp file)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

I knew there was something already but I couldn't find it. Thank you for
pointing me there :)

I will inspect the code and then decide, maybe I will incorporate some
of his stuff into mine or take his one over.

Am 12.09.2018 um 08:04 schrieb Roman Lebedev via Phabricator:

> lebedev.ri added a comment.
> 
> Thank you for working on this! I really miss this check.
>  To be noted, there is preexisting, almost finished version - 
> https://reviews.llvm.org/D27621.
>  I'm not sure in what state it really is, but it might be simpler to take 
> that over and finish it.
> 
> Repository:
> 
>   rCTE Clang Tools Extra
> 
> https://reviews.llvm.org/D51949


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Thank you for working on this! I really miss this check.
To be noted, there is preexisting, almost finished version - 
https://reviews.llvm.org/D27621.
I'm not sure in what state it really is, but it might be simpler to take that 
over and finish it.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Ty for the initial review, I am currently rewriting the approach for it.
I will probably make the business logic as standalone class/functions to
utilize this functionality in multiple checks.

I first try to evaluate my approach and as I had many unclear things i
opened this revision to point at actual code when I have a question.

Am 12.09.2018 um 00:59 schrieb Eugene Zelenko via Phabricator:

> Eugene.Zelenko added a comment.
> 
> I would suggest to use declaration or declarations in check name instead of 
> abbreviation.
> 
> Actually there are existing coding guidelines for thus rule, like SEI CERT 
> https://wiki.sei.cmu.edu/confluence/display/c/DCL04-C.+Do+not+declare+more+than+one+variable+per+declaration.
> 
> Repository:
> 
>   rCTE Clang Tools Extra
> 
> https://reviews.llvm.org/D51949


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

I would suggest to use declaration or declarations in check name instead of 
abbreviation.

Actually there are existing coding guidelines for thus rule, like SEI CERT 
.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:29
+
+namespace {
+std::string GetIsolatedDecl(const VarDecl *D, const SourceManager ,

Please use static instead.



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:37
+  VarType.getAsString(TypePrinter) + " " + D->getNameAsString();
+  std::string Initializer = "";
+

See readability-redundant-string-init.



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:60
+  const auto *WholeDecl = Result.Nodes.getNodeAs("decl_stmt");
+  std::string Replacement = "";
+

See readability-redundant-string-init.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

JonasToth created this revision.
JonasToth added reviewers: rsmith, aaron.ballman, alexfh, hokein.
Herald added subscribers: cfe-commits, xazax.hun, mgorny.

The idea of this check is to enforce one decl per statement and to include
an automated code transformation for all multi-decl statments.
It turns out this is more complicated then I excepted, so this differential
is a playground for now to allow easy commenting and discussions on the code.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/IsolateDeclCheck.cpp
  clang-tidy/readability/IsolateDeclCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-isolate-decl.rst
  test/clang-tidy/readability-isolate-decl.cpp

Index: test/clang-tidy/readability-isolate-decl.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-isolate-decl.cpp
@@ -0,0 +1,65 @@
+// RUN: %check_clang_tidy %s readability-isolate-decl %t
+
+void f() {
+  int i;
+}
+
+void f2() {
+  int i, j, *k, lala = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: make only one declaration per statement
+  // CHECK-FIXES: int i;int j;int * k;int lala = 42;
+  //
+  int normal, weird = /* comment */ 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: make only one declaration per statement
+  // CHECK-FIXES: int normal;int weird = 42;
+}
+
+void f3() {
+  int i, *pointer1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: make only one declaration per statement
+  // CHECK-FIXES: int i;int * pointer1;
+  //
+  int *pointer2 = nullptr, *pointer3 = 
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: make only one declaration per statement
+  // CHECK-FIXES: int * pointer2 = nullpt;int * pointer3 = 
+}
+
+void f4() {
+  // clang-format off
+  double d = 42./* foo */, z = 43., /* hi */ y, c /* */ /*  */, l = 2.;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: make only one declaration per statement
+  // CHECK-FIXES: double d = 42;double z = 43;double y;double c;double l = 2.;
+  // clang-format on
+}
+
+struct SomeClass {
+  SomeClass() = default;
+  SomeClass(int value);
+};
+void f5() {
+  SomeClass v1, v2(42), v3{42}, v4(42.5);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: make only one declaration per statement
+  // CHECK-FIXES: SomeClass v1 = v1;SomeClass v2 = v2(42);SomeClass v3 = v3{42};SomeClass v4 = v4(42.5);
+
+  SomeClass v5 = 42, *p1 = nullptr;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: make only one declaration per statement
+  // CHECK-FIXES: SomeClass v5 = v5 = 42;SomeClass * p1 = nullpt;
+}
+
+void f6() {
+  int array1[] = {1, 2, 3, 4}, array2[] = {1, 2, 3}, value1, value2 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: make only one declaration per statement
+  // CHECK-FIXES: int [4] array1 = {1, 2, 3, 4};int [3] array2 = {1, 2, 3};int value1;int value2 = 42;
+}
+
+template 
+struct TemplatedType {
+TemplatedType() = default;
+TemplatedType(T value);
+};
+
+void f7() {
+  TemplatedType TT1(42), TT2{42}, TT3;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: make only one declaration per statement
+  // CHECK-FIXES: TemplatedType TT1 = TT1(42);TemplatedType TT2 = TT2{42};TemplatedType TT3 = TT;
+}
Index: docs/clang-tidy/checks/readability-isolate-decl.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-isolate-decl.rst
@@ -0,0 +1,6 @@
+.. title:: clang-tidy - readability-isolate-decl
+
+readability-isolate-decl
+
+
+FIXME: Describe what patterns does the check detect and why. Give examples.
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -225,6 +225,7 @@
readability-identifier-naming
readability-implicit-bool-conversion
readability-inconsistent-declaration-parameter-name
+   readability-isolate-decl
readability-magic-numbers
readability-misleading-indentation
readability-misplaced-array-index
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -93,6 +93,11 @@
   Flags uses of ``absl::StrCat()`` to append to a ``std::string``. Suggests 
   ``absl::StrAppend()`` should be used instead.
 
+- New :doc:`readability-isolate-decl
+  ` check.
+
+  FIXME: add release notes.
+
 - New :doc:`readability-magic-numbers
   ` check.
 
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -20,6 +20,7 @@
 #include "IdentifierNamingCheck.h"
 #include "ImplicitBoolConversionCheck.h"
 #include