Search code examples
arraysc++11gccrvalueexpression-templates

Assignment to an array subsection: am I assigning to an Rvalue here, and if so how do I fix it?


In the hope of making my Fortran code easier to port over to C++ one day, I've been working on some expression template code to provide whole-array arithmetic operators and the ability to copy from and assign to specified sections of longer arrays. Unfortunately I can't think of a way of coming to my question, without a fair bit of boilerplate code which I'll cut down as much as I can.

First of all I have a very simple 'C-style array struct' encapsulating a pointer and a length, suitable for being easily passed back and forth between the C, Fortran, C++ and Java parts of my mixed-language application:

typedef struct {
    int *p;    /*!< Pointer to the data */
    int n;     /*!< The number of elements; int, not size_t, for Fortran compatibility  */
} int_array_C;

typedef struct {
    float *p;    /*!< Pointer to the data */
    int n;       /*!< The number of elements; int, not size_t, for Fortran compatibility  */
} float_array_C;

typedef struct {
    double *p;   /*!< Pointer to the data */
    int n;       /*!< The number of elements; int, not size_t, for Fortran compatibility  */
} double_array_C;

...and so on for all the native types. I then define some very simple expression templates based on the approach suggested in the Wikipedia entry on that subject:

template <typename E, typename T_C >
class VecExpression
{
    typedef typename std::remove_pointer<decltype(T_C::p)>::type TT;
public:
    //! Returns a const reference to the i'th element in the array
    TT operator[] (int i) const noexcept 
    {
        return static_cast<E const&>(*this)[i];
    }

    //! Returns the total size of the array
    int size() const noexcept
    {
        return static_cast<E const &>(*this).size();
    }

    operator E&() { return static_cast<E&>(*this); }
    operator E const&() const { return static_cast<const E&>(*this); }
};

template <typename E1, typename T_C, typename E2, typename U_C  >
class VecSum : public VecExpression< VecSum<E1, T_C, E2, U_C>, T_C >
{
    E1 const & _u;
    E2 const & _v;
public:
    //! Constructor taking two VecExpressions
    VecSum(VecExpression<E1, T_C> const& u, VecExpression<E2, U_C> const &v) : _u(u), _v(v)
    {
        assert(u.size() == v.size());
    }

    int size() const noexcept { return _v.size(); }

    auto operator[](int i) const
        -> const decltype(_u[i] + _v[i]) { return _u[i] + _v[i]; }
                 // Automatically takes care of type promotion e.g. int to double
                 // according to the compiler's normal rules
};

template <typename E1, typename T_C, typename E2, typename U_C  >
VecSum<E1, T_C, E2, U_C> const operator+(VecExpression<E1, T_C> const &u,
                                         VecExpression<E2, U_C> const &v)
{
    return VecSum<E1, T_C, E2, U_C>(u, v);
}

To give me a way of manipulating the contents of my C-style vectors, I define some templates: one which manipulates data in a pre-existing buffer, and another which manages its own memory by using a std::vector:

