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);
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);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
