Author: shuaiwang Date: Tue Jul 10 15:51:06 2018 New Revision: 336737 URL: http://llvm.org/viewvc/llvm-project?rev=336737&view=rev Log: Use ExprMutationAnalyzer in performance-for-range-copy
Summary: This gives better coverage to the check as ExprMutationAnalyzer is more accurate comparing to isOnlyUsedAsConst. Majority of wins come from const usage of member field, e.g.: for (auto widget : container) { // copy of loop variable if (widget.type == BUTTON) { // const usage only recognized by ExprMutationAnalyzer // ... } } Reviewers: george.karpenkov Subscribers: a.sidorin, cfe-commits Differential Revision: https://reviews.llvm.org/D48854 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=336737&r1=336736&r2=336737&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp Tue Jul 10 15:51:06 2018 @@ -8,7 +8,7 @@ //===----------------------------------------------------------------------===// #include "ForRangeCopyCheck.h" -#include "../utils/DeclRefExprUtils.h" +#include "../utils/ExprMutationAnalyzer.h" #include "../utils/FixItHintUtils.h" #include "../utils/TypeTraits.h" @@ -79,8 +79,8 @@ bool ForRangeCopyCheck::handleCopyIsOnly utils::type_traits::isExpensiveToCopy(LoopVar.getType(), Context); if (LoopVar.getType().isConstQualified() || !Expensive || !*Expensive) return false; - if (!utils::decl_ref_expr::isOnlyUsedAsConst(LoopVar, *ForRange.getBody(), - Context)) + if (utils::ExprMutationAnalyzer(ForRange.getBody(), &Context) + .isMutated(&LoopVar)) return false; diag(LoopVar.getLocation(), "loop variable is copied but only used as const reference; consider " 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=336737&r1=336736&r2=336737&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 Tue Jul 10 15:51:06 2018 @@ -117,6 +117,11 @@ struct Mutable { ~Mutable() {} }; +struct Point { + ~Point() {} + int x, y; +}; + Mutable& operator<<(Mutable &Out, bool B) { Out.setBool(B); return Out; @@ -214,6 +219,15 @@ void positiveOnlyUsedAsConstArguments() } } +void positiveOnlyAccessedFieldAsConst() { + for (auto UsedAsConst : View<Iterator<Point>>()) { + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy] + // CHECK-FIXES: for (const auto& UsedAsConst : View<Iterator<Point>>()) { + use(UsedAsConst.x); + use(UsedAsConst.y); + } +} + void positiveOnlyUsedInCopyConstructor() { for (auto A : View<Iterator<Mutable>>()) { // CHECK-MESSAGES: [[@LINE-1]]:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy] _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits