poelmanc updated this revision to Diff 219957.
poelmanc added a comment.

Thanks for the stressing test cases. I reverted to your original test case with 
a single >, added a separate one with a single <, and expanded the final case 
to have a non-balanced number of > and <. I added all your new cases, with 
variations for non-fixed (multiple typedef) and fixed (single typedef) examples.

To make the braces example pass, we now only abandon the fix when encountering 
a non-nested open brace.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D67460

Files:
  clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
  clang-tools-extra/test/clang-tidy/modernize-use-using.cpp

Index: clang-tools-extra/test/clang-tidy/modernize-use-using.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/modernize-use-using.cpp
+++ clang-tools-extra/test/clang-tidy/modernize-use-using.cpp
@@ -182,4 +182,53 @@
 class E : public C<E> {
   void f() override { super::f(); }
 };
+
+template <typename T1, typename T2>
+class TwoArgTemplate {
+  typedef TwoArgTemplate<T1, T2> self;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'using' instead of 'typedef'
+  // CHECK-FIXES: using self = TwoArgTemplate<T1, T2>;
+};
 }
+
+template <bool B, typename T>
+struct S {};
+
+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;
+
+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;
+
+typedef S<(0 > 0 && (3 > 1) && (1 < 1)), int> S3_t;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using S3_t = S<(0 > 0 && (3 > 1) && (1 < 1)), int>;
+
+template <bool B>
+struct Q {};
+
+constexpr bool b[1] = {true};
+
+typedef Q<b[0 < 0]> Q_t, *Q_p;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: typedef Q<b[0 < 0]> Q_t, *Q_p;
+
+typedef Q<b[0 < 0]> Q2_t;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using Q2_t = Q<b[0 < 0]>;
+
+struct T {
+  constexpr T(bool) {}
+
+  static constexpr bool b = true;
+};
+
+typedef Q<T{0 < 0}.b> Q3_t, *Q3_p;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: typedef Q<T{0 < 0}.b> Q3_t, *Q3_p;
+
+typedef Q<T{0 < 0}.b> Q3_t;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using Q3_t = Q<T{0 < 0}.b>;
Index: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -40,24 +40,54 @@
 
   Token Tok;
   int ParenLevel = 0;
+  int BraceLevel = 0;
+  int AngleBracketLevel = 0;
+  int SquareBracketLevel = 0;
   bool FoundTypedef = false;
 
   while (!DeclLexer.LexFromRawLexer(Tok) && !Tok.is(tok::semi)) {
     switch (Tok.getKind()) {
     case tok::l_brace:
+      if (ParenLevel == 0 && SquareBracketLevel == 0 && AngleBracketLevel == 0) {
+        // At top level, this might be the `typedef struct {...} T;` case.
+        // Inside parens, square brackets, or angle brackets it's not.
+        return false;
+      }
+      BraceLevel++;
+      break;
     case tok::r_brace:
-      // This might be the `typedef struct {...} T;` case.
-      return false;
+      BraceLevel--;
+      break;
     case tok::l_paren:
       ParenLevel++;
       break;
     case tok::r_paren:
       ParenLevel--;
       break;
+    case tok::l_square:
+      SquareBracketLevel++;
+      break;
+    case tok::r_square:
+      SquareBracketLevel--;
+      break;
+    case tok::less:
+      // If not nested, treat as opening angle bracket.
+      if (ParenLevel == 0 && SquareBracketLevel == 0 && BraceLevel == 0)
+        AngleBracketLevel++;
+      break;
+    case tok::greater:
+      // Per C++ 17 Draft N4659, Section 17.2/3
+      //   https://timsong-cpp.github.io/cppwp/n4659/temp.names#3:
+      // "When parsing a template-argument-list, the first non-nested > is
+      // taken as the ending delimiter rather than a greater-than operator."
+      // If not nested, treat as closing angle bracket.
+      if (ParenLevel == 0 && SquareBracketLevel == 0 && BraceLevel == 0)
+        AngleBracketLevel--;
+      break;
     case tok::comma:
-      if (ParenLevel == 0) {
-        // If there is comma and we are not between open parenthesis then it is
-        // two or more declarations in this chain.
+      if (ParenLevel == 0 && SquareBracketLevel == 0 && BraceLevel == 0 &&
+          AngleBracketLevel == 0) {
+        // If there is comma not nested then it is two or more declarations in this chain.
         return false;
       }
       break;
@@ -88,8 +118,7 @@
   if (StartLoc.isMacroID() && IgnoreMacros)
     return;
 
-  auto Diag =
-      diag(StartLoc, "use 'using' instead of 'typedef'");
+  auto Diag = diag(StartLoc, "use 'using' instead of 'typedef'");
 
   // do not fix if there is macro or array
   if (MatchedDecl->getUnderlyingType()->isArrayType() || StartLoc.isMacroID())
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to