adding xt::detail::has_fixed_size and replacing is_array to expand supported types. - #2556
Conversation
Sorry, something went wrong.
|
The build only fails on the older msvc 19.0, but it's not clear why. It says use of undefined type 'xt::detail::get_fixed_size<T,void>', but this should use the specialization in xshape.hpp at line 399. |
Sorry, something went wrong.
JohanMabille
left a comment
There was a problem hiding this comment.
The error on Windows seems legit.
Sorry, something went wrong.
| # pragma clang diagnostic ignored "-Wmismatched-tags" | ||
| #endif | ||
|
|
||
| namespace std |
There was a problem hiding this comment.
A good practice is to avoid opening std namespace. You can directly specialize tuple_size with the following syntax:
tempate <class ET, class S, xt::layout_type L, bool SH, class TAG>
struct std::tuple_size<xt::xfixed_container<ET, S, L, SH, Tag>> :
// ...Also pay attention that tuple_size is a struct, not a class (therefore, no need to specify a public inheritance, and this will avoid a lot of warnings).
Sorry, something went wrong.
There was a problem hiding this comment.
This specialization was done in the same style as here:
xtensor/include/xtensor/xstorage.hpp
Line 1915 in 3084585
It also contains a comment about tuple_size being a class in c++ standard, I can just copy the comment here, or update also the xstorage version.
Sorry, something went wrong.
There was a problem hiding this comment.
My initial comment was based on the standard definition of tuple_size. However, it appears that th standard uses class tuple_size in other parts of the spec, and is therefore inconsistent. libc++ is consistent and uses class everywhere, so we can keep it. See this discussion for more detail about this issue.
The specializations in xstorage.hpp should be updated to avoid opening the std namespace, though.
Sorry, something went wrong.
This pr adds type trait xt::detail::has_fixed_size to replace a common usage of xt::detail::is_array where implementation can benefit if the size of the container (usually used for shape) is known at compile time. This allows to add xt::sequence_view for example to supported types for shape in xt::adapt, but even much much more.
This also adds xt::detail::get_fixed_shape as a wrapper to std::tuple_size. The reason this is needed is that for example for xfunction that has fixed shape, we also want to use std::tuple_size, but we cannot partially specialize std::tuple_size only for the cases when xfunction has fixed shape.