Author: hokein
Date: Fri Aug 10 01:25:51 2018
New Revision: 339415
URL: http://llvm.org/viewvc/llvm-project?rev=339415&view=rev
Log:
[clang-tidy] Omit cases where loop variable is not used in loop body in
performance-for-range-copy check.
Summary:
The upstream change r336737 make the check too smart to fix the case
where loop variable could be used as `const auto&`.
But for the case below, changing to `const auto _` will introduce
an unused complier warning.
```
for (auto _ : state) {
// no references for _.
}
```
This patch omit this case, and it is safe to do it as the case is very rare.
Reviewers: ilya-biryukov, alexfh
Subscribers: xazax.hun, cfe-commits
Differential Revision: https://reviews.llvm.org/D50447
Modified:
clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/performance-for-range-copy.cpp
Modified: clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp?rev=339415&r1=339414&r2=339415&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp
(original)
+++ clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp Fri
Aug 10 01:25:51 2018
@@ -8,6 +8,7 @@
//===----------------------------------------------------------------------===//
#include "ForRangeCopyCheck.h"
+#include "../utils/DeclRefExprUtils.h"
#include "../utils/ExprMutationAnalyzer.h"
#include "../utils/FixItHintUtils.h"
#include "../utils/TypeTraits.h"
@@ -79,15 +80,27 @@ bool ForRangeCopyCheck::handleCopyIsOnly
utils::type_traits::isExpensiveToCopy(LoopVar.getType(), Context);
if (LoopVar.getType().isConstQualified() || !Expensive || !*Expensive)
return false;
- if (utils::ExprMutationAnalyzer(ForRange.getBody(), &Context)
- .isMutated(&LoopVar))
- return false;
- diag(LoopVar.getLocation(),
- "loop variable is copied but only used as const reference; consider "
- "making it a const reference")
- << utils::fixit::changeVarDeclToConst(LoopVar)
- << utils::fixit::changeVarDeclToReference(LoopVar, Context);
- return true;
+ // We omit the case where the loop variable is not used in the loop body.
E.g.
+ //
+ // for (auto _ : benchmark_state) {
+ // }
+ //
+ // Because the fix (changing to `const auto &`) will introduce an unused
+ // compiler warning which can't be suppressed.
+ // Since this case is very rare, it is safe to ignore it.
+ if (!utils::ExprMutationAnalyzer(ForRange.getBody(), &Context)
+ .isMutated(&LoopVar) &&
+ !utils::decl_ref_expr::allDeclRefExprs(LoopVar, *ForRange.getBody(),
+ Context)
+ .empty()) {
+ diag(LoopVar.getLocation(),
+ "loop variable is copied but only used as const reference; consider "
+ "making it a const reference")
+ << utils::fixit::changeVarDeclToConst(LoopVar)
+ << utils::fixit::changeVarDeclToReference(LoopVar, Context);
+ return true;
+ }
+ return false;
}
} // namespace performance
Modified: clang-tools-extra/trunk/test/clang-tidy/performance-for-range-copy.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/performance-for-range-copy.cpp?rev=339415&r1=339414&r2=339415&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/performance-for-range-copy.cpp
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/performance-for-range-copy.cpp Fri
Aug 10 01:25:51 2018
@@ -260,3 +260,8 @@ void PositiveConstNonMemberOperatorInvok
bool result = ConstOperatorInvokee != Mutable();
}
}
+
+void IgnoreLoopVariableNotUsedInLoopBody() {
+ for (auto _ : View<Iterator<S>>()) {
+ }
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
