xtensor-stack / xtl

The x template library
BSD 3-Clause "New" or "Revised" License
205 stars 96 forks source link

xt::xstrided_slice<std::ptrdiff_t> are not comparable #257

Open jblespiau opened 2 years ago

jblespiau commented 2 years ago

It's as simple as doing:

xt::all() == xt::all()

experimental/users/jblespiau/tensorfn/pybind11/pmap_function.cc:72:30: error: invalid operands to binary expression ('xt::xall_tag' and 'xt::xall_tag')
          bool c = xt::all() == xt::all();
                   ~~~~~~~~~ ^  ~~~~~~~~~
./third_party/xtensor/xutils.hpp:737:17: note: candidate template ignored: could not match 'tracking_allocator<T, AT, PT>' against 'xt::xall_tag'
    inline bool operator==(const tracking_allocator<T, AT, PT>&, const tracking_allocator<U, AU, PU>&)
                ^
./third_party/xtensor/xstorage.hpp:545:17: note: candidate template ignored: could not match 'uvector<T, A>' against 'xt::xall_tag'
    inline bool operator==(const uvector<T, A>& lhs, const uvector<T, A>& rhs)
                ^
./third_party/xtensor/xstorage.hpp:1270:17: note: candidate template ignored: could not match 'std::vector<T>' against 'xt::xall_tag'
    inline bool operator==(const std::vector<T>& lhs, const svector<T, N, A, Init>& rhs)
                ^
./third_party/xtensor/xstorage.hpp:1276:17: note: candidate template ignored: could not match 'svector<T, N, A, Init>' against 'xt::xall_tag'
    inline bool operator==(const svector<T, N, A, Init>& lhs, const std::vector<T>& rhs)
                ^
experimental/users/jblespiau/tensorfn/pybind11/pmap_function.cc:72:30: note: remaining 9 candidates omitted; pass -fshow-overloads=all to show them
          bool c = xt::all() == xt::all();

The type is defined as:

    template <class T>
    using xstrided_slice = xtl::variant<
        T,

        xrange_adaptor<placeholders::xtuph, T, T>,
        xrange_adaptor<T, placeholders::xtuph, T>,
        xrange_adaptor<T, T, placeholders::xtuph>,

        xrange_adaptor<T, placeholders::xtuph, placeholders::xtuph>,
        xrange_adaptor<placeholders::xtuph, T, placeholders::xtuph>,
        xrange_adaptor<placeholders::xtuph, placeholders::xtuph, T>,

        xrange_adaptor<T, T, T>,
        xrange_adaptor<placeholders::xtuph, placeholders::xtuph, placeholders::xtuph>,

        xrange<T>,
        xstepped_range<T>,

        xall_tag,
        xellipsis_tag,
        xnewaxis_tag
    >;

As a side-note, I was wondering if we should upgrade to C++ 17 to use std::variant instead of a backport. Another altnertive would be to use e.g. https://github.com/abseil/abseil-cpp/blob/master/absl/types/variant.h which will be an alias to std::variant when available.

JohanMabille commented 2 years ago

Hi,

Sorry for the late reply. The issue is indseed dut to the lack of coparison operators for the slices, that's something we should add in xtensor.

Regarding the upgrade to C++17, it has been discussed for xtensor. However, the XTL is also used by libraries which needs to support C++11/14, thus the need for a backport of std::variant. We are happy with this backport (easier to update than retrieving it from a huge library), but we could definitely add a "selection" layer here, that will simply use the standard implementation when available.