[PATCH] D70270: clang-tidy: modernize-use-using uses AST and now supports struct defintions and multiple types in a typedef

2020-01-07 Thread Mitchell via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG73d93617d3ae: [clang-tidy] modernize-use-using uses AST and 
now supports struct defintions… (authored by mitchell-stellar).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70270

Files:
  clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-using.rst
  clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
  clang/include/clang/Basic/SourceLocation.h

Index: clang/include/clang/Basic/SourceLocation.h
===
--- clang/include/clang/Basic/SourceLocation.h
+++ clang/include/clang/Basic/SourceLocation.h
@@ -230,6 +230,11 @@
 return B != X.B || E != X.E;
   }
 
+  // Returns true iff other is wholly contained within this range.
+  bool fullyContains(const SourceRange ) const {
+return B <= other.B && E >= other.E;
+  }
+
   void print(raw_ostream , const SourceManager ) const;
   std::string printToString(const SourceManager ) const;
   void dump(const SourceManager ) const;
Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
@@ -84,7 +84,11 @@
 
 typedef int bla1, bla2, bla3;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef int bla1, bla2, bla3;
+// CHECK-MESSAGES: :[[@LINE-2]]:17: warning: use 'using' instead of 'typedef'
+// CHECK-MESSAGES: :[[@LINE-3]]:23: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using bla1 = int;
+// CHECK-FIXES-NEXT: using bla2 = int;
+// CHECK-FIXES-NEXT: using bla3 = int;
 
 #define CODE typedef int INT
 
@@ -136,16 +140,16 @@
 
 typedef struct Q1 { int a; } S1;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef struct Q1 { int a; } S1;
+// CHECK-FIXES: using S1 = struct Q1 { int a; };
 typedef struct { int b; } S2;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef struct { int b; } S2;
+// CHECK-FIXES: using S2 = struct { int b; };
 struct Q2 { int c; } typedef S3;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: struct Q2 { int c; } typedef S3;
+// CHECK-FIXES: using S3 = struct Q2 { int c; };
 struct { int d; } typedef S4;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: struct { int d; } typedef S4;
+// CHECK-FIXES: using S4 = struct { int d; };
 
 namespace my_space {
   class my_cclass {};
@@ -196,11 +200,15 @@
 
 typedef S<(0 > 0), int> S_t, *S_p;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef S<(0 > 0), int> S_t, *S_p;
+// CHECK-MESSAGES: :[[@LINE-2]]:28: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using S_t = S<(0 > 0), int>;
+// CHECK-FIXES-NEXT: using S_p = S_t*;
 
 typedef S<(0 < 0), int> S2_t, *S2_p;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef S<(0 < 0), int> S2_t, *S2_p;
+// CHECK-MESSAGES: :[[@LINE-2]]:29: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using S2_t = S<(0 < 0), int>;
+// CHECK-FIXES-NEXT: using S2_p = S2_t*;
 
 typedef S<(0 > 0 && (3 > 1) && (1 < 1)), int> S3_t;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
@@ -213,7 +221,9 @@
 
 typedef Q Q_t, *Q_p;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef Q Q_t, *Q_p;
+// CHECK-MESSAGES: :[[@LINE-2]]:24: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using Q_t = Q;
+// CHECK-FIXES-NEXT: using Q_p = Q_t*;
 
 typedef Q Q2_t;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
@@ -227,7 +237,9 @@
 
 typedef Q Q3_t, *Q3_p;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef Q Q3_t, *Q3_p;
+// CHECK-MESSAGES: :[[@LINE-2]]:27: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using Q3_t = Q;
+// CHECK-FIXES-NEXT: using Q3_p = Q3_t*;
 
 typedef Q Q3_t;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
@@ -246,4 +258,12 @@
 
 typedef Variadic >, S<(0 < 0), Variadic > > > Variadic_t, *Variadic_p;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef Variadic >, S<(0 < 0), Variadic > > > Variadic_t, *Variadic_p;
+// CHECK-MESSAGES: :[[@LINE-2]]:103: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using Variadic_t =

[PATCH] D70270: clang-tidy: modernize-use-using uses AST and now supports struct defintions and multiple types in a typedef

2019-12-24 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D70270



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

[PATCH] D70270: clang-tidy: modernize-use-using uses AST and now supports struct defintions and multiple types in a typedef

2019-12-21 Thread Conrad Poelman via Phabricator via cfe-commits 
poelmanc updated this revision to Diff 235039.
poelmanc added a subscriber: sammccall.
poelmanc added a comment.

Update patch to rebase on latest: Changed `SourceLocation::contains` to 
`SourceLocation::fullyContains`, and removed new `SourceLocation` comparison 
operators since coincidentally @sammccall added them just days ago.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D70270

Files:
  clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-using.rst
  clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
  clang/include/clang/Basic/SourceLocation.h

Index: clang/include/clang/Basic/SourceLocation.h
===
--- clang/include/clang/Basic/SourceLocation.h
+++ clang/include/clang/Basic/SourceLocation.h
@@ -230,6 +230,11 @@
 return B != X.B || E != X.E;
   }
 
