[PATCH] D107378: Make enum iteration with seq safe by default
Guillaume Chatelet via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 15 02:06:28 PDT 2021
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
More information about the llvm-commits
mailing list