4

I have a class with a const member that needs both a move constructor and assignment.

I implemented it the following way:

struct C
{
    const int i;
    C(int i) : i{i} {}
    C(C && other) noexcept: i{other.i} {}

    C & operator=(C && other) noexcept
    {
        //Do not move from ourselves or all hell will break loose
        if (this == &other)
            return *this;

        //Call our own destructor to clean-up before moving
        this->~C();

        //Use our own move constructor to do the actual work
        new(this) C {std::move(other)};

        return *this;
    }

    //Other stuff here (including the destructor)....
}

This compiles and works as expected.

The question is whether this is the normal way to implement such a move assignment or there is a less contrived way to do it?

Equilibrius
  • 328
  • 1
  • 13
  • The requirements (const member and any form of assignment that changes the state of said member) seem contradictory. In the end, you're writing to a const thing, which should be undefined behaviour. – juanchopanza Oct 16 '19 at 13:56
  • @juanchopanza This is just a small example. In the actual class there are many other members that are not const. And, since this is a **move**, no logical state actually changes since the other (non-const) member of the moved-from class are modified to mark it as invalid. – Equilibrius Oct 16 '19 at 13:58
  • @juanchopanza And "writing to a const thing" is not undefined in this case, since the move constructor not only happily assignes to it, but it is **required** to do so, same as in the normal constructor, since there is no other way (short of using a const_cast) to initialize const members. – Equilibrius Oct 16 '19 at 13:59
  • It _is_ indefined behaviour. Assigment is not initialization. You're writing to it in an assignment operator. – juanchopanza Oct 16 '19 at 14:01
  • The move constructor *initialized* the const member - it didn't *assign* - there's an important difference. Also, for the move assignment, there *is* a logical state change in the moved-to object. – Sander De Dycker Oct 16 '19 at 14:01
  • For example : what happens if the compiler relies on the `const`-ness of the member to optimize accesses to it, and a move assignment is allowed to change the member's value anyway ? – Sander De Dycker Oct 16 '19 at 14:04
  • @juanchopanza In that case, why is the move constructor allowed to write to it? What is the different between writing to newly allocated memory and memory where some object, of the same type, lived before? – Equilibrius Oct 16 '19 at 14:09
  • @SanderDeDycker This is exactly why I invoke the move **constructor** from the assignment. And the member is **const** exactly for the reason so the compiler could optimize access to it. But how is it any different from move constructing? – Equilibrius Oct 16 '19 at 14:10
  • @Equilibrius It is allowed to write to it because the compiler isn't smart enough to figure out you're invoking UB. – juanchopanza Oct 16 '19 at 14:19
  • 1
    Invoking the move constructor isn't a magic bullet that allows you to modify a `const`. Think about it (`C obj{42};`) : if the compiler assumes that the value of `obj.i` won't change - ie. will always be `42` (it can assume that - it's `const` after all), and then you do `C obj2{666}; obj = std::move(obj2);`, suddenly `obj.i` will *actually* be `666`, which means the compiler's assumption is violated. – Sander De Dycker Oct 16 '19 at 14:20
  • @SanderDeDycker The thing is, your code is perfectly valid. In fact, this **is** how the move assignment should be used. Given: `C o{C{666}};` The sate of `o` after this is valid and contains 666, no matter what (undefined) value `i` had at the initial construction of `o`. – Equilibrius Oct 16 '19 at 14:23
  • You have 3 people here telling you the same thing in different ways, and none of them seem to sink in. May I suggest you take a few minutes to think the example from my previous comment through ? Specifically, think about how the compiler would be able to optimize access to a `const` if (according to what you assert) its value can be legally changed by invoking the move constructor. Such optimization would be impossible, and `const` wouldn't mean anything. For that reason, modifying a `const` is not allowed (in any way). – Sander De Dycker Oct 16 '19 at 14:29
  • @SanderDeDycker This code is **valid**: `struct C { const int i; C(int i) : i{i} {} C(C && other) : i{other.i + 1} {} };` Note that `i` is **modified** in the move constructor. So modifying a `const` is allowed. Otherwise how could you *initialise* it? – Equilibrius Oct 16 '19 at 14:31
  • Sure, move construction is valid (as said before, there's no `const` being modified during construction - it's being *initialized*). But we're talking about move *assignment*. – Sander De Dycker Oct 16 '19 at 14:33
  • I suspect the confusion lies in what *initialize* means : it means to set the initial value - ie. there was no value before it (it was uninitialized before it), hence no value is being modified. – Sander De Dycker Oct 16 '19 at 14:37
  • @SanderDeDycker When it is *initialized* the memory contents where it is placed is *modified*, or, in other words, *assigned* to. I try to understand the difference. – Equilibrius Oct 16 '19 at 14:37
  • @SanderDeDycker To emphasize, to my understanding *initialisation* is just an assignment to a previously undefined value. But, non the less, it **is** assignment to some value. And, please note, my `i` is **always** trivially cosntructible. It's never an object. – Equilibrius Oct 16 '19 at 14:39
  • You're conflating different abstraction layers. The `const` refers to the object. An object can be stored in memory, but just because the object is `const` doesn't mean that memory is immutable. Whatever happens to be in that memory location before the object is initialized is irrelevant - the object didn't exist yet, and hence the `const` didn't apply. As soon as the object is initialized though, the `const` has to be honored. Or iow : while the memory contents are modified during the constructor call, the object isn't (because it didn't exist yet). – Sander De Dycker Oct 16 '19 at 14:55
  • @SanderDeDycker From https://timsong-cpp.github.io/cppwp/basic.life I read that when the destructor executes the object is no longer alive. Given that in my code I do execute the destructor *before* invoking the move constructor, would it not means that I'm moving into a, basically, *uninitialized* object? So this makes invoking the move constructor valid, no? – Equilibrius Oct 16 '19 at 15:18

