Author: alexfh Date: Mon Jun 11 05:46:48 2018 New Revision: 334400 URL: http://llvm.org/viewvc/llvm-project?rev=334400&view=rev Log: Add support for arrays in performance-implicit-conversion-in-loop
Summary: Add support for arrays (and structure that use naked pointers for their iterator, like std::array) in performance-implicit-conversion-in-loop Reviewers: alexfh Reviewed By: alexfh Subscribers: cfe-commits Patch by Alex Pilkiewicz. Differential Revision: https://reviews.llvm.org/D47945 Modified: clang-tools-extra/trunk/clang-tidy/performance/ImplicitConversionInLoopCheck.cpp clang-tools-extra/trunk/clang-tidy/performance/ImplicitConversionInLoopCheck.h clang-tools-extra/trunk/test/clang-tidy/performance-implicit-conversion-in-loop.cpp Modified: clang-tools-extra/trunk/clang-tidy/performance/ImplicitConversionInLoopCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/ImplicitConversionInLoopCheck.cpp?rev=334400&r1=334399&r2=334400&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/performance/ImplicitConversionInLoopCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/performance/ImplicitConversionInLoopCheck.cpp Mon Jun 11 05:46:48 2018 @@ -28,7 +28,7 @@ namespace performance { static bool IsNonTrivialImplicitCast(const Stmt *ST) { if (const auto *ICE = dyn_cast<ImplicitCastExpr>(ST)) { return (ICE->getCastKind() != CK_NoOp) || - IsNonTrivialImplicitCast(ICE->getSubExpr()); + IsNonTrivialImplicitCast(ICE->getSubExpr()); } return false; } @@ -39,7 +39,9 @@ void ImplicitConversionInLoopCheck::regi // conversion. The check on the implicit conversion is done in check() because // we can't access implicit conversion subnode via matchers: has() skips casts // and materialize! We also bind on the call to operator* to get the proper - // type in the diagnostic message. + // type in the diagnostic message. We use both cxxOperatorCallExpr for user + // defined operator and unaryOperator when the iterator is a pointer, like + // for arrays or std::array. // // Note that when the implicit conversion is done through a user defined // conversion operator, the node is a CXXMemberCallExpr, not a @@ -47,10 +49,14 @@ void ImplicitConversionInLoopCheck::regi // cxxOperatorCallExpr() matcher. Finder->addMatcher( cxxForRangeStmt(hasLoopVariable( - varDecl(hasType(qualType(references(qualType(isConstQualified())))), - hasInitializer(expr(hasDescendant(cxxOperatorCallExpr().bind( - "operator-call"))) - .bind("init"))) + varDecl( + hasType(qualType(references(qualType(isConstQualified())))), + hasInitializer( + expr(anyOf(hasDescendant( + cxxOperatorCallExpr().bind("operator-call")), + hasDescendant(unaryOperator(hasOperatorName("*")) + .bind("operator-call")))) + .bind("init"))) .bind("faulty-var"))), this); } @@ -60,7 +66,7 @@ void ImplicitConversionInLoopCheck::chec const auto *VD = Result.Nodes.getNodeAs<VarDecl>("faulty-var"); const auto *Init = Result.Nodes.getNodeAs<Expr>("init"); const auto *OperatorCall = - Result.Nodes.getNodeAs<CXXOperatorCallExpr>("operator-call"); + Result.Nodes.getNodeAs<Expr>("operator-call"); if (const auto *Cleanup = dyn_cast<ExprWithCleanups>(Init)) Init = Cleanup->getSubExpr(); @@ -79,7 +85,7 @@ void ImplicitConversionInLoopCheck::chec void ImplicitConversionInLoopCheck::ReportAndFix( const ASTContext *Context, const VarDecl *VD, - const CXXOperatorCallExpr *OperatorCall) { + const Expr *OperatorCall) { // We only match on const ref, so we should print a const ref version of the // type. QualType ConstType = OperatorCall->getType().withConst(); Modified: clang-tools-extra/trunk/clang-tidy/performance/ImplicitConversionInLoopCheck.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/ImplicitConversionInLoopCheck.h?rev=334400&r1=334399&r2=334400&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/performance/ImplicitConversionInLoopCheck.h (original) +++ clang-tools-extra/trunk/clang-tidy/performance/ImplicitConversionInLoopCheck.h Mon Jun 11 05:46:48 2018 @@ -28,7 +28,7 @@ public: private: void ReportAndFix(const ASTContext *Context, const VarDecl *VD, - const CXXOperatorCallExpr *OperatorCall); + const Expr *OperatorCall); }; } // namespace performance Modified: clang-tools-extra/trunk/test/clang-tidy/performance-implicit-conversion-in-loop.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/performance-implicit-conversion-in-loop.cpp?rev=334400&r1=334399&r2=334400&view=diff ============================================================================== --- clang-tools-extra/trunk/test/clang-tidy/performance-implicit-conversion-in-loop.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/performance-implicit-conversion-in-loop.cpp Mon Jun 11 05:46:48 2018 @@ -40,7 +40,7 @@ class ImplicitWrapper { template <typename T> class OperatorWrapper { public: - explicit OperatorWrapper(const T& t); + OperatorWrapper() = delete; }; struct SimpleClass { @@ -101,7 +101,7 @@ void ImplicitSimpleClassIterator() { // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the loop variable 'foo' is different from the one returned by the iterator and generates an implicit conversion; you can either change the type to the matching one ('const SimpleClass &' but 'const auto&' is always a valid option) or remove the reference to make it explicit that you are creating a new value [performance-implicit-conversion-in-loop] // for (ImplicitWrapper<SimpleClass>& foo : SimpleView()) {} for (const ImplicitWrapper<SimpleClass> foo : SimpleView()) {} - for (ImplicitWrapper<SimpleClass>foo : SimpleView()) {} + for (ImplicitWrapper<SimpleClass> foo : SimpleView()) {} } void ImplicitSimpleClassRefIterator() { @@ -109,7 +109,16 @@ void ImplicitSimpleClassRefIterator() { // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const SimpleClass &'.*}} // for (ImplicitWrapper<SimpleClass>& foo : SimpleRefView()) {} for (const ImplicitWrapper<SimpleClass> foo : SimpleRefView()) {} - for (ImplicitWrapper<SimpleClass>foo : SimpleRefView()) {} + for (ImplicitWrapper<SimpleClass> foo : SimpleRefView()) {} +} + +void ImplicitSimpleClassArray() { + SimpleClass array[5]; + for (const ImplicitWrapper<SimpleClass>& foo : array) {} + // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const SimpleClass &'.*}} + // for (ImplicitWrapper<SimpleClass>& foo : array) {} + for (const ImplicitWrapper<SimpleClass> foo : array) {} + for (ImplicitWrapper<SimpleClass> foo : array) {} } void ImplicitComplexClassIterator() { @@ -117,15 +126,24 @@ void ImplicitComplexClassIterator() { // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const ComplexClass &'.*}} // for (ImplicitWrapper<ComplexClass>& foo : ComplexView()) {} for (const ImplicitWrapper<ComplexClass> foo : ComplexView()) {} - for (ImplicitWrapper<ComplexClass>foo : ComplexView()) {} + for (ImplicitWrapper<ComplexClass> foo : ComplexView()) {} } void ImplicitComplexClassRefIterator() { + ComplexClass array[5]; + for (const ImplicitWrapper<ComplexClass>& foo : array) {} + // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const ComplexClass &'.*}} + // for (ImplicitWrapper<ComplexClass>& foo : array) {} + for (const ImplicitWrapper<ComplexClass> foo : array) {} + for (ImplicitWrapper<ComplexClass> foo : array) {} +} + +void ImplicitComplexClassArray() { for (const ImplicitWrapper<ComplexClass>& foo : ComplexRefView()) {} // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const ComplexClass &'.*}} // for (ImplicitWrapper<ComplexClass>& foo : ComplexRefView()) {} for (const ImplicitWrapper<ComplexClass> foo : ComplexRefView()) {} - for (ImplicitWrapper<ComplexClass>foo : ComplexRefView()) {} + for (ImplicitWrapper<ComplexClass> foo : ComplexRefView()) {} } void OperatorSimpleClassIterator() { @@ -133,7 +151,7 @@ void OperatorSimpleClassIterator() { // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const SimpleClass &'.*}} // for (OperatorWrapper<SimpleClass>& foo : SimpleView()) {} for (const OperatorWrapper<SimpleClass> foo : SimpleView()) {} - for (OperatorWrapper<SimpleClass>foo : SimpleView()) {} + for (OperatorWrapper<SimpleClass> foo : SimpleView()) {} } void OperatorSimpleClassRefIterator() { @@ -141,7 +159,16 @@ void OperatorSimpleClassRefIterator() { // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const SimpleClass &'.*}} // for (OperatorWrapper<SimpleClass>& foo : SimpleRefView()) {} for (const OperatorWrapper<SimpleClass> foo : SimpleRefView()) {} - for (OperatorWrapper<SimpleClass>foo : SimpleRefView()) {} + for (OperatorWrapper<SimpleClass> foo : SimpleRefView()) {} +} + +void OperatorSimpleClassArray() { + SimpleClass array[5]; + for (const OperatorWrapper<SimpleClass>& foo : array) {} + // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const SimpleClass &'.*}} + // for (OperatorWrapper<SimpleClass>& foo : array) {} + for (const OperatorWrapper<SimpleClass> foo : array) {} + for (OperatorWrapper<SimpleClass> foo : array) {} } void OperatorComplexClassIterator() { @@ -149,7 +176,7 @@ void OperatorComplexClassIterator() { // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const ComplexClass &'.*}} // for (OperatorWrapper<ComplexClass>& foo : ComplexView()) {} for (const OperatorWrapper<ComplexClass> foo : ComplexView()) {} - for (OperatorWrapper<ComplexClass>foo : ComplexView()) {} + for (OperatorWrapper<ComplexClass> foo : ComplexView()) {} } void OperatorComplexClassRefIterator() { @@ -157,5 +184,14 @@ void OperatorComplexClassRefIterator() { // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const ComplexClass &'.*}} // for (OperatorWrapper<ComplexClass>& foo : ComplexRefView()) {} for (const OperatorWrapper<ComplexClass> foo : ComplexRefView()) {} - for (OperatorWrapper<ComplexClass>foo : ComplexRefView()) {} + for (OperatorWrapper<ComplexClass> foo : ComplexRefView()) {} +} + +void OperatorComplexClassArray() { + ComplexClass array[5]; + for (const OperatorWrapper<ComplexClass>& foo : array) {} + // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const ComplexClass &'.*}} + // for (OperatorWrapper<ComplexClass>& foo : array) {} + for (const OperatorWrapper<ComplexClass> foo : array) {} + for (OperatorWrapper<ComplexClass> foo : array) {} } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits