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

[Guillaume Chatelet via Phabricator via llvm-commits]

gchatelet 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 "
----------------
kuhar wrote:
> 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? 
Yep sounds good to me. Thx for the explanation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107378