zinovy.nis created this revision.
zinovy.nis added reviewers: malcolm.parsons, alexfh.
zinovy.nis added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, xazax.hun.

This patch is a fix for https://reviews.llvm.org/D45405 where spaces were also 
considered as a part of a type name. So length("int *") was 5 instead of 3 with 
RemoveStars=0 or 4 with RemoveStars=1.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45927

Files:
  clang-tidy/modernize/UseAutoCheck.cpp
  clang-tidy/modernize/UseAutoCheck.h
  docs/clang-tidy/checks/modernize-use-auto.rst
  test/clang-tidy/modernize-use-auto-min-type-name-length.cpp

Index: test/clang-tidy/modernize-use-auto-min-type-name-length.cpp
===================================================================
--- test/clang-tidy/modernize-use-auto-min-type-name-length.cpp
+++ test/clang-tidy/modernize-use-auto-min-type-name-length.cpp
@@ -1,29 +1,82 @@
-// RUN: %check_clang_tidy %s modernize-use-auto %t -- \
-// RUN:   -config="{CheckOptions: [{key: modernize-use-auto.MinTypeNameLength, value: '5'}]}" \
-// RUN:   -- -std=c++11 -frtti
+// RUN: %check_clang_tidy -check-suffix=0-0 %s modernize-use-auto %t  -- -config="{CheckOptions: [{key: modernize-use-auto.RemoveStars, value: 0}, {key: modernize-use-auto.MinTypeNameLength, value: 0}]}" -- --std=c++11 -frtti
+// RUN: %check_clang_tidy -check-suffix=0-5 %s modernize-use-auto %t  -- -config="{CheckOptions: [{key: modernize-use-auto.RemoveStars, value: 0}, {key: modernize-use-auto.MinTypeNameLength, value: 5}]}" -- --std=c++11 -frtti
+// RUN: %check_clang_tidy -check-suffix=1-0 %s modernize-use-auto %t  -- -config="{CheckOptions: [{key: modernize-use-auto.RemoveStars, value: 1}, {key: modernize-use-auto.MinTypeNameLength, value: 0}]}" -- --std=c++11 -frtti
+// RUN: %check_clang_tidy -check-suffix=1-5 %s modernize-use-auto %t  -- -config="{CheckOptions: [{key: modernize-use-auto.RemoveStars, value: 1}, {key: modernize-use-auto.MinTypeNameLength, value: 5}]}" -- --std=c++11 -frtti
 