template <typename T_C> class nArray : public T_C, public VecExpression<nArray <T_C>, T_C >
{                                                  // This is the 'curiously recurring template
                                                   // pattern' (CRTP)
    typedef typename std::remove_pointer<decltype(T_C::p)>::type TT;

    struct startingIndex : public T_C
    {
        size_t start;

        startingIndex(const T_C *initialiser) noexcept
        {
            *(static_cast<T_C *>(this)) = *initialiser;
        }

        nArray to(int element) noexcept
        {
            T_C::n = element - start + 1;
            nArray<T_C> newArray(*(static_cast<T_C *>(this)));
            return newArray;
        }
    };

public:
    //! Constructor to create an nArray from an array_C, without copying its memory
    nArray(T_C theArray) noexcept
    {
        T_C::p = theArray.p;
        T_C::n = theArray.n;
    }

    //! Constructor to create an nArray from an ordinary C array, without copying its memory
    template<std::size_t N>
    nArray(TT (&theArray)[N]) noexcept
    {
        T_C::p = &theArray[0];
        T_C::n = N;
    }

    nArray & operator=(VecExpression<nArray<T_C>, T_C> const& source) &
    {
        // Note that we cannot use the copy-and-swap idiom here because we don't have the means to
        // construct a new temporary memory buffer. Therefore we have to handle the assignment-to-self
        // case explicitly.
        if (&source == this) return *this;
        assert(T_C::n == source.size());
        for (int i=0; i<T_C::n; ++i) T_C::p[i] = source[i];
        return *this;
    }

    //! Copy assignment operator taking a VecExpression of a different (but compatible) type
    //! without allocating any new memory
    template <typename E, typename U_C>
    nArray operator=(VecExpression<E, U_C> const& source) &
    {
        assert(T_C::n == source.size());
        for (int i=0; i<T_C::n; ++i) T_C::p[i] = static_cast<TT>(source[i]);
        return *this;
    }

    //! Returns a non-const reference to the i'th element in the array
    TT& operator[] (int i) noexcept
    {
        return T_C::p[i];
    }

    //! Returns a const reference to the i'th element in the array
    const TT& operator[] (int i) const noexcept
    {
        return T_C::p[i];
    }

    startingIndex from(int element) const noexcept
    {
        startingIndex theNewArray(this);
        theNewArray.p = &T_C::p[static_cast<size_t>(element)];
        theNewArray.n = T_C::n - element;
        theNewArray.start = element;
        return theNewArray;
    }

    nArray to(int element) const noexcept
    {
        nArray theNewArray;
        theNewArray.p = T_C::p;
        theNewArray.n = element + 1;
        return theNewArray;
    }

    // ... and a whole bunch of other functions
};

template <typename T_C> class nVector : public nArray<T_C>
{
    typedef typename std::remove_pointer<decltype(T_C::p)>::type TT;

public:
    template<std::size_t N>
    nVector(TT (&source)[N]) 
    {
        contents.resize(N);
        update_basetype();
        std::copy(&source[0], &source[N], contents.begin());
    }

    // ...and a whole bunch of other constructors and assignment operators
    // which echo those of nArray with the additional step of resizing the
    // internal std::vector and copying the contents into it

private:
    void update_basetype() noexcept
    {
        T_C::p = contents.size() > 0 ? contents.data() : nullptr;
        T_C::n = contents.size();
    }

    std::vector<TT> contents;
};

typedef nArray<float_array_C> float_array;
typedef nVector<float_array_C> float_vector;

// ...and so on

Phew! From this point, I can do things like

float a[] = { 1.0f, 2.0f, 3.0f, 4.0f, 5.0f, 6.0f };
float b[] = { 9.0f, 8.0f, 7.0f, 6.0f, 5.0f, 4.0f };

float_array aArray(a);  // The array contents aren't copied, only
float_array bArray(b);  // the pointers

float_vector aVector = aArray.from(2);  // aVector is { 3.0f, 4.0f, 5.0f, 6.0f }
float_vector bVector = bArray.to(3);    // bVector is { 9.0f, 8.0f, 7.0f, 6.0f } 
float_vector cVector = aArray.from(2).to(4) + bArray.from(1).to(3);
                                        // cVector is { 11.0f, 11.0f, 11.0f } 

...and they work a treat. Now, finally, I can come to my question. Suppose I want to assign to an array subsection, for example:

float_vector dVector(10);  // An empty 10-element array
dVector.from(3).to(5) = aArray.from(2).to(4) + bArray.from(1).to(3);

As a matter of fact if I compile in Visual C++ 2013 this works just fine, but in gcc it doesn't. Compilation fails at the assignment, with the message:

error: no match for 'operator=' (operand types are 'nArray<float_array_C>' and 'const VecSum<nArray<float_array_C>, float_array_C, nArray<float_array_C>, float_array_C>')
note: candidates are:
     < ...skipping over a long list of utterly implausible options>