+  // Returns true iff other is wholly contained within this range.
+  bool fullyContains(const SourceRange ) const {
+return B <= other.B && E >= other.E;
+  }
+
   void print(raw_ostream , const SourceManager ) const;
   std::string printToString(const SourceManager ) const;
   void dump(const SourceManager ) const;
Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
@@ -84,7 +84,11 @@
 
 typedef int bla1, bla2, bla3;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef int bla1, bla2, bla3;
+// CHECK-MESSAGES: :[[@LINE-2]]:17: warning: use 'using' instead of 'typedef'
+// CHECK-MESSAGES: :[[@LINE-3]]:23: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using bla1 = int;
+// CHECK-FIXES-NEXT: using bla2 = int;
+// CHECK-FIXES-NEXT: using bla3 = int;
 
 #define CODE typedef int INT
 
@@ -136,16 +140,16 @@
 
 typedef struct Q1 { int a; } S1;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef struct Q1 { int a; } S1;
+// CHECK-FIXES: using S1 = struct Q1 { int a; };
 typedef struct { int b; } S2;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef struct { int b; } S2;
+// CHECK-FIXES: using S2 = struct { int b; };
 struct Q2 { int c; } typedef S3;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: struct Q2 { int c; } typedef S3;
+// CHECK-FIXES: using S3 = struct Q2 { int c; };
 struct { int d; } typedef S4;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: struct { int d; } typedef S4;
+// CHECK-FIXES: using S4 = struct { int d; };
 
 namespace my_space {
   class my_cclass {};
@@ -196,11 +200,15 @@
 
 typedef S<(0 > 0), int> S_t, *S_p;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef S<(0 > 0), int> S_t, *S_p;
+// CHECK-MESSAGES: :[[@LINE-2]]:28: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using S_t = S<(0 > 0), int>;
+// CHECK-FIXES-NEXT: using S_p = S_t*;
 
 typedef S<(0 < 0), int> S2_t, *S2_p;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef S<(0 < 0), int> S2_t, *S2_p;
+// CHECK-MESSAGES: :[[@LINE-2]]:29: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using S2_t = S<(0 < 0), int>;
+// CHECK-FIXES-NEXT: using S2_p = S2_t*;
 
 typedef S<(0 > 0 && (3 > 1) && (1 < 1)), int> S3_t;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
@@ -213,7 +221,9 @@
 
 typedef Q Q_t, *Q_p;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef Q Q_t, *Q_p;
+// CHECK-MESSAGES: :[[@LINE-2]]:24: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using Q_t = Q;
+// CHECK-FIXES-NEXT: using Q_p = Q_t*;
 
 typedef Q Q2_t;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
@@ -227,7 +237,9 @@
 
 typedef Q Q3_t, *Q3_p;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef Q Q3_t, *Q3_p;
+// CHECK-MESSAGES: :[[@LINE-2]]:27: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using Q3_t = Q;
+// CHECK-FIXES-NEXT: using Q3_p = Q3_t*;
 
 typedef Q Q3_t;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
@@ -246,4 +258,12 @@
 
 typedef Variadic >, S<(0 < 0), Variadic > > > Variadic_t, *Variadic_p;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef Variadic >, S<(0 < 0), Variadic > > > Variadic_t, *Variadic_p;

[PATCH] D70270: clang-tidy: modernize-use-using uses AST and now supports struct defintions and multiple types in a typedef

2019-12-20 Thread Conrad Poelman via Phabricator via cfe-commits 
poelmanc added a comment.

Any further feedback or thoughts on this?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D70270



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

[PATCH] D70270: clang-tidy: modernize-use-using uses AST and now supports struct defintions and multiple types in a typedef

2019-11-16 Thread Conrad Poelman via Phabricator via cfe-commits 
poelmanc added a comment.

> If there's a way to match only `CXXRecordDecl`s that are immediately followed 
> by a `TypedefDecl`...

Alternatively, within `check()` when we get the `TypedefDecl`, is there any way 
to navigate up the AST to find its immediately preceding sibling node in the 
AST and check whether it's a `CXXRecordDecl`? If so we could eliminate 
`Finder->addMatcher(cxxRecordDecl(unless(isImplicit())).bind("struct"), this);` 
altogether. I didn't see a way to do that though.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D70270



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

[PATCH] D70270: clang-tidy: modernize-use-using uses AST and now supports struct defintions and multiple types in a typedef

2019-11-16 Thread Conrad Poelman via Phabricator via cfe-commits 
poelmanc marked 6 inline comments as done.
poelmanc added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp:30
+  // They appear in the AST just *prior* to the typedefs.
+  Finder->addMatcher(cxxRecordDecl().bind("struct"), this);
 }

