3

I have a matrix class with a transposedView() method that I have been using for years as a "zero overhead" conversion between row and column vectors.

template<int M, int N=M, typename T = double>
struct mat {
    std::array<T,M*N> buf;
    // ...
    template<int Md = M, int Nd = N, typename = std::enable_if_t<Md == 1 || Nd == 1>>
    const mat<N, M, T>& transposedView() const {
        static_assert(M == 1 || N == 1, "transposedView() supports only vectors, not general matrices.");
        return *reinterpret_cast<const mat<N, M, T>*>(this);
    }
}

I used to trust this because the memory layout of mat<1,N> corresponds exactly to mat<N,1>, but I have just learned that this function has Undefined Behavior. Do you have any advice on what I could replace the contents/implementation of this function with?

Museful
  • 6,711
  • 5
  • 42
  • 68
  • A `std::vector(M*N)` where you do the x,y calculation to access the cell. – Eljay Jul 29 '21 at 01:40
  • Analyze what the cast is doing (and _why_ you are doing it). It's usually a little costly to do the conversion properly but ... a little more context may help. Why cast at all? – Ted Lyngmo Jul 29 '21 at 01:44
  • 1
    @TedLyngmo I'm doing the cast because any row or column vector is supposed to have exactly the same memory layout as its own transpose, so I cast as I feel it is unnecessary to create a new object. – Museful Jul 29 '21 at 01:48
  • "_supposed to have exactly the same memory layout_" - but, then you wouldn't need to cast, would you? – Ted Lyngmo Jul 29 '21 at 01:49
  • 1
    I suggest that you create a 1D array and map your matrices on top of that. – Ted Lyngmo Jul 29 '21 at 01:51
  • @TedLyngmo Yes I have to perform some kind of cast because row vectors and column vectors are different data types. For example if `a` is a `mat<2,1>` and `b` is a `mat<1,2>`, then `a*b` is a `mat<2,2>` while `b*a` is a `mat<1,1>`. I say this just to explain that `a` and `b` need to have different data types even if they have the same memory layout. – Museful Jul 29 '21 at 01:54
  • @TedLyngmo `mat::buf` is exactly that 1D array, if I understand you correctly. – Museful Jul 29 '21 at 01:56
  • 2
    FWIW, while this is formally UB in most (all? haven't seen this break) compilers this will just work as long as `buf` is your only data member, or as long as it is a trivial type. – NathanOliver Jul 29 '21 at 01:57
  • If this has been working for years I would trust it works. If you want future reassurance add more unit tests. Doing large rewrites of fundamental pieces has its own risks way those against of leaving it alone. – Martin York Jul 29 '21 at 02:06
  • @MartinYork For the sake of argument: It has been working for years and doesn't anymore because of assumtions. – Ted Lyngmo Jul 29 '21 at 02:08
  • You can redesign your member as `std::unique_ptr>`, and implement `transposedView` by constructing a new `mat` by moving the member. This is cheap, though not zero-overhead. – xskxzr Jul 29 '21 at 02:36
  • 1
    @xskxzr `transposedView` implies a view on an existing object. That is, the data should remain in place and the new object should allow the user to interact with it as-if it had a different format. If you `move` the internals, it is not longer a view, it is a transformation. – François Andrieux Jul 29 '21 at 13:16
  • @NathanOliver: It will work on any compilers that subscribe to the spirit of C principle "Don't prevent the programmer from doing what needs to be done". Unfortunately, both C and C++ have progressed from "There's no need to mandate that compilers support a construct in cases where it would be useful, because nobody would buy a compiler that did otherwise" to "The lack of mandated support for the construct means that there's never been a reason for well-written programs to use it", and the fact that people choosing compilers are often no longer the people writing code for them... – supercat Jul 30 '21 at 15:50
  • ...means that compiler writers who interpret the Standards as an excuse to behave nonsensically when given code that commercial compilers would process meaningfully will be shielded from market pressures that would otherwise deter such behavior. – supercat Jul 30 '21 at 15:51

1 Answers1

1

You can design a new view class template, like std::string_view corresponding to std::string, or Eigen::Map corresponding to Eigen::Matrix (so you can see, this is a common design in C++). The view class template may simply hold a pointer pointing to the beginning of the sequence it views and a size (if you only want to support a view for a whole matrix, you can omit the size member as it can be inferred from its template parameters). Your transposedView function can return such a view class instead.

template<int M, int N=M, typename T = double>
struct mat_view {
    T *begin;
    std::size_t size;
    // member functions you want to support
    // ...
}

template<int M, int N=M, typename T = double>
struct mat {
    std::array<T,M*N> buf;
    // ...
    template<int Md = M, int Nd = N, typename = std::enable_if_t<Md == 1 || Nd == 1>>
    const mat_view<N, M, T> transposedView() const {
        static_assert(M == 1 || N == 1, "transposedView() supports only vectors, not general matrices.");
        return {std::begin(buf), M * N};
    }
}
xskxzr
  • 12,442
  • 12
  • 37
  • 77