Duccio
Duccio

Reputation: 163

attempting to reference a deleted function when passing by reference with a const member to a class

This is sort of an extension to the question attempting to reference a deleted function when passing by reference with a const member

I read that and I do understand the explanations given about the MoveConstructible (or at least, I believe), but I cannot figure out why, given the following code:

struct vertex {
    const double x;
    const double y;
    const double z;
};

const std::vector<vertex> test_vertices()
{
    std::vector<vertex> vertices;

    vertices.emplace_back(vertex(-3.0, -3.0, 0.0));
    vertices.emplace_back(vertex(3.0, -3.0, 0.0));
    vertices.emplace_back(vertex(0.0, 0.0, 1.5));
}

this code works flawlessly:

std::vector<vertex> panel_vertices = test_vertices();
draw(panel_vertices);

but this other code does not and returns a "C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.23.28105\include\xutility(1762,1): error C2280: 'p_geometry::vertex &p_geometry::vertex::operator =(const p_geometry::vertex &)': attempting to reference a deleted function" (ignore namespaces, they are ok in the code):

class PanelView {

    std::vector<vertex> vertices;
public:

    PanelView(int x, int y, int w, int h, const char* l = 0);
    void setVertices(const std::vector<vertex>& vertices): vertices(vertices) {};
    void draw() {
        other_draw(vertices);
    };
};

I have no sort operator here, but besides that, I'm missing something, because both snippets of code appear to do the same thing.

UPDATE

After your responses (thanks!!) I tried to clarify both my thoughts and my doubts. I tried several configurations, and I can get that the copy operator is deleted when there're const members (a pretty dumb behavior imho, because when you create a copy you're not trying to modify anything from the original, but that's outside of the scope of this question).

I came down with the following four scenarios [1, 2, 2bis, 3]:

#include <iostream>

struct vertex {
    const double x;
    const double y;
    const double z;
};

const std::vector<vertex> test_vertices()
{
    std::vector<vertex> vertices;
    vertices.push_back(vertex {-3.0, -3.0, 0.0});
    vertices.emplace_back(vertex{3.0, -3.0, 0.0});
    vertices.emplace_back(vertex{0.0, 0.0, 1.5});
    return vertices;
}

class PanelView {
    std::vector<vertex> mvertices;
    std::vector<vertex> *mvpointer;
public:

    //[1] PanelView(const std::vector<vertex>& vertices): mvertices(vertices) {};
    //[2] void setVertices(const std::vector<vertex>& v)  {mvertices = v;  };
    //[2 bis] void setVertices(std::vector<vertex> v)  {mvertices = v;  };
    //[3] void setVerticesPointer(std::vector<vertex> *v)  {mvpointer = v;  };
    void draw() {std::cout<<mvertices[0].x; };
    void drawp() {std::cout<<(*mvpointer)[0].x; };
};


int main()
{

    std::vector<vertex> panel_vertices = test_vertices();

    //[1] Works fine
    //PanelView pv(panel_vertices);
    //pv.draw();

    //[2 and 2 bis] - Error C2280: 'vertex &vertex::operator =(const vertex &)': deleted
    //PanelView pv;
    //pv.setVertices(panel_vertices);

    //[3] - OK
    PanelView pv;
    pv.setVerticesPointer(&panel_vertices);
    pv.drawp();


    std::cout<<panel_vertices[0].x;
}

The PanelView constructor [1] and the setVertices method [2 and 2 bis] do the same exact thing, take "const std::vector& vertices" as input and copy it (or what?) to the class attribute "std::vector mvertices;".

First doubt: why with the setVertices method it fails, while with the constructor it does not? What is the difference "behind the scene"?

Second doubt: I always believed that in C++ passing by reference and passing by pointer was basically the same, but passing by reference was more C++-style and avoided all the boilerplate code, making it more readable. Why the compiler complains with [2 (by reference)], but is ok with [3 with pointers]?

Upvotes: 1

Views: 1345

Answers (2)

mfnx
mfnx

Reputation: 3018

First of all, let's correct some errors in your code:

vertices.emplace_back(vertex(-3.0, -3.0, 0.0));

You construct an instance of vertex with a user-defined constructor that does not exist: vertex::vertex(double,double,double). You could solve that problem by implementing the constructor vertex::vertex(double,double,double), or by using aggregate initialization:

vertices.emplace_back(vertex {-3.0, -3.0, 0.0}); // C++11
vertices.emplace_back(vertex {3.0, -3.0, 0.0});
vertices.emplace_back(vertex {0.0, 0.0, 1.5});

Another line which is actually no valid C++ code:

void setVertices(const std::vector& vertices): vertices(vertices) {};