aaron.ballman wrote:
> It's unfortunate that we can't restrict this matcher more -- there tend to be 
> a lot of struct declarations, so I am a bit worried about the performance of 
> this matching on so many of them. At a minimum, I think you can restrict it 
> to non-implicit structs here and simplify the code in `check()` a bit.
Done, thanks!

If there's a way to match only `CXXRecordDecl`s that are //immediately 
followed// by a `TypedefDecl`, that would cut down the matches just to the ones 
we need. That said, at least the amount of work done on each `check()` call for 
the non-implicit `CXXRecordDecl`s is minimal: it just stores its `SourceRange` 
and returns.



Comment at: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp:85
+ReplaceRange.setBegin(LastReplacementEnd);
+Using = "; using ";
+

aaron.ballman wrote:
> Should there be a newline here, to avoid putting all the using declarations 
> on the same line?
Done. Updated test cases and documentation to reflect this.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D70270



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

[PATCH] D70270: clang-tidy: modernize-use-using uses AST and now supports struct defintions and multiple types in a typedef

2019-11-16 Thread Conrad Poelman via Phabricator via cfe-commits 
poelmanc updated this revision to Diff 229678.
poelmanc edited the summary of this revision.

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D70270

Files:
  clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-using.rst
  clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
  clang/include/clang/Basic/SourceLocation.h

Index: clang/include/clang/Basic/SourceLocation.h
===
--- clang/include/clang/Basic/SourceLocation.h
+++ clang/include/clang/Basic/SourceLocation.h
@@ -192,6 +192,18 @@
   return LHS.getRawEncoding() < RHS.getRawEncoding();
 }
 
+inline bool operator>(const SourceLocation , const SourceLocation ) {
+  return LHS.getRawEncoding() > RHS.getRawEncoding();
+}
+
+inline bool operator<=(const SourceLocation , const SourceLocation ) {
+  return !(LHS > RHS);
+}
+
+inline bool operator>=(const SourceLocation , const SourceLocation ) {
+  return !(LHS < RHS);
+}
+
 /// A trivial tuple used to represent a source range.
 class SourceRange {
   SourceLocation B;
@@ -219,6 +231,11 @@
 return B != X.B || E != X.E;
   }
 
+  // Returns true iff other is wholly contained within this range.
+  bool contains(const SourceRange ) const {
+return B <= other.B && E >= other.E;
+  }
+
   void print(raw_ostream , const SourceManager ) const;
   std::string printToString(const SourceManager ) const;
   void dump(const SourceManager ) const;
Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
@@ -84,7 +84,11 @@
 
 typedef int bla1, bla2, bla3;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef int bla1, bla2, bla3;
