3
\$\begingroup\$

Here are implementations of both non-const and const view of submatrices, which is a subclass of my toy C++ matrix project: (C++20 : N-dimensional minimal Matrix class)

MatrixView.h

template <std::semiregular T, std::size_t N, bool Const>
class MatrixView final : public MatrixBase<MatrixView<T, N, Const>, T, N> {
public:
    using Base = MatrixBase<MatrixView<T, N, Const>, T, N>;
    using Base::size;
    using Base::dims;
    using Base::strides;
    using Base::applyFunction;
    using Base::applyFunctionWithBroadcast;
    using Base::operator=;
    using Base::operator+=;
    using Base::operator-=;
    using Base::operator*=;
    using Base::operator/=;
    using Base::operator%=;
    using value_type = T;
    using reference = std::conditional_t<Const, const T&, T&>;
    using const_reference = const T&;
    using pointer = std::conditional_t<Const, const T*, T*>;
    using stride_type = std::array<std::size_t, N>;

private:
    pointer data_view_;
    stride_type orig_strides_;

public:
    ~MatrixView() noexcept = default;

    explicit MatrixView(const stride_type& dims, pointer data_view, const stride_type& orig_strides);

    template <typename DerivedOther>
    MatrixView(const MatrixBase<DerivedOther, T, N>& other);

    template <typename DerivedOther, std::semiregular U> requires std::is_convertible_v<U, T>
    MatrixView& operator=(const MatrixBase<DerivedOther, U, N>& other) requires (!Const);

    friend void swap(MatrixView& a, MatrixView& b) noexcept {
        swap(static_cast<Base&>(a), static_cast<Base&>(b));
        std::swap(a.data_view_, b.data_view_);
        std::swap(a.orig_strides_, b.orig_strides_);
    }


    template <bool IterConst>
    struct MVIterator {
        using difference_type = std::ptrdiff_t;
        using value_type = T;
        using pointer = std::conditional_t<(Const || IterConst), const T*, T*>;
        using reference = std::conditional_t<(Const || IterConst), const T&, T&>;
        using iterator_category = std::random_access_iterator_tag;
        using MatViewType = std::conditional_t<(Const || IterConst), const MatrixView*, MatrixView*>;

        MatViewType ptr_ = nullptr;
        std::array<std::size_t, N> pos_ = {0};
        std::size_t offset_ = 0;
        std::size_t index_ = 0;

        MVIterator() = default;

        MVIterator(MatViewType ptr, std::array<std::size_t, N> pos = {0}) : ptr_ {ptr}, pos_ {pos} {
            ValidateOffset();
        }

        reference operator*() const {
            return ptr_->data_view_[offset_];
        }

        pointer operator->() const {
            return ptr_->data_view_ + offset_;
        }

        void ValidateOffset() {
            offset_ = std::inner_product(std::cbegin(pos_), std::cend(pos_), std::cbegin(ptr_->origStrides()), 0lu);
            index_ = std::inner_product(std::cbegin(pos_), std::cend(pos_), std::cbegin(ptr_->strides()), 0lu);
            assert(index_ <= ptr_->size());
        }

        void Increment() {
            for (std::size_t i = N - 1; i < N; --i) {
                ++pos_[i];
                if (pos_[i] != ptr_->dims(i) || i == 0) {
                    break;
                } else {
                    pos_[i] = 0;
                }
            }
            ValidateOffset();
        }

        void Increment(std::ptrdiff_t n) {
            if (n < 0) {
                Decrement(-n);
                return;
            }
            auto carry = static_cast<std::size_t>(n);
            for (std::size_t i = N - 1; i < N; --i) {
                std::size_t curr_dim = ptr_->dims(i);
                pos_[i] += carry;
                if (pos_[i] < curr_dim || i == 0) {
                    break;
                } else {
                    carry = pos_[i] / curr_dim;
                    pos_[i] %= curr_dim;
                }
            }
            ValidateOffset();
        }

        void Decrement() {
            for (std::size_t i = N - 1; i < N; --i) {
                --pos_[i];
                if (pos_[i] != static_cast<std::size_t>(-1) || i == 0) {
                    break;
                } else {
                    pos_[i] = ptr_->dims(i) - 1;
                }
            }
            ValidateOffset();
        }

