How to copy (or swap) objects of a type that contains members that are references or const?
Asked Answered
R

6

12

The problem I am trying to address arises with making containers such as an std::vector of objects that contain reference and const data members:

struct Foo;

struct Bar {
  Bar (Foo & foo, int num) : foo_reference(foo), number(num) {}
private:
  Foo & foo_reference;
  const int number;
  // Mutable member data elided
};

struct Baz {
  std::vector<Bar> bar_vector;
};

This won't work as-is because the default assignment operator for class Foo can't be built due to the reference member foo_reference and const member number.

One solution is to change that foo_reference to a pointer and get rid of the const keyword. This however loses the advantages of references over pointers, and that const member really should be const. They are private members, so the only thing that can do harm is my own code, but I have shot myself in the foot (or higher) with my own code.

I've seen solutions to this problem on the web in the form of swap methods that appear to be chock full of undefined behavior based on the wonders of reinterpret_cast and const_cast. It happens that those techniques do appear to work on my computer. Today. With one particular version of one particular compiler. Tomorrow, or with a different compiler? Who knows. I am not going to use a solution that relies on undefined behavior.

Related answers on stackoverflow:

So is there a way to write a swap method / copy constructor for such a class that does not invoke undefined behavior, or am I just screwed?

Edit
Just to make it clear, I already am quite aware of this solution:

struct Bar {
  Bar (Foo & foo, int num) : foo_ptr(&foo), number(num) {}
private:
  Foo * foo_ptr;
  int number;
  // Mutable member data elided
};

This explicitly eliminates the constness of number and the eliminates the implied constness of foo_reference. This is not the solution I am after. If this is the only non-UB solution, so be it. I am also quite aware of this solution:

void swap (Bar & first, Bar & second) {
    char temp[sizeof(Bar)];
    std::memcpy (temp, &first, sizeof(Bar));
    std::memcpy (&first, &second, sizeof(Bar));
    std::memcpy (&second, temp, sizeof(Bar));
}

