Adding concept to a part of the code

[alexandrehoffmann]

Checklist

• [ X] The title and commit message(s) are descriptive.
• [ X] Small commits made to fix your PR have been squashed to avoid history pollution.
• [ X] Tests have been added for new features or bug fixes.
• [ X] API of new functions and classes are documented.

Description

This PR C++20's concept and C++17's if constexpr to a moderatly large chunk of xtensor.

[JohanMabille]

When all the enable_if have been replaced, I think we can start removing most of the metafunction used in the concept definitinos, and defined the concept with specific C++20 syntax:

template <class E>
concept has_simd_interface = requires(R e, size_t i)
{
    e.load_simd(i);
    e.store_simd(i, E::vaue_type());
};

That should be done in a dedicated PR.

[JohanMabille]

Could you use std::input_iterator instead ?

[alexandrehoffmann]

changed in commit 4809115

[JohanMabille]

Nitpicking: xscalar_concept would be more consistent with the other names.

[alexandrehoffmann]

changed in last commit #4809115

[gouarin]

#2839 must be merged for the CI

[alexandrehoffmann]

Git pre-commit tends to switch requires and inline keywords . Is it possible to amand the git .pre-commit-config.yaml so that it does not happens ?

[spectre-ns]

@alexandrehoffmann @JohanMabille Great work! Concept are a great match for this library! When looking into views I found that the concepts of linear assignment and data interface (has_data_interface) are conflated in places. This means that views of functions are much slower than functions of views even though they are the same operations and commutative. Hopefully, we can come up with better logic for these properties once the code base is ported to C++20. :)

[alexandrehoffmann]

Ok so, as for now, clang doesn't allow to implement methods outside of the class scope if said method uses concepts.
As an example:

    template <class CT, class... S>
    template <class T>
        requires(has_data_interface_concept<T> and strided_view_concept<CT, S...>)
    auto xview<CT, S...>::linear_begin() -> linear_iterator
    {
        return m_e.storage().begin() + data_offset();
    }

does not compile with clang but

template <class CT, class... S>
class xview
{
[...]
        template <class T = xexpression_type>
            requires has_data_interface_concept<T> and strided_view_concept<CT, S...>
        const_linear_iterator linear_begin() const { return m_e.storage().begin() + data_offset(); } 
[...]       
};

does.

To me the two options are
a) wait for proper concept implementation in clang or upgrade to a newer version
b) declare all the method that rely on concepts within the class's scope.

[JohanMabille]

Does the trailing requires syntax work ? I.e. something like

    template <class CT, class... S>
    template <class T>
    auto xview<CT, S...>::linear_begin() -> linear_iterator
        requires(has_data_interface_concept<T> and strided_view_concept<CT, S...>)
    {
        return m_e.storage().begin() + data_offset();
    }

[alexandrehoffmann]

Does the trailing requires syntax work ? I.e. something like

    template <class CT, class... S>
    template <class T>
    auto xview<CT, S...>::linear_begin() -> linear_iterator
        requires(has_data_interface_concept<T> and strided_view_concept<CT, S...>)
    {
        return m_e.storage().begin() + data_offset();
    }

Unfortunately is does not, at least on my side:

/home/alex/Desktop/xtensor/include/xtensor/generators/../views/xview.hpp:1175:27: error: out-of-line definition of 'linear_begin' does not match any declaration in 'xview<CT, S...>'
 1175 |     auto xview<CT, S...>::linear_begin() -> linear_iterator

A possible solution would be to forward declare and then fully declare within the class e.g. :

template <class CT, class... S>
class xview
{
/////////// declaration zone
[...]
        template <class T = xexpression_type>
            requires has_data_interface_concept<T> and strided_view_concept<CT, S...>
        const_linear_iterator linear_begin() const;
[...]       
/////////// definition zone
        template <class T = xexpression_type>
            requires has_data_interface_concept<T> and strided_view_concept<CT, S...>
        const_linear_iterator linear_begin() const { return m_e.storage().begin() + data_offset(); } 
};

[JohanMabille]

A possible solution would be to forward declare and then fully declare within the class e.g

I think it's a good compromise.

[alexandrehoffmann]

So using the trailing requires works for most compilers but not clang-17 and 18

[JohanMabille]

Awesome, thanks!