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

[Guillaume Chatelet via Phabricator via llvm-commits]

gchatelet added inline comments.


================
Comment at: llvm/include/llvm/ADT/Sequence.h:165-216
+  value_type BeginValue;
+  value_type PastEndValue;
+
+  explicit iota_range(T Begin, T End, bool Inclusive)
+      : BeginValue(Begin), PastEndValue(Inclusive ? NextValue(End) : End) {
+    // We only check relative order of input values here (i.e. `Begin`/`End`).
+    assert(Begin <= End);
----------------
@Quuxplusone when using `seq_inclusive` with `End == std::numeric_limits<T>::max()` iteration works fine but iterator comparison is broken.
There are two possibilities here:
 1. Leave it as-is and document the broken behavior while allowing inclusive `End` being `INT_MAX`
 2. assert when using `seq_inclusive` and `End == std::numeric_limits<T>::max()`, effectively disabling iteration up to - and including - `INT_MAX`.

The current implementation is for 1. but I think 2. is more reasonable.
What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103900