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

[Arthur O'Dwyer via Phabricator via llvm-commits]

Quuxplusone added inline comments.


================
Comment at: llvm/include/llvm/ADT/Sequence.h:186
+  auto rbegin() const { return const_reverse_iterator(*--end()); }
+  auto rend() const { return const_reverse_iterator(*--begin()); }
 
----------------
I've just noticed that this is unidiomatic: on normal iterators, `--begin()` would be UB. (`std::reverse_iterator` always stores one-past-where-it's-"pointing"-to, for this reason.) I guess it's OK here, but if you've got the time, maybe think about some way to avoid the hiccup here.


================
Comment at: llvm/include/llvm/ADT/Sequence.h:203
+  auto result = seq(Begin, End);
+  result.End = *++result.end();
+  return result;
----------------
Phab somehow ate my comment here.
This should be just `return seq(Begin, static_cast<T>(End + 1));`
and then in the constructor of `iota_range`, please `assert(Begin <= End)` just as a sanity check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103900