diff options
author | Haojian Wu <hokein@google.com> | 2018-08-10 08:25:51 +0000 |
---|---|---|
committer | Haojian Wu <hokein@google.com> | 2018-08-10 08:25:51 +0000 |
commit | 46c3aff567f62842fe6253061630d1075ffc0e94 (patch) | |
tree | 68cf7e632362fcdf51cf28887095dbe0677196de /clang-tidy/performance | |
parent | a32ea61ae09dc772fd7f688a89a0bd07c1bcc4f1 (diff) |
[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
git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@339415 91177308-0d34-0410-b5e6-96231b3b80d8
Diffstat (limited to 'clang-tidy/performance')
-rw-r--r-- | clang-tidy/performance/ForRangeCopyCheck.cpp | 31 |
1 files changed, 22 insertions, 9 deletions
diff --git a/clang-tidy/performance/ForRangeCopyCheck.cpp b/clang-tidy/performance/ForRangeCopyCheck.cpp index c8c90638..867d95d8 100644 --- a/clang-tidy/performance/ForRangeCopyCheck.cpp +++ b/clang-tidy/performance/ForRangeCopyCheck.cpp @@ -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::handleCopyIsOnlyConstReferenced( 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 |