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

[Jakub Kuderski via Phabricator via llvm-commits]

kuhar added inline comments.


================
Comment at: llvm/include/llvm/ADT/Sequence.h:333
+    static_assert(
+        enum_traits::is_iterable,
+        "Enum not iterable. Use DECLARE_ITERABLE_ENUM or override this "
----------------
gchatelet wrote:
> I think I already asked the question but can't you inline this? I mean removing the `using` above,
Previously, I had a local variable which I inlined into the `decltype`.

I prefer the current version over having everything inside the `static_assert`. This is because there are 3 failure points here:
1.  `add_enum_traits` is not defined and fails.
2.  `add_enum_traits` is defined but incorrectly and does not return a type with `::traits_type`.
3.  `enum_traits::is_iterable` is `false`.

I think compiler error messages may be confusing if we put all possible 3 failures in one place. With this version, 3. should appear on its own line.

I also think that the local typedef helps with debugging with type traits, in case someone want's to inspect their type with something like `std::is_same` etc.

What do you think? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107378