alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.
================
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 = {{.*}}
----------------
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;
```
https://reviews.llvm.org/D45927
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
