[PATCH] D107378: Make enum iteration with seq safe by default

[Guillaume Chatelet via Phabricator via llvm-commits]

gchatelet added a comment.

In D107378#3072709 <https://reviews.llvm.org/D107378#3072709>, @aaron.ballman wrote:

> Personally, I sort of wonder if it makes sense to to disallow enum iteration entirely. Are we convinced we want to encourage people to do this even when the enumeration is held just right?

I don't think we want to encourage this pattern but when we have to use it I think it's better to have a tool to help with everything that can go wrong (integer promotion, non UB wrap around for signed types, reverse iteration, etc...).
Being in a position where people re-implement it with subtle bugs is not great either.

Now if enum iteration usage can be brought to 0 then yes we can certainly disallow it entirely.
If not, maybe we want a separate interface to stress that it's an uncommon pattern (most of the underlying logic is shared with integer iteration so it would be best not to implement it twice).

There are not a lot of places in llvm-project where we make use of enum iteration. MachineValueType <https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/Support/MachineValueType.h#L1407-L1453> is probably the most pervasive. It relies on enum iteration to produce MVT or EVT on the fly.
A quick search through the code base gives the following calls for `all_valuetypes()`, `integer_valuetypes()`, `fp_valuetypes()` and  `vector_valuetypes`.

   % git grep all_valuetypes\(\) | grep -v include/llvm/Support/MachineValueType.h | wc -l
  4
   % git grep integer_valuetypes\(\) | grep -v include/llvm/Support/MachineValueType.h | wc -l
  33
   % git grep fp_valuetypes\(\) | grep -v include/llvm/Support/MachineValueType.h | wc -l
  16
   % git grep vector_valuetypes\(\) | grep -v include/llvm/Support/MachineValueType.h | wc -l
  49

If we can afford it, these functions can be implemented with static arrays (either lazily or eagerly initialized). Having real storage will allow reverse iteration which was the main motivation behind me touching this file D102679 <https://reviews.llvm.org/D102679>.
Once these are gone, it may be possible to kill enum iteration entirely if we wish too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107378