[PATCH] D79440: Fix ForRangeCopyCheck not triggering on iterators returning elements by value in C++17.

[Dmitri Gribenko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3adaa97f0157: Fix ForRangeCopyCheck not triggering on iterators returning elements by value… (authored by gribozavr).

Changed prior to commit:
  https://reviews.llvm.org/D79440?vs=262202&id=262307#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79440

Files:
  clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy.cpp
@@ -28,6 +28,7 @@
 };
 template <typename T>
 struct View {
+  View() = default;
   T begin() { return T(); }
   T begin() const { return T(); }
   T end() { return T(); }
@@ -270,3 +271,28 @@
   for (auto _ : View<Iterator<S>>()) {
   }
 }
+
+template <typename T>
+struct ValueReturningIterator {
+  void operator++() {}
+  T operator*() { return T(); }
+  bool operator!=(const ValueReturningIterator &) { return false; }
+  typedef const T &const_reference;
+};
+
+void negativeValueIterator() {
+  // Check does not trigger for iterators that return elements by value.
+  for (const S SS : View<ValueReturningIterator<S>>()) {
+  }
+}
+
+View<Iterator<S>> createView(S) { return View<Iterator<S>>(); }
+
+void positiveValueIteratorUsedElseWhere() {
+  for (const S SS : createView(*ValueReturningIterator<S>())) {
+    // CHECK-MESSAGES: [[@LINE-1]]:16: warning: the loop variable's type is not
+    // a reference type; this creates a copy in each iteration; consider making
+    // this a reference [performance-for-range-copy] CHECK-FIXES: for (const S&
+    // SS : createView(*ValueReturningIterator<S>())) {
+  }
+}
Index: clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
+++ clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
@@ -37,12 +37,18 @@
   // Match loop variables that are not references or pointers or are already
   // initialized through MaterializeTemporaryExpr which indicates a type
   // conversion.
-  auto LoopVar = varDecl(
-      hasType(qualType(
-          unless(anyOf(hasCanonicalType(anyOf(referenceType(), pointerType())),
-                       hasDeclaration(namedDecl(
-                           matchers::matchesAnyListedName(AllowedTypes))))))),
-      unless(hasInitializer(expr(hasDescendant(materializeTemporaryExpr())))));
+  auto HasReferenceOrPointerTypeOrIsAllowed = hasType(qualType(
+      unless(anyOf(hasCanonicalType(anyOf(referenceType(), pointerType())),
+                   hasDeclaration(namedDecl(
+                       matchers::matchesAnyListedName(AllowedTypes)))))));
+  auto IteratorReturnsValueType = cxxOperatorCallExpr(
+      hasOverloadedOperatorName("*"),
+      callee(
+          cxxMethodDecl(returns(unless(hasCanonicalType(referenceType()))))));
+  auto LoopVar =
+      varDecl(HasReferenceOrPointerTypeOrIsAllowed,
+              unless(hasInitializer(expr(hasDescendant(expr(anyOf(
+                  materializeTemporaryExpr(), IteratorReturnsValueType)))))));
   Finder->addMatcher(cxxForRangeStmt(hasLoopVariable(LoopVar.bind("loopVar")))
                          .bind("forRange"),
                      this);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D79440.262307.patch
Type: text/x-patch
Size: 3065 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200506/a480f824/attachment.bin>