+// CHECK-MESSAGES: :[[@LINE-2]]:17: warning: use 'using' instead of 'typedef'
+// CHECK-MESSAGES: :[[@LINE-3]]:23: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using bla1 = int;
+// CHECK-FIXES-NEXT: using bla2 = int;
+// CHECK-FIXES-NEXT: using bla3 = int;
 
 #define CODE typedef int INT
 
@@ -136,16 +140,16 @@
 
 typedef struct Q1 { int a; } S1;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef struct Q1 { int a; } S1;
+// CHECK-FIXES: using S1 = struct Q1 { int a; };
 typedef struct { int b; } S2;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef struct { int b; } S2;
+// CHECK-FIXES: using S2 = struct { int b; };
 struct Q2 { int c; } typedef S3;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: struct Q2 { int c; } typedef S3;
+// CHECK-FIXES: using S3 = struct Q2 { int c; };
 struct { int d; } typedef S4;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: struct { int d; } typedef S4;
+// CHECK-FIXES: using S4 = struct { int d; };
 
 namespace my_space {
   class my_cclass {};
@@ -196,11 +200,15 @@
 
 typedef S<(0 > 0), int> S_t, *S_p;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef S<(0 > 0), int> S_t, *S_p;
+// CHECK-MESSAGES: :[[@LINE-2]]:28: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using S_t = S<(0 > 0), int>;
+// CHECK-FIXES-NEXT: using S_p = S_t*;
 
 typedef S<(0 < 0), int> S2_t, *S2_p;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef S<(0 < 0), int> S2_t, *S2_p;
+// CHECK-MESSAGES: :[[@LINE-2]]:29: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using S2_t = S<(0 < 0), int>;
+// CHECK-FIXES-NEXT: using S2_p = S2_t*;
 
 typedef S<(0 > 0 && (3 > 1) && (1 < 1)), int> S3_t;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
@@ -213,7 +221,9 @@
 
 typedef Q Q_t, *Q_p;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef Q Q_t, *Q_p;
+// CHECK-MESSAGES: :[[@LINE-2]]:24: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using Q_t = Q;
+// CHECK-FIXES-NEXT: using Q_p = Q_t*;
 
 typedef Q Q2_t;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
@@ -227,7 +237,9 @@
 
 typedef Q Q3_t, *Q3_p;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef Q Q3_t, *Q3_p;
+// CHECK-MESSAGES: :[[@LINE-2]]:27: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using Q3_t = Q;
+// CHECK-FIXES-NEXT: using Q3_p = Q3_t*;
 
 typedef Q Q3_t;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of

[PATCH] D70270: clang-tidy: modernize-use-using uses AST and now supports struct defintions and multiple types in a typedef

2019-11-16 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp:30
+  // They appear in the AST just *prior* to the typedefs.
+  Finder->addMatcher(cxxRecordDecl().bind("struct"), this);
 }

It's unfortunate that we can't restrict this matcher more -- there tend to be a 
lot of struct declarations, so I am a bit worried about the performance of this 
matching on so many of them. At a minimum, I think you can restrict it to 
non-implicit structs here and simplify the code in `check()` a bit.



Comment at: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp:55
 