My best guess is you meant something like:

void setVertices(std::vector<vertex> v) { vertices = v; }

Well, this code would not compile since you have const members in your struct vertex. In other words, it is impossible to change the value of any of the members of vertex, and hence, it is impossible to change any of the instances of vertex in your vector vertices. The compiler would tell you something like: error: static assertion failed: type is not assignable. This problem can be solved by removing the const keyword.

The error attempting to reference a deleted function is due to the above mentioned: the copy constructor (or copy assignment operator) is deleted because you cannot assign an instance of vertex to another instance of vertex.

gcc 10.0.0: error: static assertion failed: type is not assignable

clang 10.0.0: copy assignment operator of 'vertex' is implicitly deleted because field 'x' is of const-qualified type 'const double' const double x;

As a side note, the signature of test_vertices should read like this:

std::vector<vertex> test_vertices() // remove the const, as you return a local variable by value

And, more importantly, your setVertices function should read like this:

void setVertices(std::vector<vertex> v) { vertices = std::move(v); } // C++11

And hey, then everything suddenly compiles, with or without the const members of vertex. Why is that? Because you never try to copy an instance of vertex. The argument std::vector<vertex> v is initialized when setVertices is called, but no copy assignment or constructor is involved. The argument std::vector<vertex> v is simply initialized with another vector. Furthermore, the user can now decide whether to move that other vector or create a copy of it (without invoking the copy instructor :p):

int main()
{
    std::vector<vertex> my_vertices = test_vertices();
    setVertices(my_vertices); // no copy constructor, v gets initialized with my_vertices. But, now there exists a copy of my_vertices (which is v)
    setVertices(std::move(my_vertices); // move my_vertices into v
    return 0;
}

Last but not least, I would like to reiterate a comment of @Phil1970:

It does not make much sense to have all data members const as you could instead have none const and make the whole struct const wherever appropriate.

This means that, instead of adding the const type qualifier to your members of vertex, you could simply remove it and use an object of const vertices:

const vertex> my_vertex;

Then again, you cannot do:

std::vector<const vertex> my_vertices;

since items in a vector must be assignable.

Anyways, you probably don't need the members of vertex to be const.

Answers to your updated question.

First doubt: why with the setVertices method it fails, while with the constructor it does not? What is the difference "behind the scene"?

PanelView(const std::vector<vertex>& vertices): mvertices{vertices} {}

This works because you are not calling the copy constructor. You just are initializing mvertices, and this is exactly the source of your confusion.

vertex v1 = {-3.0, -3.0, 0.0}; // this is not an assignment, it is an initialization
vertex v2; // declaration -> the members are initialized with default values
v2 = v1; // assignment -> the copy constructor is called: error

So, in the last line, you are trying to change the default values of v2 by those of v1, which is not possible as your members are const. However:

vertex v3 = v2;

Here v3 is initialized with v2 and no copying occurs.

That said, the constructor should read:

PanelView(std::vector<vertex> v) : mvertices{std::move(v)} {}

Your setVertices does not work because you assign v to mvertices, hence you are calling the copy constructor, which is implicitly deleted. You should move v into mvertices as explained earlier.

Second doubt: I always believed that in C++ passing by reference and passing by pointer was basically the same, but passing by reference was more C++-style and avoided all the boilerplate code, making it more readable. Why the compiler complains with [2 (by reference)], but is ok with [3 with pointers]?

A reference variable can be said as another name for an already existing variable. Pointers, on the other hand, are variables that store the address of a variable. And you can copy the pointer as many times you want. However, that doesn't mean you are copying the object itself. You should be careful with such practices, as several pointers would point to the same object, but I'll consider this out of scope. You should avoid raw pointers to vectors.

Upvotes: 1

AbbysSoul
AbbysSoul

Reputation: 403

The simple answer is that in your example operator= - which is deleted - is not invoked and that is why it works. For contrast:

setVertices(const std::vector<vertex>& v)

takes an existing vector and copy each value, one by one, into a new vector vertices. Well at least this is the code compiler attempts to generate is something like:

vertices.reserve(v.size);
for (size_t i = 0; i < v.size(); ++i) {
 vertices[i] = v[i];
}

Note the above code expects an operator= to assign vertices[i] = v[i]; Also, note :) This is not the actual code that STL would generate - it is only for illustration point of where operator= is expected. Actual code is most likely to use generic copy algorithm and iterators.

As to your example - vertices.emplace_back(vertex(0.0, 0.0, 1.5)); does not requere operator= as it calls copy constructor - which I believe is generated by default. Copy constructor is called via placement new.

Upvotes: 2

Related Questions