        void Decrement(std::ptrdiff_t n) {
            if (n < 0) {
                Increment(-n);
                return;
            }
            auto carry = static_cast<std::size_t>(n);
            for (std::size_t i = N - 1; i < N; --i) {
                std::size_t curr_dim = ptr_->dims(i);
                pos_[i] -= carry;
                if (pos_[i] < curr_dim || i == 0) {
                    break;
                } else {
                    carry = static_cast<std::size_t>(-quot(static_cast<long>(pos_[i]), static_cast<long>(curr_dim)));
                    pos_[i] = mod(static_cast<long>(pos_[i]), static_cast<long>(curr_dim));
                }
            }
            ValidateOffset();
        }

        MVIterator& operator++() {
            Increment();
            return *this;
        }

        MVIterator operator++(int) {
            MVIterator temp = *this;
            Increment();
            return temp;
        }

        MVIterator& operator--() {
            Decrement();
            return *this;
        }

        MVIterator operator--(int) {
            MVIterator temp = *this;
            Decrement();
            return temp;
        }

        MVIterator operator+(difference_type n) const {
            MVIterator temp = *this;
            temp.Increment(n);
            return temp;
        }

        MVIterator& operator+=(difference_type n) {
            Increment(n);
            return *this;
        }

        MVIterator operator-(difference_type n) const {
            MVIterator temp = *this;
            temp.Decrement(n);
            return temp;
        }

        MVIterator& operator-=(difference_type n) {
            Decrement(n);
            return *this;
        }

        reference operator[](difference_type n) const {
            return *(*this + n);
        }

        template <bool IterConstOther>
        difference_type operator-(const MVIterator<IterConstOther>& other) const {
            return index_ - other.index_;
        }

    };

    template <bool IterConst>
    friend bool operator==(const MVIterator<IterConst>& it1, const MVIterator<IterConst>& it2) {
        return it1.ptr_ == it2.ptr_ && it1.index_ == it2.index_;
    }

    template <bool IterConst>
    friend bool operator!=(const MVIterator<IterConst>& it1, const MVIterator<IterConst>& it2) {
        return !(it1 == it2);
    }

    template <bool IterConst1, bool IterConst2>
    friend auto operator<=>(const MVIterator<IterConst1>& it1, const MVIterator<IterConst2>& it2) {
        return it1.pos_ <=> it2.pos_;
    }

    using iterator = MVIterator<Const>;
    using const_iterator = MVIterator<true>;
    using reverse_iterator = std::reverse_iterator<iterator>;
    using const_reverse_iterator = std::reverse_iterator<const_iterator>;

    iterator begin() { return iterator(this);}
    const_iterator begin() const { return const_iterator(this); }
    const_iterator cbegin() const { return const_iterator(this); }
    iterator end() { return iterator(this, {dims(0), });}
    const_iterator end() const { return const_iterator(this, {dims(0), });}
    const_iterator cend() const { return const_iterator(this, {dims(0), });}
    reverse_iterator rbegin() { return std::make_reverse_iterator(end());}
    const_reverse_iterator rbegin() const { return std::make_reverse_iterator(cend());}
    const_reverse_iterator crbegin() const { return std::make_reverse_iterator(cend());}
    reverse_iterator rend() { return std::make_reverse_iterator(begin());}
    const_reverse_iterator rend() const { return std::make_reverse_iterator(cbegin());}
    const_reverse_iterator crend() const { return std::make_reverse_iterator(cbegin());}

    [[nodiscard]] pointer dataView() const {
        return data_view_;
    }

    [[nodiscard]] const stride_type& origStrides() const {
        return orig_strides_;
    }

    MatrixView& operator-() {
        Base::Base::operator-();
        return *this;
    }

};

MatrixBase.h

template <typename Derived, std::semiregular T, std::size_t N>
class MatrixBase : public ObjectBase<MatrixBase<Derived, T, N>> {
    static_assert(N > 1);
public:
    static constexpr std::size_t ndim = N;
    using extent_type = std::array<std::size_t, N>;
    using value_type = T;
    using reference = T&;
    using const_reference = const T&;
    using pointer = T*;
    using view_type = MatrixView<T, N, false>;
    using const_view_type = MatrixView<T, N, true>;
    using row_type = MatrixView<T, N - 1, false>;
    using const_row_type = MatrixView<T, N - 1, true>;

// ... details ...