-  // do not fix if there is macro or array
-  if (MatchedDecl->getUnderlyingType()->isArrayType() || StartLoc.isMacroID())
+  // warn at StartLoc but do not fix if there is macro or array
+  if (MatchedDecl->getUnderlyingType()->isArrayType() || StartLoc.isMacroID()) 
{

warn -> Warn
Add a full stop to the end of the comment.



Comment at: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp:57
+  if (MatchedDecl->getUnderlyingType()->isArrayType() || StartLoc.isMacroID()) 
{
+auto Diag = diag(StartLoc, UseUsingWarning);
 return;

Is there a purpose to the `Diag` local variable, or can it just be removed?



Comment at: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp:85
+ReplaceRange.setBegin(LastReplacementEnd);
+Using = "; using ";
+

Should there be a newline here, to avoid putting all the using declarations on 
the same line?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D70270



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

[PATCH] D70270: clang-tidy: modernize-use-using uses AST and now supports struct defintions and multiple types in a typedef

2019-11-14 Thread Conrad Poelman via Phabricator via cfe-commits 
poelmanc created this revision.
poelmanc added reviewers: alexfh, aaron.ballman.
poelmanc added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, mgehre.
Herald added a project: clang.

In D67460#1737529 , @aaron.ballman 
wrote:

> I'm a bit worried that this manual parsing technique will need fixing again 
> in the future


That concern plus prior comments and help from `clang -ast-dump` led me to 
develop this new approach based entirely on the AST that completely eliminates 
manual parsing. It now handles `typedef`s that include comma-separated multiple 
types, and handles embedded `struct` definitions, which previously could not be 
automatically converted.

For example, with this patch `modernize-use-using` now can convert:

  typedef struct { int a; } R_t, *R_p;

to:

  using R_t = struct { int a; }; using R_p = R_t*;

`-ast-dump` showed that the `CXXRecordDecl` definitions and multiple 
`TypedefDecl`s come consecutively in the tree, so `check()` stores information 
between calls to determine when it is receiving a second or additional 
`TypedefDecl` within a single `typedef`, or when the current `TypedefDecl` 
refers to an embedded `CXXRecordDecl` like a `struct`.

This patch is independent of D67460  except 
for borrowing its extended set of tests. All those tests pass, plus several 
that previously remained as `typedef` are now modernized to use `using`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D70270

Files:
  clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-using.rst
  clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
  clang/include/clang/Basic/SourceLocation.h

Index: clang/include/clang/Basic/SourceLocation.h
===
--- clang/include/clang/Basic/SourceLocation.h
+++ clang/include/clang/Basic/SourceLocation.h
@@ -192,6 +192,18 @@
   return LHS.getRawEncoding() < RHS.getRawEncoding();
 }
 
+inline bool operator>(const SourceLocation , const SourceLocation ) {
+  return LHS.getRawEncoding() > RHS.getRawEncoding();
+}
+
+inline bool operator<=(const SourceLocation , const SourceLocation ) {
+  return !(LHS > RHS);
+}
+
+inline bool operator>=(const SourceLocation , const SourceLocation ) {
+  return !(LHS < RHS);
+}
+
 /// A trivial tuple used to represent a source range.
 class SourceRange {
   SourceLocation B;
@@ -219,6 +231,11 @@
 return B != X.B || E != X.E;
   }
 
+  // Returns true iff other is wholly contained within this range.
+  bool contains(const SourceRange ) const {
+return B <= other.B && E >= other.E;
+  }
+
   void print(raw_ostream , const SourceManager ) const;
   std::string printToString(const SourceManager ) const;
   void dump(const SourceManager ) const;
Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
@@ -84,7 +84,9 @@
 
 typedef int bla1, bla2, bla3;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef int bla1, bla2, bla3;
+// CHECK-MESSAGES: :[[@LINE-2]]:17: warning: use 'using' instead of 'typedef'
+// CHECK-MESSAGES: :[[@LINE-3]]:23: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using bla1 = int; using bla2 = int; using bla3 = int;
 
 #define CODE typedef int INT
 
@@ -136,16 +138,16 @@
 
 typedef struct Q1 { int a; } S1;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef struct Q1 { int a; } S1;
+// CHECK-FIXES: using S1 = struct Q1 { int a; };
 typedef struct { int b; } S2;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef struct { int b; } S2;
+// CHECK-FIXES: using S2 = struct { int b; };
 struct Q2 { int c; } typedef S3;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: struct Q2 { int c; } typedef S3;
+// CHECK-FIXES: using S3 = struct Q2 { int c; };
 struct { int d; } typedef S4;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: struct { int d; } typedef S4;
+// CHECK-FIXES: using S4 = struct { int d; };
 
 namespace my_space {
   class my_cclass {};
@@ -183,3 +185,77 @@
   void f() override { super::f(); }
 };
 }
+
+template 
+class TwoArgTemplate {
+  typedef TwoArgTemplate self;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'using' instead of 'typedef'
+  // CHECK-FIXES: using self = TwoArgTemplate;
+};
+
+template 
+struct S {};
+
+typedef S<(0 > 0), int> S_t, *S_p;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: