This revision was automatically updated to reflect the committed changes. Closed by commit rL339415: [clang-tidy] Omit cases where loop variable is not used in loop body in (authored by hokein, committed by ). Herald added a subscriber: llvm-commits.
Repository: rL LLVM https://reviews.llvm.org/D50447 Files: clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp clang-tools-extra/trunk/test/clang-tidy/performance-for-range-copy.cpp Index: clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp +++ clang-tools-extra/trunk/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 @@ 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 Index: clang-tools-extra/trunk/test/clang-tidy/performance-for-range-copy.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/performance-for-range-copy.cpp +++ clang-tools-extra/trunk/test/clang-tidy/performance-for-range-copy.cpp @@ -260,3 +260,8 @@ bool result = ConstOperatorInvokee != Mutable(); } } + +void IgnoreLoopVariableNotUsedInLoopBody() { + for (auto _ : View<Iterator<S>>()) { + } +}
Index: clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp +++ clang-tools-extra/trunk/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 @@ 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 Index: clang-tools-extra/trunk/test/clang-tidy/performance-for-range-copy.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/performance-for-range-copy.cpp +++ clang-tools-extra/trunk/test/clang-tidy/performance-for-range-copy.cpp @@ -260,3 +260,8 @@ 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