[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

Torbjörn Klatt via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun May 12 12:50:13 PDT 2019


torbjoernk added inline comments.


================
Comment at: clang-tools-extra/test/clang-tidy/modernize-loop-convert-basic.cpp:273
 
+  for (S::const_iterator It = Ss.begin(), E = Ss.end(); It != E; ++It) {
+    printf("s has value %d\n", (*It).X);
----------------
hintonda wrote:
> I think you need to fix the comment and add checks to these also, which is what the `if else` was dealing with:
> 
>    434    // V.begin() returns a user-defined type 'iterator' which, since it's
>    435    // different from const_iterator, disqualifies these loops from
>    436    // transformation.
>    437    dependent<int> V;
>    438    for (dependent<int>::const_iterator It = V.begin(), E = V.end();
>    439         It != E; ++It) {
>    440      printf("Fibonacci number is %d\n", *It);
>    441    }
>    442
>    443    for (dependent<int>::const_iterator It(V.begin()), E = V.end();
>    444         It != E; ++It) {
>    445      printf("Fibonacci number is %d\n", *It);
>    446    }
>    447  }
> 
This seems weird and I reckon my setup is broken as also a bunch of other lit tests (make check-clang-tools) fail.

modernize-loop-convert-basic fails once I change this to the following:

```
  dependent<int> V;
  for (dependent<int>::const_iterator It = V.begin(), E = V.end();
       It != E; ++It) {
    printf("Fibonacci number is %d\n", *It);
  }
  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
  // CHECK-FIXES: for (int It : V)
  // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", It);

  for (dependent<int>::const_iterator It(V.begin()), E = V.end();
       It != E; ++It) {
    printf("Fibonacci number is %d\n", *It);
  }
  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
  // CHECK-FIXES: for (int It : V)
  // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", It);
```

However, copy&pasted outsite of llvm-lit in a minimal example, it is detected and fixed as expected.

Is it possible that llvm-lit picks up my (older) globally available clang-tidy?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61827





More information about the cfe-commits mailing list