note: nArray<T_C>& nArray<T_C>::operator=(const VecExpression<nArray<T_C>, T_C>&) & [with T_C = float_array_C]
note: no known conversion for implicit 'this' parameter form 'nArray<float_array_C>' to 'nArray<float_array_C>&'

Now, this error message seems to crop up on the literature when trying to assign a temporary object to a non-const reference, or when trying to make an assignment to an Rvalue, and Visual C++ is documented as being slacker with respect to this rule than gcc is (with gcc being the one that conforms to the standard, naturally). I can kind-of-understand why the compiler might regard

dVector.from(3).to(5)

as an Rvalue, even though I've bent over backwards to try to prevent it from being so. For example my startingIndex::to() method diligently returns an nArray object by value, not by reference, and if I write

auto test1 = dVector.from(3).to(5);
auto test2 = aArray.from(2).to(4) + bArray.from(1).to(3);
test1 = test2;

...then this works fine and the compiler tells me that 'test1' is an 'nArray<float_array_C>' (i.e. a float_array) exactly as it should be.

So, my question is: am I in fact guilty of trying to assign to an Rvalue here? And if I am, how can I cease doing so, while still being able to make assignments to sub-arrays in this way, or at least some similarly-readable way. I truly hope that this can be done in some way in C++, otherwise I guess I'll need to go back to Fortran-land, writing

dVector(3:5) = aArray(2:4) + bArray(1:3)

...and living happily ever after.

Update

Following Chris Dodd's suggestion I experimented with a couple of different forms for a further nArray constructor and settled on:

nArray && operator=(VecExpression<nArray<T_C>, T_C> const& source) &&
{
    if (&source == this) return *this;
    assert(T_C::n == source.size());
    for (int i=0; i<T_C::n; ++i) T_C::p[i] = source[i];
    return *this;
}

(leaving the need to shore up the assignment-to-self check for the time being). The gcc compiler seemed to get past this, but the next error I got was:

no known conversion for argument 1 from 'const VecSum<nArray<float_array_C>, float_array_C, nArray<float_array_C>, float_array_C>' to 'const VecExpression<nArray<float_array_C>, float_array_C>&'

This is at least a different message (always a sign of progress) but it still implies that the basic problem is an assignment to the wrong sort of reference. Unfortunately I'm not sure where I should be looking for this: I experiemented with making my operator+ return a VecSum<E1, T_C, E2, U_C> const & rather than a by-value return, but this made precisely no difference at all. For now I'm stuck again, so my next strategy is to install clang in my Linux partition and see if I get a more helpful error message from it...

Further update:

Clang wasn't especially helpful; all it could say was:

candidate function not viable: no known conversion from 'nArray<[...]>' to 'nArray<[...]>' for object argument

which doesn't give many clues!

Final update:

I'm actually quite embarrassed about how obvious, in retrospect, the solution turned out to be. All I needed to do was to give my assignment operator exactly the same form as an ordinary move assignment operator would have:

nArray & operator=(VecExpression<nArray<T_C>, T_C> const& source) &&
{
    // Better assignment-to-self check pending...
    assert(T_C::n == source.size());
    for (int i=0; i<T_C::n; ++i) T_C::p[i] = source[i];
    return *this;
}

This of course is verbatim what Chris Dodd suggested in the first place, and works absolutely fine in clang and gcc on Linux and Windows.


Solution

  • Your nArray assignment operator:

    nArray & operator=(VecExpression<nArray<T_C>, T_C> const& source) &
    

    is explicitly defined as only applying to lvalue nArray objects (the final & on that line), and not to rvalue objects, so can't be used to assign to temporary slices such as that you get from dVector.from(3).to(5). You need an assignment operator with an rvalue reference to "this":

    nArray & operator=(VecExpression<nArray<T_C>, T_C> const& source) &&
    

    in order to be able to call this assignment operator on a temporary slice like this.

    Note that your &source == this check for self-assignment isn't adequate. You might have distinct nArray objects that refer to the same underlying storage, with different slicing. Consider what happens if you try something like

    aVector.from(3).to(7) = aVector.from(1).to(5)