Index: clang-tools-extra/trunk/clang-tidy/performance/ImplicitConversionInLoopCheck.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/performance/ImplicitConversionInLoopCheck.h +++ clang-tools-extra/trunk/clang-tidy/performance/ImplicitConversionInLoopCheck.h @@ -28,7 +28,7 @@ private: void ReportAndFix(const ASTContext *Context, const VarDecl *VD, - const CXXOperatorCallExpr *OperatorCall); + const Expr *OperatorCall); }; } // namespace performance Index: clang-tools-extra/trunk/clang-tidy/performance/ImplicitConversionInLoopCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/performance/ImplicitConversionInLoopCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/performance/ImplicitConversionInLoopCheck.cpp @@ -28,7 +28,7 @@ static bool IsNonTrivialImplicitCast(const Stmt *ST) { if (const auto *ICE = dyn_cast(ST)) { return (ICE->getCastKind() != CK_NoOp) || - IsNonTrivialImplicitCast(ICE->getSubExpr()); + IsNonTrivialImplicitCast(ICE->getSubExpr()); } return false; } @@ -39,7 +39,9 @@ // 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 @@ // 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 @@ const auto *VD = Result.Nodes.getNodeAs("faulty-var"); const auto *Init = Result.Nodes.getNodeAs("init"); const auto *OperatorCall = - Result.Nodes.getNodeAs("operator-call"); + Result.Nodes.getNodeAs("operator-call"); if (const auto *Cleanup = dyn_cast(Init)) Init = Cleanup->getSubExpr(); @@ -79,7 +85,7 @@ 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(); Index: clang-tools-extra/trunk/test/clang-tidy/performance-implicit-conversion-in-loop.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/performance-implicit-conversion-in-loop.cpp +++ clang-tools-extra/trunk/test/clang-tidy/performance-implicit-conversion-in-loop.cpp @@ -40,7 +40,7 @@ template class OperatorWrapper { public: - explicit OperatorWrapper(const T& t); + OperatorWrapper() = delete; }; struct SimpleClass { @@ -101,7 +101,7 @@ // 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& foo : SimpleView()) {} for (const ImplicitWrapper foo : SimpleView()) {} - for (ImplicitWrapperfoo : SimpleView()) {} + for (ImplicitWrapper foo : SimpleView()) {} } void ImplicitSimpleClassRefIterator() { @@ -109,7 +109,16 @@ // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const SimpleClass &'.*}} // for (ImplicitWrapper& foo : SimpleRefView()) {} for (const ImplicitWrapper foo : SimpleRefView()) {} - for (ImplicitWrapperfoo : SimpleRefView()) {} + for (ImplicitWrapper foo : SimpleRefView()) {} +} + +void ImplicitSimpleClassArray() { + SimpleClass array[5]; + for (const ImplicitWrapper& foo : array) {} + // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const SimpleClass &'.*}} + // for (ImplicitWrapper& foo : array) {} + for (const ImplicitWrapper foo : array) {} + for (ImplicitWrapper foo : array) {} } void ImplicitComplexClassIterator() { @@ -117,15 +126,24 @@ // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const ComplexClass &'.*}} // for (ImplicitWrapper& foo : ComplexView()) {} for (const ImplicitWrapper foo : ComplexView()) {} - for (ImplicitWrapperfoo : ComplexView()) {} + for (ImplicitWrapper foo : ComplexView()) {} } void ImplicitComplexClassRefIterator() { + ComplexClass array[5]; + for (const ImplicitWrapper& foo : array) {} + // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const ComplexClass &'.*}} + // for (ImplicitWrapper& foo : array) {} + for (const ImplicitWrapper foo : array) {} + for (ImplicitWrapper foo : array) {} +} + +void ImplicitComplexClassArray() { for (const ImplicitWrapper& foo : ComplexRefView()) {} // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const ComplexClass &'.*}} // for (ImplicitWrapper& foo : ComplexRefView()) {} for (const ImplicitWrapper foo : ComplexRefView()) {} - for (ImplicitWrapperfoo : ComplexRefView()) {} + for (ImplicitWrapper foo : ComplexRefView()) {} } void OperatorSimpleClassIterator() { @@ -133,7 +151,7 @@ // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const SimpleClass &'.*}} // for (OperatorWrapper& foo : SimpleView()) {} for (const OperatorWrapper foo : SimpleView()) {} - for (OperatorWrapperfoo : SimpleView()) {} + for (OperatorWrapper foo : SimpleView()) {} } void OperatorSimpleClassRefIterator() { @@ -141,7 +159,16 @@ // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const SimpleClass &'.*}} // for (OperatorWrapper& foo : SimpleRefView()) {} for (const OperatorWrapper foo : SimpleRefView()) {} - for (OperatorWrapperfoo : SimpleRefView()) {} + for (OperatorWrapper foo : SimpleRefView()) {} +} + +void OperatorSimpleClassArray() { + SimpleClass array[5]; + for (const OperatorWrapper& foo : array) {} + // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const SimpleClass &'.*}} + // for (OperatorWrapper& foo : array) {} + for (const OperatorWrapper foo : array) {} + for (OperatorWrapper foo : array) {} } void OperatorComplexClassIterator() { @@ -149,7 +176,7 @@ // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const ComplexClass &'.*}} // for (OperatorWrapper& foo : ComplexView()) {} for (const OperatorWrapper foo : ComplexView()) {} - for (OperatorWrapperfoo : ComplexView()) {} + for (OperatorWrapper foo : ComplexView()) {} } void OperatorComplexClassRefIterator() { @@ -157,5 +184,14 @@ // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const ComplexClass &'.*}} // for (OperatorWrapper& foo : ComplexRefView()) {} for (const OperatorWrapper foo : ComplexRefView()) {} - for (OperatorWrapperfoo : ComplexRefView()) {} + for (OperatorWrapper foo : ComplexRefView()) {} +} + +void OperatorComplexClassArray() { + ComplexClass array[5]; + for (const OperatorWrapper& foo : array) {} + // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const ComplexClass &'.*}} + // for (OperatorWrapper& foo : array) {} + for (const OperatorWrapper foo : array) {} + for (OperatorWrapper foo : array) {} }