-extern int foo();
-
-using VeryVeryVeryLongTypeName = int;
+template <class T>
+extern T foo();
 
 int bar() {
-  int a = static_cast<VeryVeryVeryLongTypeName>(foo());
-  // strlen('int') = 4 <  5, so skip it,
-  // even strlen('VeryVeryVeryLongTypeName') > 5.
-
-  unsigned b = static_cast<unsigned>(foo());
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name [modernize-use-auto]
-  // CHECK-FIXES: auto b = static_cast<unsigned>(foo());
-
-  bool c = static_cast<bool>(foo());
-  // strlen('bool') = 4 <  5, so skip it.
-
-  const bool c1 = static_cast<const bool>(foo());
-  // strlen('bool') = 4 <  5, so skip it, even there's a 'const'.
-
-  unsigned long long ull = static_cast<unsigned long long>(foo());
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name [modernize-use-auto]
-  // CHECK-FIXES: auto ull = static_cast<unsigned long long>(foo());
+  {
+    using VeryVeryVeryLongTypeName = int;
+    int i = static_cast<VeryVeryVeryLongTypeName>(foo<int>());
+    // CHECK-FIXES-0-0: auto i = {{.*}}
+    // CHECK-FIXES-0-5: int i = {{.*}}
+    // CHECK-FIXES-1-0: auto  i = {{.*}}
+    // CHECK-FIXES-1-5: int i = {{.*}}
+    int *pi = static_cast<VeryVeryVeryLongTypeName *>(foo<int*>());
+    // CHECK-FIXES-0-0: auto *pi = {{.*}}
+    // CHECK-FIXES-0-5: int *pi = {{.*}}
+    // CHECK-FIXES-1-0: auto pi = {{.*}}
+    // CHECK-FIXES-1-5: int *pi = {{.*}}
+    int **ppi = static_cast<VeryVeryVeryLongTypeName **>(foo<int**>());
+    // CHECK-FIXES-0-0: auto **ppi = {{.*}}
+    // CHECK-FIXES-0-5: int **ppi = {{.*}}
+    // CHECK-FIXES-1-0: auto ppi = {{.*}}
+    // CHECK-FIXES-1-5: auto ppi = {{.*}}
+    }
+
+  {
+    bool b = static_cast<bool>(foo<bool>());
+    // CHECK-FIXES-0-0: auto b = {{.*}}
+    // CHECK-FIXES-0-5: bool b = {{.*}}
+    // CHECK-FIXES-1-0: auto  b = {{.*}}
+    // CHECK-FIXES-1-5: bool b = {{.*}}
+    bool *pb = static_cast<bool *>(foo<bool*>());
+    // CHECK-FIXES-0-0: auto *pb = {{.*}}
+    // CHECK-FIXES-0-5: bool *pb = {{.*}}
+    // CHECK-FIXES-1-0: auto pb = {{.*}}
+    // CHECK-FIXES-1-5: auto pb = {{.*}}
+  }
+
+  {
+    const bool cb = static_cast<const bool>(foo<const bool>());
+    // CHECK-FIXES-0-0: auto cb = {{.*}}
+    // CHECK-FIXES-0-5: bool cb = {{.*}}
+    // CHECK-FIXES-1-0: auto  cb = {{.*}}
+    // CHECK-FIXES-1-5: bool cb = {{.*}}
+    const bool *pcb = static_cast<const bool *>(foo<const bool*>());
+    // CHECK-FIXES-0-0: const auto *pcb = {{.*}}
+    // CHECK-FIXES-0-5: const bool *pcb = {{.*}}
+    // CHECK-FIXES-1-0: const auto pcb = {{.*}}
+    // CHECK-FIXES-1-5: const auto pcb = {{.*}}
+  }
+
+  {
+    long int li = static_cast<long int>(foo<long int>());
+    // CHECK-FIXES-0-0: auto li = {{.*}}
+    // CHECK-FIXES-0-5: auto li = {{.*}}
+    // CHECK-FIXES-1-0: auto  li = {{.*}}
+    // CHECK-FIXES-1-5: auto  li = {{.*}}
+    long int *pli = static_cast<long int *>(foo<long int*>());
+    // CHECK-FIXES-0-0: auto *pli = {{.*}}
+    // CHECK-FIXES-0-5: auto *pli = {{.*}}
+    // CHECK-FIXES-1-0: auto pli = {{.*}}
+    // CHECK-FIXES-1-5: auto pli = {{.*}}
+  }
+
+  {
+    unsigned long long ull = static_cast<unsigned long long>(foo<unsigned long long>());
+    // CHECK-FIXES-0-0: auto ull = {{.*}}
+    // CHECK-FIXES-0-5: auto ull = {{.*}}
+    // CHECK-FIXES-1-0: auto  ull = {{.*}}
+    // CHECK-FIXES-1-5: auto  ull = {{.*}}
+    unsigned long long *pull = static_cast<unsigned long long *>(foo<unsigned long long*>());
+    // CHECK-FIXES-0-0: auto *pull = {{.*}}
+    // CHECK-FIXES-0-5: auto *pull = {{.*}}
+    // CHECK-FIXES-1-0: auto pull = {{.*}}
+    // CHECK-FIXES-1-5: auto pull = {{.*}}
+  }
 
   return 1;
 }
Index: docs/clang-tidy/checks/modernize-use-auto.rst
===================================================================
--- docs/clang-tidy/checks/modernize-use-auto.rst
+++ docs/clang-tidy/checks/modernize-use-auto.rst
@@ -182,20 +182,38 @@
    If the option is set to non-zero (default `5`), the check will ignore type
    names having a length less than the option value. The option affects
    expressions only, not iterators.
+   If ``RemoveStars`` option (see below) is set to non-zero, then ``*s`` in 
+   the type are also counted as a part of the type name.
 
 .. code-block:: c++
 
-  // MinTypeNameLength = 0
+  // MinTypeNameLength = 0, RemoveStars=0
 
   int a = static_cast<int>(foo());            // ---> auto a = ...
   bool b = new bool;                          // ---> auto b = ...
   unsigned c = static_cast<unsigned>(foo());  // ---> auto c = ...
 
-  // MinTypeNameLength = 8
+  // MinTypeNameLength = 5, RemoveStars=0
 
