[PATCH] D45927: [clang-tidy] [modernize-use-auto] Correct way to calculate a type name length for multi-token types

Zinovy Nis via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 30 08:31:10 PDT 2018


zinovy.nis added inline comments.


================
Comment at: clang-tidy/modernize/UseAutoCheck.cpp:33
+
+bool IsNotSpace(const char& C) {
+  return !std::isspace(static_cast<unsigned char>(C));
----------------
alexfh wrote:
> Why `const char&` and not just `char`? Moreover, these two functions can be replaced with lambdas. See below.
Agree.


================
Comment at: clang-tidy/modernize/UseAutoCheck.cpp:445-449
+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);
+}
----------------
alexfh wrote:
> ```
> static size_t GetTypeNameLength(const TypeLoc &Loc, const ASTContext &Context, bool IgnoreStars) {
>   const StringRef S = tooling::fixit::getText(Loc.getSourceRange(), Context);
>   if (IgnoreStars)
>     return llvm::count_if(S, [] (char C) { return std::isspace(C) || C == '*'; });
>   return llvm::count_if(S, [] (char C) { return std::isspace(C); });
> }
> ```
`IgnoreStars` is initialized once in the ctor and is used widely for all the literals in the translation unit.
IMHO it's better to eliminate branches on `IgnoreStars`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45927





More information about the cfe-commits mailing list