3 Answers3

6

Unfortunately this is undefined behavior. You cannot overwrite a const object like this and refer to it by the same name afterwards. This is covered by [basic.life]/8

If, after the lifetime of an object has ended and before the storage which the object occupied is reused or released, a new object is created at the storage location which the original object occupied, a pointer that pointed to the original object, a reference that referred to the original object, or the name of the original object will automatically refer to the new object and, once the lifetime of the new object has started, can be used to manipulate the new object, if: [...]

  • the type of the original object is not const-qualified, and, if a class type, does not contain any non-static data member whose type is const-qualified or a reference type, [...]

The simple fix is to make i non const and private and just use a getter to stop any modification from happening.

Community
  • 1
  • 1
NathanOliver
  • 171,901
  • 28
  • 288
  • 402
  • I'm not sure this applies the my case. The normal move (or any) constructor initializes the const member. This is perfectly normal and, in fact, required. The question is about the implementation of the move **assignment**. And my `i` is `const` not to restrict access (it's not used outside the object and is private in my actual implementation) but to notify the compiler that it's value will never change. Thus allowing it to catch programming error and better optimize the code. – Equilibrius Oct 16 '19 at 14:06
  • 1
    @Equilibrius This does apply because when you do `new(this) C {std::move(other)};` you emplace over a const member so you can no longer refer to that object. You'd have to do something like `auto ptr = new(this) C {std::move(other)}; return *ptr;` but even then you still can't use the object in your calling code. – NathanOliver Oct 16 '19 at 14:11
  • I'm still finding it difficult to understand why this is invalid. I think it is safe to assume that when the compiler actually invokes a move **constructor** it also uses the same emplace mechanism, since the destination memory is already allocated. And, in that case, there is no problem using the moved-to object after that. – Equilibrius Oct 16 '19 at 14:17
  • I can not really understand how my case is different. The moved-to object does not moves in memory. It only changes some internal state. From "https://en.cppreference.com/w/cpp/language/new": Such allocation functions are known as "placement new", ... void* operator new(std::size_t, void*), which simply returns its second argument unchanged. – Equilibrius Oct 16 '19 at 14:17
  • @Equilibrius Think about *The moved-to object does not moves in memory. It only changes some internal state*. You are putting a new value in memory that should not change. How should the machine handle that? C++ says it doesn't and you can only get this new value by access through a pointer or reference to the `new` object you create. – NathanOliver Oct 16 '19 at 14:20
  • I'll quote the code from my comment in the question thread: `struct C { const int i; C(int i) : i{i} {} C(C && other) : i{other.i + 1} {} };` How is this handled? Given that when the move constructor starts execution there is already some, undefined, value at the memory location of `i`. And the constructor changes that value. This is `const`, not `constexpr`. So there is actually memory allocated. – Equilibrius Oct 16 '19 at 14:36
  • @Equilibrius I'm not sure what more I can say here. The standard explicitly states this is not allowed. The construction is allowed because until you reach `i{other.i + 1} ` there is no actual `i` object. That line creates and initializes it in one go. – NathanOliver Oct 16 '19 at 14:39
  • I'm sorry. I'm just really confused about this. I can understand why this is not allowed if `i` is not trivially constructible. And I understand why this actually works in my case. Your quote sais that "and, if a class type, does not contain any non-static data member". But my `i` is **not** a class type. It's always int. – Equilibrius Oct 16 '19 at 14:43
  • @Equilibrius `i` is not a class type but `C` is. You can no longer refer to the `C` object and that is transitive to `i` since `i` is a member of the `C` object you just invalidated. – NathanOliver Oct 16 '19 at 14:44
  • @Equilibrius note that the standard no longer refers to const sub-objects but only the complete object. https://timsong-cpp.github.io/cppwp/basic.life#8 This is likely because a complete object can be placed in ROM but not objects that are heterogeneous. As such the lifetime can be ended and a new object with different const values constructed. – doug Apr 06 '22 at 02:26