-  int a = static_cast<int>(foo());            // ---> int  a = ...
-  bool b = new bool;                          // ---> bool b = ...
-  unsigned c = static_cast<unsigned>(foo());  // ---> auto c = ...
+  int a = static_cast<int>(foo());                 // ---> int  a = ...
+  bool b = static_cast<bool>(foo());               // ---> bool b = ...
+  // length(bool *)=4 when RemoveStars is 0
+  bool *pb = static_cast<bool*>(foo());            // ---> bool *pb = ...
+  unsigned c = static_cast<unsigned>(foo());       // ---> auto c = ...
+  // length(long int) = 4 + 3 = 7
+  long int d = static_cast<unsigned int>(foo());   // ---> auto d = ...
+
+  // MinTypeNameLength = 5, RemoveStars=1
+
+  int a = static_cast<int>(foo());                 // ---> int  a = ...
+  // length(int **)=5 when RemoveStars is 1
+  int **pa = static_cast<int**>(foo());            // ---> auto pa = ...
+  bool b = static_cast<bool>(foo());               // ---> bool b = ...
+  // length(bool *)=5 when RemoveStars is 1
+  bool *pb = static_cast<bool*>(foo());            // ---> auto pb = ...
+  unsigned c = static_cast<unsigned>(foo());       // ---> auto c = ...
+  // length(long int) = 4 + 3 = 7
+  long int d = static_cast<unsigned int>(foo());   // ---> auto d = ...
 
 .. option:: RemoveStars
 
Index: clang-tidy/modernize/UseAutoCheck.h
===================================================================
--- clang-tidy/modernize/UseAutoCheck.h
+++ clang-tidy/modernize/UseAutoCheck.h
@@ -28,9 +28,12 @@
   void replaceExpr(const DeclStmt *D, ASTContext *Context,
                    llvm::function_ref<QualType(const Expr *)> GetType,
                    StringRef Message);
+  size_t GetTypeNameLength(const SourceRange &SR, const ASTContext &Context);
 
   const unsigned int MinTypeNameLength;
   const bool RemoveStars;
+
+  std::function<bool(const char&)> SpacePredicate;
 };
 
 } // namespace modernize
Index: clang-tidy/modernize/UseAutoCheck.cpp
===================================================================
--- clang-tidy/modernize/UseAutoCheck.cpp
+++ clang-tidy/modernize/UseAutoCheck.cpp
@@ -12,6 +12,8 @@
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/FixIt.h"
+#include <algorithm>
+#include <cctype>
 
 using namespace clang;
 using namespace clang::ast_matchers;
@@ -27,6 +29,13 @@
 const char DeclWithCastId[] = "decl_cast";
 const char DeclWithTemplateCastId[] = "decl_template";
 
+
+bool IsNotSpace(const char& C) {
+  return !std::isspace(static_cast<unsigned char>(C));
+}
+
+bool IsNotSpaceOrStar(const char &C) { return C != '*' && IsNotSpace(C); }
+
 /// \brief Matches variable declarations that have explicit initializers that
 /// are not initializer lists.
 ///
@@ -289,7 +298,8 @@
 UseAutoCheck::UseAutoCheck(StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       MinTypeNameLength(Options.get("MinTypeNameLength", 5)),
-      RemoveStars(Options.get("RemoveStars", 0)) {}
+      RemoveStars(Options.get("RemoveStars", 0)),
+      SpacePredicate(RemoveStars ? IsNotSpace : IsNotSpaceOrStar) {}
 
 void UseAutoCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "MinTypeNameLength", MinTypeNameLength);
@@ -417,10 +427,9 @@
     Loc = Loc.getNextTypeLoc();
   }
   SourceRange Range(Loc.getSourceRange());
-
   if (MinTypeNameLength != 0 &&
-      tooling::fixit::getText(Loc.getSourceRange(), FirstDecl->getASTContext())
-              .size() < MinTypeNameLength)
+      GetTypeNameLength(Loc.getSourceRange(), FirstDecl->getASTContext()
+                        ) < MinTypeNameLength)
     return;
 
   auto Diag = diag(Range.getBegin(), Message);
@@ -433,6 +442,12 @@
        << StarRemovals;
 }
 
+size_t UseAutoCheck::GetTypeNameLength(const SourceRange &SR,
+                                       const ASTContext &Context) {
+  const StringRef S = tooling::fixit::getText(SR, Context);
+  return std::count_if(S.begin(), S.end(), SpacePredicate);
+}
+
 void UseAutoCheck::check(const MatchFinder::MatchResult &Result) {
   if (const auto *Decl = Result.Nodes.getNodeAs<DeclStmt>(IteratorDeclStmtId)) {
     replaceIterators(Decl, Result.Context);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to