CPP20Hackathon: Add more concepts and use them in pattern/forall#2012
CPP20Hackathon: Add more concepts and use them in pattern/forall#2012MrBurmark wants to merge 1 commit into
Conversation
| union U{}; | ||
| using L = decltype([]{}); | ||
| static_assert(!PotentialInvokable<I>); | ||
| static_assert(!PotentialInvokable<I&>); |
There was a problem hiding this comment.
Could you make a variadic template that tries to substitute the pack into std::is_invocable?
| { | ||
| /// Adapter to replace specific implementations for the icount variants | ||
| template<typename Range, typename Body, typename IndexT> | ||
| template<Range Container, PotentialInvokable Body, IndexTypeConcept IndexT> |
There was a problem hiding this comment.
We might want to review what concretely we care about in term of Body. I think we might something conceptually much narrower than Invocable
There was a problem hiding this comment.
I agree with that. Also, it seems like the PotentialInvokable name is somewhat confusing (vs. Invokable which may be a requirement we want to enforce). If the Body arg is NOT invokable, what does that mean?
| Range Container, | ||
| typename LoopBody, | ||
| typename Res = typename resources::get_resource<ExecutionPolicy>::type> | ||
| PotentialInvokable Body, |
There was a problem hiding this comment.
Maybe we want to create a concept for lambdas for external, user facing function templates, and then another Privatizer based concept for internal calls
| using container_type = typename std::decay<Range>::type; | ||
| using container_type = typename std::decay<Container>::type; | ||
| typename container_type::iterator begin_it; | ||
| Index_type icount; |
There was a problem hiding this comment.
| Index_type icount; | |
| index_type icount; |
outside of this PR, but line 100 defines using index_type = typename std::decay<IndexT>::type; are we using the right one?
| using index_type = typename std::decay<IndexT>::type; | ||
| typename std::decay<Body>::type body; | ||
| using container_type = typename std::decay<Range>::type; | ||
| using container_type = typename std::decay<Container>::type; |
There was a problem hiding this comment.
| using container_type = typename std::decay<Container>::type; | |
| using container_type = std::decay_t<Container>; |
More compact
| template<Range Container, PotentialInvokable Body, IndexTypeConcept IndexT> | ||
| struct icount_adapter | ||
| { | ||
| using index_type = typename std::decay<IndexT>::type; |
There was a problem hiding this comment.
| using index_type = typename std::decay<IndexT>::type; | |
| using index_type = std::decay_t<IndexT>; |
There was a problem hiding this comment.
@artv3 I think this sort of thing is now your favorite type of PR review comment 😆
| template < typename B > | ||
| concept PotentialInvokable = | ||
| std::is_class_v<std::remove_cvref_t<B>> || | ||
| std::is_union_v<std::remove_cvref_t<B>> || |
There was a problem hiding this comment.
do we have callable unions in our applications?
5 participants
I tried to conceptify the remaining types in the pattern forall interface.
I notice that the
RAJA::Resourceconcept is close to overlapping withcamp::resources::Resource.The
RAJA::Resourceconcept seems to be more of a typed resource concept and does not seem to includecamp::resources::Resource. We may want to consider renaming for clarity.Some of the concepts that I added, like
PotentiallyInvokableorNonResource, are vague but may still be useful to document the types that we are intended to be passed through some functions.