    view_type submatrix(const extent_type& pos_begin);
    view_type submatrix(const extent_type& pos_begin, const extent_type& pos_end);
    row_type row(std::size_t n);
    row_type col(std::size_t n);
    row_type operator[](std::size_t n) { return row(n); }

    const_view_type submatrix(const extent_type& pos_begin) const;
    const_view_type submatrix(const extent_type& pos_begin, const extent_type& pos_end) const;
    const_row_type row(std::size_t n) const;
    const_row_type col(std::size_t n) const;
    const_row_type operator[](std::size_t n) const { return row(n); }

// ... details ...
};

// N == 1 specialization
template <typename Derived, std::semiregular T>
class MatrixBase<Derived, T, 1> : public ObjectBase<MatrixBase<Derived, T, 1>> {
public:
    static constexpr std::size_t ndim = 1;
    using extent_type = std::array<std::size_t, 1>;
    using value_type = T;
    using reference = T&;
    using const_reference = const T&;
    using pointer = T*;
    using view_type = MatrixView<T, 1, false>;
    using const_view_type = MatrixView<T, 1, true>;
    using row_type = reference;
    using const_row_type = const_reference;
// ... details ...
};

Maintaining const-correctness correctly is extremely tedious and tricky. I posted code only relevant to techniques implementating const and non-const iterators and views. If you're interested in the full code, you can visit https://github.com/frozenca/Ndim-Matrix

Feel free to comment anything!

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Unnecessary use of using

You are publicly inheriting from MatrixBase<...>, so there is usually no need to use using Base::... to explicitly bring member functions and operator overloads into the derived class. The only reason to do this is if you are providing an overload of such a member function in the derived class with different arguments types, and still want the version from base to be available as well (by default it would be hidden).

I would strongly recommend removing the redundant usings, it makes it seem like there is something special going on when there's not, and it's just a lot of work to maintain for no gain at all.

Maintaining const-correctness

Maintaining const-correctness correctly is extremely tedious and tricky.

Indeed, it is a bit tedious. However, you can reduce the amount of code you need to write somewhat. First, instead of duplicating the whole body of a member function you need to make const and non-const overloads for, just have one with the full body, and others call the full version where possible. For example, an easy case is this:

const_iterator begin() const { return const_iterator(this); }
const_iterator cbegin() const { return begin(); }

It would also help if iterator didn't have a bool template parameter, but could automatically detect if it should be const or not. Perhaps this makes things simpler:

template <MatViewType>
requires std::same_as<std::remove_cv_t<MatViewType>, MatrixView>
struct MVIterator {
    constexpr bool IterConst = std::is_const_v<MatViewType>;
    using std::conditional_t<Const || IterConst, const T*, T*>;
    ...
    MatViewType *ptr_{};
    ...
    MVIterator(MatViewType *ptr, ...): ptr_{ptr} {...}
    ...
};

With that, iterator(this) would automatically get you a const iterator if this is const.

Perhaps MatrixView itself could determine in a similar way if it's const or not, based on whether pointer data_view is const or not.

\$\endgroup\$
3
  • 1
    \$\begingroup\$ "The explicit keyword only has an effect on constructors that take exactly one argument." I don't think that's true though? It prevents implicit conversions like return { blah, blah, blah }; which one might want to avoid. \$\endgroup\$
    – user673679
    Commented Oct 14, 2021 at 21:13
  • 1
    \$\begingroup\$ @user673679 You are right. Although normally I would not think that is something you want to avoid (it only works if the return type is known in advance and it's clear you are going to construct a new object of that type), since there's inheritance involved here, it might indeed be better to keep the explicit to avoid surprises. \$\endgroup\$
    – G. Sliepen
    Commented Oct 15, 2021 at 7:37
  • 1
    \$\begingroup\$ There is also a use for "redundant" using if the base-class depends on a template-argument, so you can use the name in the derived class. Not that that is the case for most of them here. \$\endgroup\$ Commented Oct 15, 2021 at 12:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.