[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]

zinovy.nis added inline comments.


================
Comment at: clang-tidy/modernize-use-auto-min-type-name-length.cpp:61-83
+    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 = {{.*}}
----------------
zinovy.nis wrote:
> zinovy.nis wrote:
> > alexfh wrote:
> > > These tests could be more useful, if they verified boundary values of the MinTypeNameLength, e.g. that `long int` is replaced with `auto` when the option is set to 8 and it stays `long int`with the option set to 9.
> > > 
> > > Please also add tests with template typenames: e.g. the lenght of `T <  int  >` should be considered 6 and the length of `T  <  const int >` is 12. I guess, the algorithm I proposed will be incorrect for pointer type arguments of templates (the length of `T<int*>` should be 7 regardless of `RemoveStars`), but this can be fixed by conditionally (on RemoveStars) trimming `*`s from the right of the type name and then treating them as punctuation. So instead of
> > > 
> > > ```
> > >   for (const unsigned char C : tooling::fixit::getText(SR, Context)) {
> > >     const CharType NextChar =
> > >         std::isalnum(C)
> > >             ? Alpha
> > >             : (std::isspace(C) || (!RemoveStars && C == '*')) ? Space
> > >                                                               : Punctuation;
> > > ```
> > > 
> > > you could use something similar to
> > > 
> > > ```
> > >   StringRef Text = tooling::fixit::getText(SR, Context);
> > >   if (RemoveStars)
> > >     Text = Text.rtrim(" \t\v\n\r*");
> > >   for (const unsigned char C : Text) {
> > >     const CharType NextChar =
> > >         std::isalnum(C) ? Alpha : std::isspace(C) ? Space : Punctuation;
> > > ```
> > > These tests could be more useful, if they verified boundary values of the MinTypeNameLength, e.g. that long int is replaced with auto when the option is set to 8 and it stays long intwith the option set to 9.
> > > 
> > 
> > `int`-test is just for that :-)
> > 
> > ```
> > int a = static_cast<int>(foo());                 // ---> int  a = ...
> > ```
> I measured lengths for template cases:
> 
> 
> ```
> S=std::string *   len=12
> S=std::vector<std::string> *   len=25
> S=std::vector<const std::string> *   len=31
> S=std::string        *   len=12
> S=std::vector<std::string>       *   len=25
> S=std::vector<const std::string>    *   len=31
> ```
> 
> 
RemoveStars==1 here.


https://reviews.llvm.org/D45927