[clang-tidy]bugprone-fold-init-type

[courbet]

Summary

Handle iterators that do not define a value_type type alias.

Get the value type from the return type of Iter::operator* instead.

[courbet]

fix typo

[Eugene.Zelenko]

clang-tools-extra/test/clang-tidy/checkers/bugprone/fold-init-type.cpp

205

Excessive newline.

[courbet]

remove extra training newline

[courbet]

ping

[courbet]

ping

[PiotrZSL]

Missing ReleaseNotes,

clang-tools-extra/clang-tidy/bugprone/FoldInitTypeCheck.cpp

41

referenceType(pointee -> references

43

BuiltinTypeWithId already got hasCanonicalType, no need to duplicate

45

maybe:

returns(qualType(hasCanonicalType(anyOf(BuiltinTypeWithId(ID), references(BuiltinTypeWithId(ID)), autoType(hasDeducedType(BuiltinTypeWithId(ID)))

instead of having anyOf on top level.

[courbet]

• Address review comments
• Add release notes.

Thanks !

[PiotrZSL]

clang-tools-extra/docs/ReleaseNotes.rst

101 ↗

(On Diff #511424)

Wrong section, this should be in 'Changes in existing checks'

[courbet]

Fix release note placement and phrase it more consistenly with other notes in that section.

[PiotrZSL]

LGTM

[courbet]

Thanks for the review.

[Eugene.Zelenko]

Please rebase from latest main.

clang-tools-extra/docs/ReleaseNotes.rst

294 ↗

(On Diff #511667)

Please keep alphabetical order (by check name) in this section.

[courbet]

clang-tools-extra/docs/ReleaseNotes.rst

294 ↗

(On Diff #511667)

Done (in d880d4e7c228).

clang-tools-extra/clang-tidy/bugprone/FoldInitTypeCheck.cpp

void FoldInitTypeCheck::registerMatchers(MatchFinder *Finder) {
   // Note: Right now we check only builtin types.
   const auto BuiltinTypeWithId = [](const char *ID) {
     return hasCanonicalType(builtinType().bind(ID));
   };
   const auto IteratorWithValueType = [&BuiltinTypeWithId](const char *ID) {
     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(
       hasType(hasCanonicalType(IteratorWithValueType("IterValueType"))));
   const auto Iterator2Param = parmVarDecl(
       hasType(hasCanonicalType(IteratorWithValueType("Iter2ValueType"))));
   const auto InitParam = parmVarDecl(hasType(BuiltinTypeWithId("InitType")));
 

clang-tools-extra/test/clang-tidy/checkers/bugprone/fold-init-type.cpp

-// 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 <class InputIt, class T>
-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 <class InputIt, class T>
-T reduce(InputIt first, InputIt last, T init);
+T reduce(InputIt first, InputIt last, T init) { (void)*first; }
 template <class ExecutionPolicy, class InputIt, class T>
 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 <class InputIt1, class InputIt2, class T>
 T inner_product(InputIt1 first1, InputIt1 last1,
-                InputIt2 first2, T value);
+                InputIt2 first2, T value) { (void)*first1; (void)*first2; }
 
 template <class ExecutionPolicy, class InputIt1, class InputIt2, class T>
 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 <typename ValueType> struct ByValueTemplateIterator {
+  ValueType operator*() const;
+};
+
+template <typename ValueType> struct ByRefTemplateIterator {
+  ValueType &operator*();
+};
+
+template <typename ValueType> struct ByRefTemplateIteratorWithAlias {
+  using reference = const ValueType&;
+  reference operator*();
+};
+
+template <typename ValueType> struct AutoByValueTemplateIterator {
+  auto operator*() const { return ValueType{}; }
+};
+
+template <typename ValueType> struct AutoByRefTemplateIterator {
+  decltype(auto) operator*() const { return value_; }
+  ValueType value_;
+};
+
 template <typename ValueType>
-struct TypedefTemplateIterator { typedef ValueType value_type; };
-template <typename ValueType>
-struct UsingTemplateIterator { using value_type = ValueType; };
-template <typename ValueType>
-struct DependentTypedefTemplateIterator { typedef typename ValueType::value_type value_type; };
-template <typename ValueType>
-struct DependentUsingTemplateIterator : public TypedefTemplateIterator<ValueType> { using typename TypedefTemplateIterator<ValueType>::value_type; };
+struct InheritingByConstRefTemplateIterator
+    : public ByRefTemplateIterator<const ValueType> {};
+
 using TypedeffedIterator = FloatIterator;
 
 // Positives.
 
 int accumulatePositive1() {
   float a[1] = {0.5f};
   return std::accumulate(a, a + 1, 0);
   // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'float' into type 'int'
 }
 
 int accumulatePositive2() {
   FloatIterator it;
   return std::accumulate(it, it, 0);
   // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'float' into type 'int'
 }
 
 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<unsigned> it;
+int accumulatePositive5() {
+  ByValueTemplateIterator<unsigned> it;
   return std::accumulate(it, it, 0);
   // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'unsigned int' into type 'int'
 }
 
-int accumulatePositive5() {
-  UsingTemplateIterator<unsigned> it;
+int accumulatePositive6() {
+  ByRefTemplateIterator<unsigned> it;
   return std::accumulate(it, it, 0);
   // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'unsigned int' into type 'int'
 }
 
-int accumulatePositive6() {
-  DependentTypedefTemplateIterator<UsingTemplateIterator<unsigned>> it;
+int accumulatePositive7() {
+  AutoByValueTemplateIterator<unsigned> 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<unsigned> it;
+int accumulatePositive9() {
+  InheritingByConstRefTemplateIterator<unsigned> 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<unsigned> it;
+  return std::accumulate(it, it, 0);
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'unsigned int' into type 'int'
+}
+
+int accumulatePositive11() {
+  ByRefTemplateIteratorWithAlias<unsigned> it;
+  return std::accumulate(it, it, 0);
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'unsigned int' into type 'int'
 }
 
 int reducePositive1() {
   float a[1] = {0.5f};
   return std::reduce(a, a + 1, 0);
   // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'float' into type 'int'
 }
 

 
 int negative3() {
   float a[1] = {0.5f};
   // This is OK because the user explicitly specified T.
   return std::accumulate<float *, float>(a, a + 1, 0);
 }
 
 int negative4() {
-  TypedefTemplateIterator<unsigned> it;
+  ByValueTemplateIterator<unsigned> it;
   // For now this is OK.
   return std::accumulate(it, it, 0.0);
 }
 
 int negative5() {
   float a[1] = {0.5f};
   float b[1] = {1.0f};
   return std::inner_product(std::par, a, a + 1, b, 0.0f);
 }
 
 namespace blah {
 namespace std {
 template <class InputIt, class T>
 T accumulate(InputIt, InputIt, T); // We should not care about this one.
 }
 
 int negative5() {
   float a[1] = {0.5f};
   // Note that this is using blah::std::accumulate.
   return std::accumulate(a, a + 1, 0);
 }
 }
+