Fix SafeIntIterator reference type

[gchatelet]

Summary

We explicitly state that the reference type for Sequence iterator is a value_type.
Since the iterator is a lazy generator, it cannot point to any memory and so it cannot have a reference type.
Fixes https://github.com/llvm/llvm-project/issues/61122

[foad]

Thanks for the fix! Unfortunately I am not competent to review it.

[kuhar]

LGTM, thanks for the fix. Just a couple of nits.

Fix #61122

Could you make the commit line more specific? Maybe something like 'Fix SafeIntIterator reference type'.

llvm/unittests/ADT/SequenceTest.cpp

299

nit: Could you rename this or add a comment explaining what the intention is behind this test?

[gchatelet]

rebase, rename test and retitle commit

[gchatelet]

Thx for the review.

llvm/include/llvm/ADT/Sequence.h

private:
   intmax_t Value;
 };
 
 template <typename T, bool IsReverse> struct SafeIntIterator {
   using iterator_category = std::random_access_iterator_tag;
   using value_type = T;
   using difference_type = intmax_t;
   using pointer = T *;
-  using reference = T &;
+  using reference = value_type; // The iterator does not reference memory.
 
   // Construct from T.
   explicit SafeIntIterator(T Value) : SI(CheckedInt::from<T>(Value)) {}
   // Construct from other direction.
   SafeIntIterator(const SafeIntIterator<T, !IsReverse> &O) : SI(O.SI) {}
 
   // Dereference
-  value_type operator*() const { return SI.to<T>(); }
+  reference operator*() const { return SI.to<T>(); }
   // Indexing
-  value_type operator[](intmax_t Offset) const { return *(*this + Offset); }
+  reference operator[](intmax_t Offset) const { return *(*this + Offset); }
 
   // Can be compared for equivalence using the equality/inequality operators.
   bool operator==(const SafeIntIterator &O) const { return SI == O.SI; }
   bool operator!=(const SafeIntIterator &O) const { return SI != O.SI; }
   // Comparison
   bool operator<(const SafeIntIterator &O) const { return (*this - O) < 0; }
   bool operator>(const SafeIntIterator &O) const { return (*this - O) > 0; }
   bool operator<=(const SafeIntIterator &O) const { return (*this - O) <= 0; }

llvm/unittests/ADT/SequenceTest.cpp

TEST(SequenceTest, NonIterableEnums) {
   EXPECT_THAT(enum_seq(UntypedEnum::A, UntypedEnum::A,
                        force_iteration_on_noniterable_enum),
               IsEmpty());
   EXPECT_THAT(enum_seq_inclusive(UntypedEnum::A, UntypedEnum::A,
                                  force_iteration_on_noniterable_enum),
               ElementsAre(UntypedEnum::A));
 }
 
+TEST(SequenceTest, Repro) {
+  std::vector<int> vals = {1, 2, 3};
+  detail::SafeIntIterator<int, false> begin(4);
+  detail::SafeIntIterator<int, false> end(6);
+  vals.insert(vals.end(), begin, end);
+  EXPECT_THAT(vals, ElementsAre(1, 2, 3, 4, 5));
+}
+
 } // namespace