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

Arthur O'Dwyer via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 23 16:44:19 PDT 2021


Quuxplusone added inline comments.


================
Comment at: llvm/include/llvm/ADT/Sequence.h:217-219
+/// Note on enum iteration: `seq` will generate all values regardless of whether
+/// they exist or not. It's the user's responsibility to make sure the range is
+/// valid.
----------------
Maybe "`seq` will generate each consecutive value, even if no enumerator with that value exists."


================
Comment at: llvm/include/llvm/ADT/Sequence.h:159
+      // This assertion forbids overflow of `PastEndValue`.
+      assert(End != std::numeric_limits<raw_type>::max() &&
+             "Forbidden End value for seq_inclusive.");
----------------
gchatelet wrote:
> courbet wrote:
> > So you can't `iota_range` and enum with `uint8_t` underlying type and 256 values ? That seems a bit artificial.
> > Maybe we could always use `intmax_t` (resp. `uintmax_t` for unsigned underlying types) as raw_type ?
> > So you can't `iota_range` and enum with `uint8_t` underlying type and 256 values ? That seems a bit artificial.
> > Maybe we could always use `intmax_t` (resp. `uintmax_t` for unsigned underlying types) as raw_type ?
> 
> Fair enough. I'm slightly concerned about integer promotion/conversion rules but it //should be safe// (famous last words).
Well, now I'm wondering why we don't just use `uintptr_t` all the time. Why does the storage type //ever// have to be signed `intptr_t`?
I think the rationale has to do with ensuring that `r.begin() < r.end()`; but maybe that just indicates that `Op::difference` is computing its return value via the wrong expression.
But anyway, I continue to be uninterested in blocking this at this point. :)

Please do verify that the test failures in the latest run aren't your fault (e.g. by rebasing and letting the tests run again) before committing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103900



More information about the llvm-commits mailing list