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

Jakub Kuderski via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 2 09:01:26 PDT 2021


kuhar added a comment.

In D107378#3102694 <https://reviews.llvm.org/D107378#3102694>, @nikic wrote:

> For what it's worth, I really don't like this change. Seems like unnecessary complexity for very little gain.
>
> Case in point, while `CmpInst::Predicate` is marked as safe to iterate by this change, this is not actually correct. It's safe to iterate all FCMP predicates and it's safe to iterate all ICMP predicates, but it's not safe to cross between those -- there is an unassigned range between them.

Good catch, I find things like this easy to miss. This made me start working on safe iteration in the first place. I'll update the patch to not declare it as safe and introduce helper function that return subranges for fcm and icmp instead.
With these in place, do you oppose this patch going in, @nikic?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107378



More information about the llvm-commits mailing list