diff --git a/clang-tools-extra/clang-tidy/bugprone/FoldInitTypeCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/FoldInitTypeCheck.cpp --- a/clang-tools-extra/clang-tidy/bugprone/FoldInitTypeCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/FoldInitTypeCheck.cpp @@ -26,9 +26,24 @@ return anyOf( // Pointer types. pointsTo(BuiltinTypeWithId(ID)), - // Iterator types. - recordType(hasDeclaration(has(typedefNameDecl( - hasName("value_type"), hasType(BuiltinTypeWithId(ID))))))); + // Iterator types have an `operator*` whose return tyoe is the type we + // care about. + // Notes: + // - `operator*` can be in one of the bases of the iterator class. + // - this does not handle cases when the `operator*` is defined + // outside the iterator class. + recordType( + hasDeclaration(cxxRecordDecl(isSameOrDerivedFrom(has(functionDecl( + hasOverloadedOperatorName("*"), + anyOf( + // `value_type& operator*();` + returns(hasCanonicalType( + referenceType(pointee(BuiltinTypeWithId(ID))))), + // `value_type operator*();` + returns(hasCanonicalType(BuiltinTypeWithId(ID))), + // `auto operator*();`, `decltype(auto) operator*();` + returns(hasCanonicalType(autoType( + hasDeducedType(BuiltinTypeWithId(ID))))))))))))); }; const auto IteratorParam = parmVarDecl( diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/fold-init-type.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/fold-init-type.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/fold-init-type.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/fold-init-type.cpp @@ -1,39 +1,67 @@ -// RUN: %check_clang_tidy %s bugprone-fold-init-type %t +// RUN: %check_clang_tidy %s bugprone-fold-init-type -std=c++17 %t namespace std { template -T accumulate(InputIt first, InputIt last, T init); +T accumulate(InputIt first, InputIt last, T init) { + // When `InputIt::operator*` returns a deduced `auto` type that refers to a + // dependent type, the return type is deduced only if `InputIt::operator*` + // is instantiated. In practice this happens somewhere in the implementation + // of `accumulate`. For tests, do it here. + (void)*first; +} template -T reduce(InputIt first, InputIt last, T init); +T reduce(InputIt first, InputIt last, T init) { (void)*first; } template T reduce(ExecutionPolicy &&policy, - InputIt first, InputIt last, T init); + InputIt first, InputIt last, T init) { (void)*first; } struct parallel_execution_policy {}; constexpr parallel_execution_policy par{}; template T inner_product(InputIt1 first1, InputIt1 last1, - InputIt2 first2, T value); + InputIt2 first2, T value) { (void)*first1; (void)*first2; } template T inner_product(ExecutionPolicy &&policy, InputIt1 first1, InputIt1 last1, - InputIt2 first2, T value); + InputIt2 first2, T value) { (void)*first1; (void)*first2; } } // namespace std struct FloatIterator { - typedef float value_type; + const float &operator*() const; }; + +struct DerivedFloatIterator : public FloatIterator { +}; + +template struct ByValueTemplateIterator { + ValueType operator*() const; +}; + +template struct ByRefTemplateIterator { + ValueType &operator*(); +}; + +template struct ByRefTemplateIteratorWithAlias { + using reference = const ValueType&; + reference operator*(); +}; + +template struct AutoByValueTemplateIterator { + auto operator*() const { return ValueType{}; } +}; + +template struct AutoByRefTemplateIterator { + decltype(auto) operator*() const { return value_; } + ValueType value_; +}; + template -struct TypedefTemplateIterator { typedef ValueType value_type; }; -template -struct UsingTemplateIterator { using value_type = ValueType; }; -template -struct DependentTypedefTemplateIterator { typedef typename ValueType::value_type value_type; }; -template -struct DependentUsingTemplateIterator : public TypedefTemplateIterator { using typename TypedefTemplateIterator::value_type; }; +struct InheritingByConstRefTemplateIterator + : public ByRefTemplateIterator {}; + using TypedeffedIterator = FloatIterator; // Positives. @@ -51,39 +79,57 @@ } int accumulatePositive3() { + DerivedFloatIterator it; + return std::accumulate(it, it, 0); + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'float' into type 'int' +} + +int accumulatePositive4() { double a[1] = {0.0}; return std::accumulate(a, a + 1, 0.0f); // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'double' into type 'float' } -int accumulatePositive4() { - TypedefTemplateIterator it; +int accumulatePositive5() { + ByValueTemplateIterator it; return std::accumulate(it, it, 0); // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'unsigned int' into type 'int' } -int accumulatePositive5() { - UsingTemplateIterator it; +int accumulatePositive6() { + ByRefTemplateIterator it; return std::accumulate(it, it, 0); // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'unsigned int' into type 'int' } -int accumulatePositive6() { - DependentTypedefTemplateIterator> it; +int accumulatePositive7() { + AutoByValueTemplateIterator it; return std::accumulate(it, it, 0); // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'unsigned int' into type 'int' } -int accumulatePositive7() { +int accumulatePositive8() { TypedeffedIterator it; return std::accumulate(it, it, 0); // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'float' into type 'int' } -int accumulatePositive8() { - DependentUsingTemplateIterator it; +int accumulatePositive9() { + InheritingByConstRefTemplateIterator it; return std::accumulate(it, it, 0); - // FIXME: this one should trigger too. + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'unsigned int' into type 'int' +} + +int accumulatePositive10() { + AutoByRefTemplateIterator it; + return std::accumulate(it, it, 0); + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'unsigned int' into type 'int' +} + +int accumulatePositive11() { + ByRefTemplateIteratorWithAlias it; + return std::accumulate(it, it, 0); + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'unsigned int' into type 'int' } int reducePositive1() { @@ -133,7 +179,7 @@ } int negative4() { - TypedefTemplateIterator it; + ByValueTemplateIterator it; // For now this is OK. return std::accumulate(it, it, 0.0); } @@ -156,3 +202,4 @@ return std::accumulate(a, a + 1, 0); } } +