Remove Impl::enable_if_type #3863
Conversation
Utility metafunction that maps a sequence of any types to the type void
dalg24
force-pushed
the
replace_enable_if_type_by_void_t
branch
from
e6c2ff4
to
faf8538
Compare
|
Squashed the fixup commits and rebased. No changes. |
There was a problem hiding this comment.
This is fine as is, but there are a few changes. I do actually think enable_if_type is a more descriptive name than void_t. (Both do the same thing, so one way this pull request could have gone is to just rename enable_if_type to use void_t). I think maybe enable_if_valid_type is a better name, or something like check_for_type_in_sfinae_safe_context is probably even better. I don't know that this is super important (I don't know that it's as bad as it looks as is), but it's slightly more reasonable and modern so I approve,
| #if defined(__cpp_lib_void_t) | ||
| // since C++17 | ||
| using std::void_t; | ||
| #else | ||
| template <class...> | ||
| using void_t = void; | ||
| #endif |
There was a problem hiding this comment.
I'm not confident this works on all of the compilers we support (particularly Intel 17)
| template <class T, class = void> | ||
| struct is_detected_execution_space : std::false_type { | ||
| using type = not_a_type; | ||
| }; | ||
|
|
||
| template <class T> | ||
| struct is_detected_execution_space<T, void_t<typename T::execution_space>> | ||
| : std::true_type { | ||
| using type = typename T::execution_space; | ||
| }; | ||
|
|
||
| template <class T> | ||
| using detected_execution_space_t = | ||
| typename is_detected_execution_space<T>::type; | ||
|
|
||
| template <class T, class = void> | ||
| struct is_detected_device_type : std::false_type { | ||
| using type = not_a_type; | ||
| }; | ||
|
|
||
| template <class T> | ||
| struct is_detected_device_type<T, void_t<typename T::device_type>> | ||
| : std::true_type { | ||
| using type = typename T::device_type; | ||
| }; | ||
|
|
||
| template <class T> | ||
| using detected_device_type_t = typename is_detected_device_type<T>::type; | ||
|
|
There was a problem hiding this comment.
Why not just put is_detected in directly instead of making special versions of each of these?
| struct not_a_type { | ||
| not_a_type() = delete; | ||
| ~not_a_type() = delete; | ||
| not_a_type(not_a_type const&) = delete; | ||
| void operator=(not_a_type const&) = delete; | ||
| }; |
There was a problem hiding this comment.
Non-blocking and super minor: In detection idiom, this is usually named nonesuch. By convention in metaprogramming (for whatever reason), not_a_type (or nat sometimes) is usually just an undefined type or completely empty at the least.
There was a problem hiding this comment.
We can rename when we add the detection idiom
|
As an alternative, I propose renaming and slightly modifying |
Rational: You are probably wondering: "What is
enable_if_typeanyway?" Well we were not using it much, I could not find it in any other packages in Trilinos, and part of its functionality is superseded bystd::void_t. I opted for removal and ended up rewritingFunctorPolicyExecutionSpace.I know some of us would like to go further and add the detection idiom but let's not address this here.