[PATCH] D103900: [llvm] Add enum iteration to Sequence

[Guillaume Chatelet via Phabricator via llvm-commits]

gchatelet marked an inline comment as done.
gchatelet added inline comments.


================
Comment at: llvm/include/llvm/ADT/Sequence.h:28
 
-template <typename T, bool IsReversed> struct iota_range_iterator {
+template <typename U, typename T, bool IsReversed> struct iota_range_iterator {
   using iterator_category = std::random_access_iterator_tag;
----------------
Quuxplusone wrote:
> Nits: Please add a newline before `struct`; and consider renaming/reordering the parameters so that `T` comes before `U`. I think that should be easy and mechanical (if tedious), because neither `T` nor `U` is deduced nor defaulted.
> Nits: Please add a newline before `struct`;

clang-format puts it back to the same line :-/

> and consider renaming/reordering the parameters so that `T` comes before `U`. I think that should be easy and mechanical (if tedious), because neither `T` nor `U` is deduced nor defaulted.

Done.


================
Comment at: llvm/include/llvm/ADT/Sequence.h:196
+  auto rbegin() const { return const_reverse_iterator(PastEndValue - 1); }
+  auto rend() const { return const_reverse_iterator(BeginValue - 1); }
 
----------------
Quuxplusone wrote:
> Note that `BeginValue-1` might be UB; e.g. `seq(INT_MIN, 0).rend()`. But I think it's fine to ignore this corner case.
> Note that `BeginValue-1` might be UB; e.g. `seq(INT_MIN, 0).rend()`. But I think it's fine to ignore this corner case.

Fixed it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103900/new/

https://reviews.llvm.org/D103900