3

I have the following small class:

/// RAII wrapper for a Lua reference
class reference
{
public:
    /// Construct empty reference
    reference() : m_L(NULL), m_ref(LUA_NOREF) {}

    /// Construct reference from Lua stack
    reference(lua_State* L, int i = -1) : m_L(L) {
        lua_pushvalue(L, i);
        m_ref = luaL_ref(L, LUA_REGISTRYINDEX);
    }

    /// Destructor
    ~reference() {
        if (m_L) luaL_unref(m_L, LUA_REGISTRYINDEX, m_ref);
    }

    /// Copy constructor
    reference(const reference& r) : m_L(r.m_L) {
        r.push();
        m_ref = luaL_ref(m_L, LUA_REGISTRYINDEX);
    }

    /// Move constructor
    reference(reference&& r) : m_L(r.m_L), m_ref(r.m_ref) {
        r.m_L = NULL; // make sure r's destructor is NOP
    }

    /// Assignment operator
    reference& operator=(reference r) {
        swap(r, *this);
        return *this;
    }

    /// Swap with other reference
    friend void swap(reference& a, reference& b)
    {
        std::swap(a.m_L,   b.m_L);
        std::swap(a.m_ref, b.m_ref);
    }

    void push() const { lua_rawgeti(m_L, LUA_REGISTRYINDEX, m_ref); }

private:        
    lua_State* m_L;
    int        m_ref;
};

Note that the assignment operator is implemented using the copy-and-swap idiom and is supposed to call the move constructor, if used with an rvalue.

However, reference r; r = reference(L); calls the copy constructor before entering the assignment operator. Why, oh why?

Writing two assignment operators helps:

    /// Assignment operator
    reference& operator=(const reference& r) {
        reference copy(r);
        swap(copy, *this);
        return *this;
    }

    /// Move assignment operator
    reference& operator=(reference&& r) {
        swap(r, *this);
        return *this;
    }

However, at the cost of disabling copy elision.

Isn't pass-by-value supposed to work here as expected? Or is even my compiler (Clang on Mac) broken?

Update:

The following small test-case works correctly:

#include <iostream>

using namespace std;

struct resource
{
    resource(int i=1) : i(i) { print(); }
    ~resource() { print(); i = 0; }

    void print() const
    {
        cout << hex << " " << uint16_t(uintptr_t(this)) << ") " << dec;
    }

    int i;
};

resource* alloc_res()
{
    cout << " (alloc_res";
    return new resource(0);
}

resource* copy_res(resource* r)
{
    cout << " (copy_res";
    return new resource(r->i);
}

void free_res(resource* r)
{
    if (r) cout << " (free_res";
    delete r;
}

struct Test
{
    void print() const
    {
        cout << hex << " [&="   << uint16_t(uintptr_t(this))
             << ", r=" << uint16_t(uintptr_t(r)) << "] " << dec;
    }

    explicit Test(int j = 0) : r(j ? alloc_res() : NULL) {
        cout << "constructor"; print();
        cout << endl;
    }

    Test(Test&& t) : r(t.r) {
        cout << "move"; print(); cout << "from"; t.print();
        t.r = nullptr;
        cout << endl;
    }

    Test(const Test& t) : r(t.r ? copy_res(t.r) : nullptr) {
        cout << "copy"; print(); cout << "from"; t.print();
        cout << endl;
    }

    Test& operator=(Test t) {
        cout << "assignment"; print(); cout << "from"; t.print(); cout << "  ";
        swap(t);
        return *this;
        cout << endl;
    }

    void swap(Test& t)
    {
        cout << "swapping"; print();
        cout << "and"; t.print();
        std::swap(r, t.r);
        cout << endl;
    }

    ~Test()
    {
        cout << "destructor"; print();
        free_res(r);
        cout << endl;
    }

    resource* r;
};

int main()
{
    Test t;
    t = Test(5);
}

If compiled with clang++ --std=c++11 -O0 -fno-elide-constructors test.cpp -o test the move constructor is called. (Thanks for the switch, Benjamin Lindley)

The question is now: why does it work now? What's the difference?

Community
  • 1
  • 1
marton78
  • 3,684
  • 2
  • 23
  • 36
  • What is `L` in `r = reference(L)`? Is that a lua_state pointer? – Benjamin Lindley Apr 24 '12 at 14:03
  • Yes, it's a `lua_State*` – marton78 Apr 24 '12 at 14:04
  • 1
    Are you compiling with optimizations enabled? – interjay Apr 24 '12 at 14:07
  • 1
    I've tried this code with g++ 4.6.3 and the copy constructor isn't called. – Vaughn Cato Apr 24 '12 at 14:18
  • 1
    Try it with optimizations then. The compiler is allowed (not required) to elide copies, so it might only do so when optimizing. – interjay Apr 24 '12 at 14:18
  • 2
    The question seems to be about why move is not happening though. Move is required when applicable, optimizations or not. – Benjamin Lindley Apr 24 '12 at 14:22
  • @Benjamin: Exactly. Clang is rather aggressive about copy elision, I created a test case of the code above without the Lua stuff, but it's unusable, as clang *always* elides the copy, even wit `-O0`. With the lua stuff, however, it doesn't, at least it should move! – marton78 Apr 24 '12 at 14:29
  • @VaughnCato: Is the move constructor called with GCC or is the copy/move elided entirely? – marton78 Apr 24 '12 at 14:29
  • @marton78: GCC 4.6 elides for me. But with -fno-elide-constructors, it moves. – Benjamin Lindley Apr 24 '12 at 14:41
  • Same thing with Clang 3.2 built from trunk, so it might be a bug from the earlier release. – Nikola Smiljanić Apr 24 '12 at 14:43
  • @marton78: Are you sure it's calling the *copy* constructor and not the regular constructor that takes a `lua_State` (which, btw, should be marked *`explicit`*)? – Nicol Bolas Apr 24 '12 at 14:48
  • @NicolBolas: yeah, you're right about the `explicit`, I forgot that, thanks. The code does call the copy constructor, I sprinkled `std::cout`s over it. – marton78 Apr 24 '12 at 14:59
  • @marton78: Can you pin down exactly where that copy constructor is being called from? With a breakpoint or something? – Nicol Bolas Apr 24 '12 at 15:09
  • @NicolBolas: You bet I did that. In the debugger it's hard to see of course, but the `cout`s indicate it happens after constructing the temporary and before entering `operator=` – marton78 Apr 24 '12 at 15:25

1 Answers1

1

There is no legal C++11 circumstance that would cause the calling of a copy constructor in r = reference(L);.

This is effectively equivalent to r.operator =(reference(L));. Since operator= takes its parameter by value, one of two things will happen.

  1. The temporary will be used to construct the value. Since it's a temporary, it will preferentially call reference's move constructor, thus causing a move.
  2. The temporary will be elided directly into the value argument. No copying or moving.

After this, operator= will be called, which doesn't do any copying internally.

So this looks like a compiler bug.

Nicol Bolas
  • 378,677
  • 53
  • 635
  • 829
  • Thanks. Another indication is that the example code from the update in the original question works. However, I didn't find the respective bug llvm's Bugzilla. – marton78 Apr 24 '12 at 15:27