and then writing the assignment operator using copy-and-swap. This gets around the reference and const problems, but is it UB? (At least it doesn't use reinterpret_cast and const_cast.) Some of the elided mutable data are objects that contain std::vectors, so I don't know if a shallow copy like this will work here.

Riha answered 28/9, 2011 at 8:52 Comment(6)
Could you not do a placement new on this, thereby invoking a copy constructor (or special swap constructor)?Kaylil
@edA-qa mort-ora-y: Hmmm. I certainly can write a valid copy constructor. Show me!Riha
First note, you can't memcpy a reference. C++11 says a struct with a reference is not a standard-layout type, thus memcpy on a reference is UB.Kaylil
Look at spraff's answer. It uses a move version, but you can do the same thing using a normal copy constructor/assignment (non-move).Kaylil
I looked. I upvoted. I haven't accepted it yet; I'm holding out on a C++03 solution for a while. Since I did not say "no C++11 solutions, please", it wouldn't be fair to reject that solution if it is the only one that is both valid and avoids moving away from reference / const data members.Riha
Shouldn't classes with const- and reference-members be noncopyable anyways? Atleast that's what I learned. That said, move-swap should work just fine, but that's not C++03.Conny
W
5

If you implement this with move operators there is a way:

Bar & Bar :: operator = (Bar && source) {
    this -> ~ Bar ();
    new (this) Bar (std :: move (source));
    return *this;
}

You shouldn't really use this trick with copy constructors because they can often throw and then this isn't safe. Move constructors should never ever throw, so this should be OK.

std::vector and other containers now exploit move operations wherever possible, so resize and sort and so on will be OK.

This approach will let you keep const and reference members but you still can't copy the object. To do that, you would have to use non-const and pointer members.

And by the way, you should never use memcpy like that for non-POD types.

Edit

A response to the Undefined Behaviour complaint.

The problem case seems to be

struct X {
    const int & member;
    X & operator = (X &&) { ... as above ... }
    ...
};

X x;
const int & foo = x.member;
X = std :: move (some_other_X);
// foo is no longer valid

True it is undefined behaviour if you continue to use foo. To me this is the same as

X * x = new X ();
const int & foo = x.member;
delete x;

in which it is quite clear that using foo is invalid.

Perhaps a naive read of the X::operator=(X&&) would lead you to think that perhaps foo is still valid after a move, a bit like this

const int & (X::*ptr) = &X::member;
X x;
// x.*ptr is x.member
X = std :: move (some_other_X);
// x.*ptr is STILL x.member

The member pointer ptr survives the move of x but foo does not.

Wes answered 28/9, 2011 at 10:15 Comment(20)
I think this works just fine for copying as well (slight syntax change) -- I don't think he meant to imply he doesn't want to be able to copy. I'm trying to figure out if placement new on this is valid for all types of objects, and if not, which ones?Kaylil
This is close to what I'm after. Upvoted since I didn't say "C++03 only" in my question. Is there any way to do this without moving to C++11? (I don't have that luxury. Our code has to be compilable with a C++03 compiler.)Riha
Re And by the way, you should never use memcpy like that for non-POD types. I know that. I posted that mainly because code like that is rampant on the 'net; I didn't want someone offering it as a solution. My current implementation is option A, don't use references and const data members. Option B, invoke undefined behavior, is a non-starter to me. I would like Option C, the magical version that works in C++03, and does so without invoking option A and without invoking UB.Riha
This is still liable to result in UB, at least in C++03. There's something in the section on object lifecycle that says that if you destroy and recreate a const object, then references (and pointers) to it taken beforehand are no longer valid. That's so that the compiler can cache the values of const objects. So doing this invalidates any references to this that the caller held before calling operator=.Radbourne
But copying not a problem even in C++03, right? Bar(const Bar &other) : foo_reference(other.foo_reference), number(other.number) {} works fine. The only problem is assignment. So do you really need a move constructor here? Could you just use placement new with the copy constructor?Hypotonic
@Nemo: Something like Bar& Bar::operator=(const Bar & src) {this->~Bar(); new(this) (src); return *this;} ? That's very very close, but what if the copy constructor throws?Riha
@Steve: Your comment makes it sound like there is no option C in C++03. IMO, PIMPL, an internal class managed with auto pointers, inheriting from a base class, ...: These are workarounds just to skirt a hole in the language. C++11 patches that hole with move semantics. So, I'll stick with option A for now (smallest kludge around the hole), put in a deferred ticket to fix when we finally switch to C++, ... and accept this answer.Riha
@David: I'm not claiming that this doesn't lead to UB in C++11, merely that it does in C++03. I suspect the same language is still there, I just haven't checked. I think you're abusing the meaning of const - you say you want this data member to never change, but then you say you want to change the whole object, including that data member, to new values. That's what assignment does, it modifies the target, so if the target is unmodifiable you shouldn't do it. It's like saying that you want an int with an unmodifiable sign bit, but you also want to assign new values to it.Radbourne
@Steve, still in C++11. 3.8.7 says its okay so long as you don't use const or reference types. I'm guessing all together what the submitter wants is guaranteed UB in some respect. That sucks.Kaylil
@edA-qamort-ora-y: thanks, that's the same as C++03, in my brief description above I said "const object" but it says the same about const and reference members. Crucially: "the name of the original object ... can be used to manipulate the new object, if ... the type of the object ... does not contain any non-static data member whose type is const-qualified". So for example Bar a; a = std::move(something); a.number; is UB in C++11. It sucks for the questioner, but I think only because he doesn't fully appreciate the true glory of const and/or what value semantics actually involve.Radbourne
@SteveJessop, I can understand the reason why const is subject to this rule, but that reference is also affected doesn't seem to make sense. The committee must still be thinking of some theoretical system that doesn't just implement references as pointers. Silly.Kaylil
@edA-qamort-ora-y: Or they're thinking of actual systems that implement references T& as T *const. The issue is that the language says references aren't reseatable, and therefore optimizers will proceed on that basis. If they were reseatable then they wouldn't have to play the same role const does in messing up assignments. So I think you're basically saying it's "silly" that references aren't reseatable, which IMO isn't silly but is a whole separate argument from whether this code is valid :-)Radbourne
@edA-qamort-ora-y: Most systems do implement references as pointers... const pointers :-)Hypotonic
@SteveJessop, this shouldn't cause problems for the optimizer. The pointer value doesn't change, only the value it points to. Nothing is attempting to change the address.Kaylil
@edA-qamort-ora-y: Oh, I see, so if Bar::foo_reference is a reference data member, then you say assigning to an instance of Bar should assign to the referand of foo_reference, not re-seat the reference? If that's what you want, then you can just do it in the assignment operator, no need for this destruct-construct business. Nothing in the language stops you, it's just not what the default assignment operator does. I guess it was thought that it would be a bit surprising, e.g. if "child" holds a ref to parent, you wouldn't expect assigning to the child to modify the parent.Radbourne
See edits for my 2 cents on this reference malarky. @Steve, I think you're complaining about use cases which are broken irrespective of whether the technique in my answer is used. It's just another dangling reference problem and talking about what the optimiser might do with wrong code is missing the point.Wes
@spraff: the problem case is as I said, Bar a; a = std::move(something); a.number;, or any other use of a after calling your move assignment operator, is UB. That's not the expected effect of assignment, to invalidate the target. Bar a; Bar &new_a = (a = std::move(something)); new_a.number; would be OK, but that is not what callers want to have to type. It also doesn't play well with containers, where people expect to be able to do, e.g., Bar &b = barvec[0]; b = something; and continue using b.Radbourne
Referencing a after the move into it is fine. There's a sequence point there! If this was a problem, it would be a problem for all Bar even without const/reference members.Wes
@spraff: you are mistaken. It has nothing to do with sequence points, and everything to do with the the const/reference members, as in 3.8/7 of C++11 cited by edA-qa mort-ora-y earlier.Radbourne
@edA-qamort-ora-y "The committee must still be thinking of some theoretical system" No. A reference is not rebindable just like a const value is not modifiable.Higbee
L
6

You can't reseat the reference. Just store the member as a pointer, as it is done in all other libraries with assignable classes.

If you want to protect yourself from yourself, move the int and the pointer to the private section of a base class. Add protected functions to only expose the int member for reading and a reference to the pointer member (e.g to prevent yourself from treating the member as an array).

class BarBase
{
    Foo* foo;
    int number;
protected:
    BarBase(Foo& f, int num): foo(&f), number(num) {}
    int get_number() const { return number; }
    Foo& get_foo() { return *foo; }
    const Foo& get_foo() const { return *foo; }
};

struct Bar : private BarBase {
  Bar (Foo & foo, int num) : BarBase(foo, num) {}

  // Mutable member data elided
};

(BTW, it doesn't have to be a base class. Could also be a member, with public accessors.)

Lineament answered 28/9, 2011 at 9:14 Comment(7)
This is the solution I have already implemented. It would be nice to have a solution that doesn't involve changing the reference to a pointer or removing the const qualifier.Riha
@David: That solution would necessarily involve modifying a const value and reseating a reference which you simply can't do legally.Lineament
@David: And nothing in your question seems to indicate that you are aware of this solution. As far as Bar is concerned, foo is (only accessible as) a reference and number is (only accessible as) const. - If you say you want to go even further than this, then it is getting paranoid.Lineament
This was in the question from the very start: One solution is to change that foo_reference to a pointer and get rid of the const keyword.Riha
@David: But you seem to be missing that this is all in a base class / member class. The non-const integer and the pointer are completely inaccessible to your class. BarBase is the entire class, restricting access while allowing assignment is all it does.Lineament
I see what you're doing here. As with Steve's answer, this is viable, but is less desirable to me than the admittedly kludgy option A. Not desirable because more code, another class to test, and it too is a kludge around a hole in the language. So, upvoted but not selected.Riha
It is a hole in the language that it doesn't let you reseat references and modify constants?! Or do you mean it is a hole in the language that it doesn't have a hole here whereas normally there is a hole for everything?Supine
W
5

If you implement this with move operators there is a way:

Bar & Bar :: operator = (Bar && source) {
    this -> ~ Bar ();
    new (this) Bar (std :: move (source));
    return *this;
}

You shouldn't really use this trick with copy constructors because they can often throw and then this isn't safe. Move constructors should never ever throw, so this should be OK.

std::vector and other containers now exploit move operations wherever possible, so resize and sort and so on will be OK.

This approach will let you keep const and reference members but you still can't copy the object. To do that, you would have to use non-const and pointer members.

And by the way, you should never use memcpy like that for non-POD types.

Edit

A response to the Undefined Behaviour complaint.

The problem case seems to be

struct X {
    const int & member;
    X & operator = (X &&) { ... as above ... }
    ...
};

X x;
const int & foo = x.member;
X = std :: move (some_other_X);
// foo is no longer valid

True it is undefined behaviour if you continue to use foo. To me this is the same as

X * x = new X ();
const int & foo = x.member;
delete x;

in which it is quite clear that using foo is invalid.

Perhaps a naive read of the X::operator=(X&&) would lead you to think that perhaps foo is still valid after a move, a bit like this

const int & (X::*ptr) = &X::member;
X x;
// x.*ptr is x.member
X = std :: move (some_other_X);
// x.*ptr is STILL x.member

The member pointer ptr survives the move of x but foo does not.

Wes answered 28/9, 2011 at 10:15 Comment(20)
I think this works just fine for copying as well (slight syntax change) -- I don't think he meant to imply he doesn't want to be able to copy. I'm trying to figure out if placement new on this is valid for all types of objects, and if not, which ones?Kaylil
This is close to what I'm after. Upvoted since I didn't say "C++03 only" in my question. Is there any way to do this without moving to C++11? (I don't have that luxury. Our code has to be compilable with a C++03 compiler.)Riha
Re And by the way, you should never use memcpy like that for non-POD types. I know that. I posted that mainly because code like that is rampant on the 'net; I didn't want someone offering it as a solution. My current implementation is option A, don't use references and const data members. Option B, invoke undefined behavior, is a non-starter to me. I would like Option C, the magical version that works in C++03, and does so without invoking option A and without invoking UB.Riha
This is still liable to result in UB, at least in C++03. There's something in the section on object lifecycle that says that if you destroy and recreate a const object, then references (and pointers) to it taken beforehand are no longer valid. That's so that the compiler can cache the values of const objects. So doing this invalidates any references to this that the caller held before calling operator=.Radbourne
But copying not a problem even in C++03, right? Bar(const Bar &other) : foo_reference(other.foo_reference), number(other.number) {} works fine. The only problem is assignment. So do you really need a move constructor here? Could you just use placement new with the copy constructor?Hypotonic
@Nemo: Something like Bar& Bar::operator=(const Bar & src) {this->~Bar(); new(this) (src); return *this;} ? That's very very close, but what if the copy constructor throws?Riha
@Steve: Your comment makes it sound like there is no option C in C++03. IMO, PIMPL, an internal class managed with auto pointers, inheriting from a base class, ...: These are workarounds just to skirt a hole in the language. C++11 patches that hole with move semantics. So, I'll stick with option A for now (smallest kludge around the hole), put in a deferred ticket to fix when we finally switch to C++, ... and accept this answer.Riha
@David: I'm not claiming that this doesn't lead to UB in C++11, merely that it does in C++03. I suspect the same language is still there, I just haven't checked. I think you're abusing the meaning of const - you say you want this data member to never change, but then you say you want to change the whole object, including that data member, to new values. That's what assignment does, it modifies the target, so if the target is unmodifiable you shouldn't do it. It's like saying that you want an int with an unmodifiable sign bit, but you also want to assign new values to it.Radbourne
@Steve, still in C++11. 3.8.7 says its okay so long as you don't use const or reference types. I'm guessing all together what the submitter wants is guaranteed UB in some respect. That sucks.Kaylil
@edA-qamort-ora-y: thanks, that's the same as C++03, in my brief description above I said "const object" but it says the same about const and reference members. Crucially: "the name of the original object ... can be used to manipulate the new object, if ... the type of the object ... does not contain any non-static data member whose type is const-qualified". So for example Bar a; a = std::move(something); a.number; is UB in C++11. It sucks for the questioner, but I think only because he doesn't fully appreciate the true glory of const and/or what value semantics actually involve.Radbourne
@SteveJessop, I can understand the reason why const is subject to this rule, but that reference is also affected doesn't seem to make sense. The committee must still be thinking of some theoretical system that doesn't just implement references as pointers. Silly.Kaylil
@edA-qamort-ora-y: Or they're thinking of actual systems that implement references T& as T *const. The issue is that the language says references aren't reseatable, and therefore optimizers will proceed on that basis. If they were reseatable then they wouldn't have to play the same role const does in messing up assignments. So I think you're basically saying it's "silly" that references aren't reseatable, which IMO isn't silly but is a whole separate argument from whether this code is valid :-)Radbourne
@edA-qamort-ora-y: Most systems do implement references as pointers... const pointers :-)Hypotonic
@SteveJessop, this shouldn't cause problems for the optimizer. The pointer value doesn't change, only the value it points to. Nothing is attempting to change the address.Kaylil
@edA-qamort-ora-y: Oh, I see, so if Bar::foo_reference is a reference data member, then you say assigning to an instance of Bar should assign to the referand of foo_reference, not re-seat the reference? If that's what you want, then you can just do it in the assignment operator, no need for this destruct-construct business. Nothing in the language stops you, it's just not what the default assignment operator does. I guess it was thought that it would be a bit surprising, e.g. if "child" holds a ref to parent, you wouldn't expect assigning to the child to modify the parent.Radbourne
See edits for my 2 cents on this reference malarky. @Steve, I think you're complaining about use cases which are broken irrespective of whether the technique in my answer is used. It's just another dangling reference problem and talking about what the optimiser might do with wrong code is missing the point.Wes
@spraff: the problem case is as I said, Bar a; a = std::move(something); a.number;, or any other use of a after calling your move assignment operator, is UB. That's not the expected effect of assignment, to invalidate the target. Bar a; Bar &new_a = (a = std::move(something)); new_a.number; would be OK, but that is not what callers want to have to type. It also doesn't play well with containers, where people expect to be able to do, e.g., Bar &b = barvec[0]; b = something; and continue using b.Radbourne
Referencing a after the move into it is fine. There's a sequence point there! If this was a problem, it would be a problem for all Bar even without const/reference members.Wes
@spraff: you are mistaken. It has nothing to do with sequence points, and everything to do with the the const/reference members, as in 3.8/7 of C++11 cited by edA-qa mort-ora-y earlier.Radbourne
@edA-qamort-ora-y "The committee must still be thinking of some theoretical system" No. A reference is not rebindable just like a const value is not modifiable.Higbee
R
3

that const member really should be const

Well, then you can't reassign the object, can you? Because that would change the value of something that you've just said really should not change: before the assigment foo.x is 1 and bar.x is 2, and you do foo = bar, then if foo.x "really should be const" then what's supposed to happen? You've told it to modify foo.x, which really should not be modified.

An element of a vector is just like foo, it's an object that the container sometimes modifies.

Pimpl might be the way to go here. Dynamically allocate an object (the "impl") containing all your data members, including const ones and references. Store a pointer to that object (the "p") in your object that goes in the vector. Then swap is trivial (swap the pointers), as is move assignment, and copy assignment can be implemented by constructing a new impl and deleting the old one.

Then, any operations on the Impl preserve the const-ness and un-reseatable-ness of your data members, but a small number of lifecycle-related operations can act directly on the P.

Radbourne answered 28/9, 2011 at 11:15 Comment(4)
Upvoted as a viable solution, but not selected. I'm not a big fan of PIMPL, and this is a bit of a kludge around a hole in language that C++11 solves.Riha
I don't think C++11 does solve it, although I could be wrong.Radbourne
Unless you want to hide the "impl" declaration and implementation, pimpl is functionally equivalent to a class having disabled copy and assign, managed by a unique_ptr or shared_ptr depending on needs.Extravert
@Emilio: yes, the difference is just whether the user chooses the smart pointer, or whether the public API presents something with value semantics that happens to involve some smart pointer under the covers.Radbourne
M
2

This however loses the advantages of references over pointers

There is no advantage. Pointers and reference are different, but none is the better one. You use a reference to ensure that there is a valid instance and a pointer if passing a nullptr is valid. In your example you could pass a reference and store a pointer

struct Bar {
   Bar (Foo & foo) : foo_reference(&foo) {}
private:
   Foo * foo_reference;
};
Moke answered 28/9, 2011 at 8:58 Comment(4)
That is what I am already doing, and I would like to avoid this if possible. There are lots of implemenations of swap on the net that do swap references -- but they invoke UB. It works, on their computer, with their compiler, and it works on mine too. But it is still UB.Riha
References are different. They may yield the same final machine code, but the semantics are different.Wes
Exactly. A reference as a data member is fairly close in behavior to a const pointer that is known to be non-null. Changing the reference to a const pointer doesn't solve the problem; it's still const.Riha
The expression "the advantages of X over Y" commonly refers to whatever differences are advantageous to the user, for a particular purpose. To say that X has advantages over Y does not mean that X is strictly better than Y in every way. Indeed, the same difference might be an "advantage of X over Y" for one purpose, but an "advantage of Y over X" for another.Radbourne
S
0

You can compose your class of members that take care of those restrictions but are assignable themselves.

#include <functional>

template <class T>
class readonly_wrapper
{
    T value;
public:
    explicit readonly_wrapper(const T& t): value(t) {}
    const T& get() const { return value; }
    operator const T& () const { return value; }
};

struct Foo{};

struct Bar {
  Bar (Foo & foo, int num) : foo_reference(foo), number(num) {}
private:
  std::reference_wrapper<Foo> foo_reference;  //C++11, Boost has one too
  readonly_wrapper<int> number;
  // Mutable member data elided
};

#include <vector>
int main()
{
  std::vector<Bar> bar_vector;
  Foo foo;
  bar_vector.push_back(Bar(foo, 10));
};
Supine answered 28/9, 2011 at 16:8 Comment(0)
M
0

I know this is a relatively old question to dig out, but I have needed something similar recently, and it made me wonder if this is possible to implement using contemporary C++, e.g. C++17. I've seen this blog post by Jonathan Boccara and fiddled with it a little, i.e. by adding implicit conversion operators and this is what I have gotten:

#include <optional>
#include <utility>
#include <iostream>
#include <functional>

template <typename T>
class assignable
{
public:
    assignable& operator=(const assignable& rhs)
    {
        mHolder.emplace(*rhs.mHolder);
        return *this;
    }

    assignable& operator=(assignable&&) = default;

    assignable(const assignable&) = default;
    assignable(assignable&&) = default;

    assignable(const T& val)
    : mHolder(val)
    {}

    assignable(T&& val)
    : mHolder(std::move(val))
    {}

    template<typename... Args>
    decltype(auto) operator()(Args&&... args)
    {
        return (*mHolder)(std::forward<Args>(args)...);
    }

    operator T&() {return *mHolder;}
    operator const T&() const {return *mHolder;}
private:
    std::optional<T> mHolder;
};

template <typename T>
class assignable<T&>
{
public:
    explicit assignable(T& val)
    : mHolder(val)
    {}

    operator T&() {return mHolder;}
    operator const T&() const {return mHolder;}

    template<typename... Args>
    decltype(auto) operator()(Args&&... args)
    {
        return mHolder(std::forward<Args>(args)...);
    }

private:
    std::reference_wrapper<T> mHolder;
};

The part some may find controversial is the behaviour of the assignment operator for reference types, but the discussion about rebinding the reference vs the underlying object has been going on there for a while already I think, so I just did according to my taste.

Live demo: https://godbolt.org/z/WjW7s34jn

Mariannmarianna answered 10/10, 2021 at 10:18 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.