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

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

I think I'm OK with this if it's moved away from the macro - using a template specialization type trait system even if that sacrifices the ability to define the trait next to a nested type. (indeed the couple of examples migrated in this patch do the registration outside the outer class rather than benefiting from this nested ability anyway), if that sounds OK?



================
Comment at: llvm/include/llvm/ADT/Sequence.h:122
+  inline auto LLVM_ATTRIBUTE_UNUSED add_enum_traits(ENUM value) {              \
+    return llvm::forge_iterable_enum(value);                                   \
+  }
----------------
dblaikie wrote:
> I think for usability (error message quality, etc) it might be worth not using `forge_iterable_enum` here, since if it appears in an error it might be confusing to users since this type isn't being forged, it's using the legitimate entry point for a type truly declared iterable. (so maybe the implementation should be `return enum_with_traits<Enum, iterable_enum_traits<Enum>>(Value);`)
Ping on this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107378