2

After some research I came to the following.

There are related questions:

The relevant information was found in:

What clarified this for me was the example in https://en.cppreference.com/w/cpp/utility/launder

struct X {const int i; };
X * p = new X{0};
X * np = new (p) X{1};

This will result in undefined behavior:

const int i = p->i

But the following is valid:

const int i = np->i;

Per my original question, a modified version of the move assignment would be needed:

struct C
{
    const int i;
    C() : i{} {}
    C(C && other) noexcept: i{other.i} {}

    C & operator=(C && other) noexcept
    {
        if (this == &other) return *this;
        this->~C(); //Ok only if ~C is trivial
        return *(new(this) C {std::move(other)});
    }
}

C a;
C b;
b = std::move(a);

//Undefined behavior!
const int i = b.i; 

This would work as expected but would result in undefined behavior for the following reasons.

When the destructor is invoked the objects' lifetime ends. Following that it is safe to call the move constructor. But at any point the compiler is free to assume that the content of b never changes. Thus, by using our move assignment we have a contradiction that results in undefined behavior.

On the other hand, although the return value from the placement new is the same as this, when the compiler performs access through that, returned, pointer/reference it must not assume anything about that object.

Given that C& C::operator=(C&&) returns the result of the placement new, the following should be valid (but not really useful).

const int i = (b = std::move(a)).i;

Thank to @NathanOliver, whos' answer was the correct one all along and, also, to him and @SanderDeDycker for playing brain ping-pong with me.

Equilibrius
  • 328
  • 1
  • 13
  • Good summary. Glad you made it through the maze that is the C++ standard :) – Sander De Dycker Oct 17 '19 at 07:04
  • Me too. I like standards. :-) At the end of the day the class that had this move assignment really did not need it at all. The only place I used it was in a test that checked this move assignment works. Catch 22. How ethical will it be to accept my own answer in this case instead of the one by @NathanOlver ? – Equilibrius Oct 17 '19 at 08:11
  • Accepting your own answer is [allowed with special rules](https://stackoverflow.blog/2009/01/06/accept-your-own-answers/). Additionally, you as the asker are typically best positioned to determine which answer best answered the question (if any). In a case like this where another correct answer exists, I'd personally ask if they are ok with you accepting your own instead - just to avoid ruffling feathers. But that's a personal preference - there's no official guidance afaik. – Sander De Dycker Oct 17 '19 at 09:32
  • I accepted @NathanOliver's answer. I guess I agree with you on the "another correct answer". Thanks for all the help! – Equilibrius Oct 17 '19 at 09:45
2

As of c++20, this can now be done without UB. This is because the basic.life now allows replacing objects that contain consts. Prior to c++20 this was disallowed.

Secondly, lack of UB can be proven using constexpr compile time evaluation. Compilers are required to detect UB in compile time evaluation. However, placement new is still not consteval so while legal in regular code w/o UB, it can't be checked in compile time functions. But c++20 provides 2 functions that will work, std::destroy_at and std::construct_at.

This code allows assignment using your example:

#include <memory>
struct C
{
    const int i;
    constexpr C(int i) : i{ i } {}
    constexpr C(const C& other) noexcept = default;
    constexpr C& operator=(const C& other) noexcept
    {
        //Do not move from ourselves or all hell will break loose
        if (this == &other)
            return *this;

        //Call our own destructor to clean-up before moving
        std::destroy_at(this);
        std::construct_at(this, other);

        return *this;
    }
};

// Compilers are required to detect any UB evaluating this
consteval int foo()
{
    C a(1), b(2);   // create two difference objects
    C c(b);         // CTOR
    a = c;
    return a.i;     // returns 2;
}

int main()
{
    return foo();
}
doug
  • 3,840
  • 1
  • 14
  • 18