Introduce concepts for AccessTraits
The patch introduces three concepts: AccessTraits, Primitives (same as AccessTraits) and Predicates (AccessTraits + tag). I put them into Concepts namespace.
In general, it works fine. One corner point is how it interacts with CTAD. clang evaluates the right hand side of CTAD before testing the concept, which leads to suboptimal errors instead of ignoring the CTAD (see godbolt). GCC, on the other hand, has a better treatment and properly rejects CTAD due to the unsatisfied requirement.
@dalg24 Could you please take a look at this PR?
Moved 'Concepts' namespace inside Details to hide them from users.
Here are what the errors look like for the tstCompileOnlyAccessTraits.cpp:
In LLVM:
<snip>/test/tstCompileOnlyAccessTraits.cpp:137:7: error: static assertion failed
137 | ArborX::Details::Concepts::AccessTraits<NoAccessTraitsSpecialization>);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<snip>/test/tstCompileOnlyAccessTraits.cpp:137:7: note: because 'NoAccessTraitsSpecialization' does not satisfy 'AccessTraits'
<snip>/src/spatial/detail/ArborX_AccessTraits.hpp:82:37: note: because 'typename ArborX::AccessTraits<T>::memory_space' would be invalid: no type named 'memory_space' in 'ArborX::AccessTraits<NoAccessTraitsSpecialization>'
82 | typename ArborX::AccessTraits<T>::memory_space;
<snip>/test/tstCompileOnlyAccessTraits.cpp:138:17: error: static assertion failed
138 | static_assert(ArborX::Details::Concepts::AccessTraits<EmptySpecialization>);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<snip>/test/tstCompileOnlyAccessTraits.cpp:138:17: note: because 'EmptySpecialization' does not satisfy 'AccessTraits'
<snip>/src/spatial/detail/ArborX_AccessTraits.hpp:82:37: note: because 'typename ArborX::AccessTraits<T>::memory_space' would be invalid: no type named 'memory_space' in 'ArborX::AccessTraits<EmptySpecialization>'
82 | typename ArborX::AccessTraits<T>::memory_space;
<snip>/test/tstCompileOnlyAccessTraits.cpp:139:17: error: static assertion failed
139 | static_assert(ArborX::Details::Concepts::AccessTraits<InvalidMemorySpace>);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<snip>/test/tstCompileOnlyAccessTraits.cpp:139:17: note: because 'InvalidMemorySpace' does not satisfy 'AccessTraits'
<snip>/src/spatial/detail/ArborX_AccessTraits.hpp:83:12: note: because 'Kokkos::is_memory_space_v<typename ArborX::AccessTraits<InvalidMemorySpace>::memory_space>' evaluated to false
83 | requires Kokkos::is_memory_space_v<
<snip>/test/tstCompileOnlyAccessTraits.cpp:141:7: error: static assertion failed
141 | ArborX::Details::Concepts::AccessTraits<MissingSizeMemberFunction>);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<snip>/test/tstCompileOnlyAccessTraits.cpp:141:7: note: because 'MissingSizeMemberFunction' does not satisfy 'AccessTraits'
<snip>/src/spatial/detail/ArborX_AccessTraits.hpp:87:22: note: because 'AccessTraits<T>::size(v)' would be invalid: no member named 'size' in 'ArborX::AccessTraits<MissingSizeMemberFunction>'
87 | AccessTraits<T>::size(v)
<snip>/test/tstCompileOnlyAccessTraits.cpp:143:7: error: static assertion failed
143 | ArborX::Details::Concepts::AccessTraits<SizeMemberFunctionNotStatic>);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<snip>/test/tstCompileOnlyAccessTraits.cpp:143:7: note: because 'SizeMemberFunctionNotStatic' does not satisfy 'AccessTraits'
<snip>/src/spatial/detail/ArborX_AccessTraits.hpp:87:22: note: because 'AccessTraits<T>::size(v)' would be invalid: call to non-static member function without an object argument
87 | AccessTraits<T>::size(v)
<snip>/test/tstCompileOnlyAccessTraits.cpp:145:7: error: static assertion failed
145 | ArborX::Details::Concepts::AccessTraits<MissingGetMemberFunction>);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<snip>/test/tstCompileOnlyAccessTraits.cpp:145:7: note: because 'MissingGetMemberFunction' does not satisfy 'AccessTraits'
<snip>/src/spatial/detail/ArborX_AccessTraits.hpp:91:20: note: because 'AccessTraits<T>::get(v, 0)' would be invalid: no member named 'get' in 'ArborX::AccessTraits<MissingGetMemberFunction>'
91 | AccessTraits<T>::get(v, 0);
<snip>/test/tstCompileOnlyAccessTraits.cpp:147:7: error: static assertion failed
147 | ArborX::Details::Concepts::AccessTraits<GetMemberFunctionNotStatic>);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<snip>/test/tstCompileOnlyAccessTraits.cpp:147:7: note: because 'GetMemberFunctionNotStatic' does not satisfy 'AccessTraits'
<snip>/src/spatial/detail/ArborX_AccessTraits.hpp:91:20: note: because 'AccessTraits<T>::get(v, 0)' would be invalid: call to non-static member function without an object argument
91 | AccessTraits<T>::get(v, 0);
<snip>/test/tstCompileOnlyAccessTraits.cpp:148:17: error: static assertion failed
148 | static_assert(ArborX::Details::Concepts::AccessTraits<GetMemberFunctionVoid>);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<snip>/test/tstCompileOnlyAccessTraits.cpp:148:17: note: because 'GetMemberFunctionVoid' does not satisfy 'AccessTraits'
<snip>/src/spatial/detail/ArborX_AccessTraits.hpp:92:6: note: because '!requires (const GetMemberFunctionVoid &v) { { AccessTraits<GetMemberFunctionVoid>::get(v, 0) } -> std::same_as<void>; }' evaluated to false
92 | } && !requires(T const &v) {
So I see two suboptimal messages:
- If specialization is undefined, we don't explicitly say that
- If specialization returns void, the error is a bit challenging to parse
It seems that the details of notes seem to depend on whether static_assert does any computations. For example,
static_assert(ArborX::Details::Concepts::AccessTraits<NoAccessTraitsSpecialization>); fails with
/tstCompileOnlyAccessTraits.cpp.o -MF test/CMakeFiles/ArborX_Test_CompileOnly.exe.dir/tstCompileOnlyAccessTraits.cpp.o.d -o test/CMakeFiles/ArborX_Test_CompileOnly.exe.dir/tstCompileOnlyAccessTraits.cpp.o -c <snip>/test/tstCompileOnlyAccessTraits.cpp
<snip>/test/tstCompileOnlyAccessTraits.cpp:137:7: error: static assertion failed
137 | ArborX::Details::Concepts::AccessTraits<NoAccessTraitsSpecialization>);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<snip>/test/tstCompileOnlyAccessTraits.cpp:137:7: note: because 'NoAccessTraitsSpecialization' does not satisfy 'AccessTraits'
<snip>/src/spatial/detail/ArborX_AccessTraits.hpp:82:37: note: because 'typename ArborX::AccessTraits<T>::memory_space' would be invalid: no type named 'memory_space' in 'ArborX::AccessTraits<NoAccessTraitsSpecialization>'
82 | typename ArborX::AccessTraits<T>::memory_space;
| ^ ^
but
static_assert(!(!ArborX::Details::Concepts::AccessTraits<NoAccessTraitsSpecialization>)); fails simply with
<snip>/test/tstCompileOnlyAccessTraits.cpp:136:17: error: static assertion failed due to requirement '!(!ArborX::Details::Concepts::AccessTraits<NoAccessTraitsSpecialization>)'
136 | static_assert(!(
| ^~
137 | !ArborX::Details::Concepts::AccessTraits<NoAccessTraitsSpecialization>));
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Which is fine, but is still interesting.
@dalg24 I added couple commits:
- Added a more thorough testing for
Predicatesconcept, which also madeis_valid_predicate_taga concept - Added
complete_typeconcept which checks whether specialization exists. I'm open to a better name.
I do agree with you that if the concepts allowed to specify our error messages, that would be ideal. The only step in that direction I've seen is the [EWG] P1267, but it went nowhere, it seems. The only alternative I see is to name each step of our concepts with some meaningful name, which is a pain.