[PATCH] D122079: [ADT] Add Enum matcher

[Jakub Kuderski via Phabricator via llvm-commits]

kuhar added a comment.

In D122079#3401001 <https://reviews.llvm.org/D122079#3401001>, @beanz wrote:

> In D122079#3396156 <https://reviews.llvm.org/D122079#3396156>, @kuhar wrote:
>
>> 



>> 2. This could be made constexpr if we wanted.
>
> I had another place that I wanted to use this in clang where the input parameter isn't constant.

`constexpr` works with both constant-evaluated contexts and runtime calls.

>> 5. Since this function is not constexpr now, how about we added an overload to `is_contained` accepting an initializer list as the second element, e.g.: `is_constained(MyEnum::A, {MyEnum::A, MyEnum::B, MyEnum::C})`?
>
> I realize this is probably a bit overkill, but `isInEnumSet<Numbers, One, Two, Three, Four, Five>(Val)` will actually optimize down to roughly `if( Val >= One && Val <= Five)` (assuming sequential values), and that really only works reliably with the template parameter unrolling.

I've checked and it seems to work just as well when passing in an array. With `initializer_list`, there are still some branches left. See https://godbolt.org/z/o7hKnKen5.

Overall, I'd lean towards the array or initializer_list version (either re-using `is_contained` or introducing a new name like `is_in` to keep things more familiar and avoid recursion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122079