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

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

I pulled down you patch, compiled and ran it.  Once I fixed the two problems I mentioned, it ran clean, e.g.:

  $ bin/llvm-lit -vv ../../llvm-project/clang-tools-extra/test/clang-tidy/modernize-loop-convert-[be]*.cpp
  -- Testing: 2 tests, 2 threads --
  PASS: Clang Tools :: clang-tidy/modernize-loop-convert-basic.cpp (1 of 2)
  PASS: Clang Tools :: clang-tidy/modernize-loop-convert-extra.cpp (2 of 2)
  Testing Time: 0.93s
    Expected Passes    : 2

Also, I don't think the omp issue is a regression, so I wouldn't worry about trying to fix it here.



================
Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:131
   DeclarationMatcher InitDeclMatcher =
-      varDecl(hasInitializer(anyOf(ignoringParenImpCasts(BeginCallMatcher),
-                                   materializeTemporaryExpr(
----------------
This change is incorrect.  The matcher actually needs all three cases.  This explains the strange test failures you were seeing.


================
Comment at: clang-tools-extra/test/clang-tidy/modernize-loop-convert-basic.cpp:274
+  for (S::const_iterator It = Ss.begin(), E = Ss.end(); It != E; ++It) {
+    printf("s has value %d\n", (*It).X);
+  }
----------------
Instead of adding a new test case, please fix the two I mentioned earlier, and 5 more in `modernize-loop-convert-extra.cpp`.  They just need you to add the appropriate CHECK line.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61827