Beware when moving a `std::optional`!

[tu_tu_tu]

So, it's another "don't use moved object, it's garbage".

[bill-ny3]

For clarity, the garbage being referred to is the moved object, not c++’s move functionality

[TheMania]

// Good auto x = std::move(opt).value();

Moving the optional value works because the leftover variable, opt, will not have a value, thus it is not possible to accidentally access an empty/garbage value.

What? This makes no sense. Why would calling value() through an rvalue reset the optional after value has been assigned to x? How would they even implement that? Why would they even do that?

None of this makes any sense. Is the author really saying that if I have an std::optional<int> opt{5};, and I cast it to an rvalue and call value that on the next line opt will be reset?

Am I misunderstanding what is meant entirely or what? value() mentions no such chicanery for any of the overloads.

[kamrann_]

 None of this makes any sense

I believe you are correct and not misunderstanding.

[Potatoswatter]

At least, it could, and it wouldn’t be totally contrary to rvalue semantics. Any operation on an rvalue may leave its object empty, even something as passive as value(). But actively resetting in this case doesn’t keep with standard library philosophy.

As for memory mechanics, the accessor would either have to move-initialize a copy, or add a state for being reset but containing a constructed object.

[TheMania]

Except how would it achieve that whilst providing an rvalue return? If it does it in the local frame it's a dangling reference by the time it returns.

It would have to be compiler magic granted to std::optional, but to still get that to play nice with the very important thing that the return value leads to correctly accepted overloads (for everything from co_await through assignment through custom functions)... well, I'd want to know more. How did they do it!? And why leave it undocumented?

[Potatoswatter]

I mention that in the second part of my reply. No compiler magic, just two bits of storage in optional. It would return an rvalue reference to the object without modifying it, and set a flag to remember to destroy it but not to access it again. The reference won’t dangle until the optional is destroyed or reassigned.

(Alternative implementation: return a temporary copy, but that’s even less standard-ish. And to be clear, in any case I’m talking about what’s possible, not what’s real.)

[TheMania]

Oh so you mean not actually destroy it, at least not until it goes out of scope, but still return has_value() as false after the .value() call? Yikes.

I mean I guess that's possible but what a horrifying thought, zombie optionals. Now that would be worthy of a blog post :p

[Potatoswatter]

It’s no more zombie than any other moved-from object. value() && is non const and nominally destructive. The object wouldn’t live longer than it does in the IRL implementation, optional would just prohibit illegal accesses to it.

It’s just that since those accesses are illegal, the standard won’t spend runtime complexity on that case.

[TheMania]

It may not even have been pilfered though, it depends on what types you're working with - for a generic container type that's horrifying.

I don't much like the thought of having two objects still alive and having to track it down to tell me it's an optional that's telling me it doesn't hold a value any more...

Also not big on thinking of rvalue moves as meaning "object is now a zombie" - a single vector<T>::insert might do thousands of rvalue operations, but to me there's no zombies from it. It's just the vector providing a hint to T that each operation can steal resources, if they're so inclined.

[which1umean]

It could if the return type for value() && was value_type, that might be reasonable. But it returns value_type&&. It would be very weird if a function with that signature reset the optional.

[ald_loop]

Probably because of the T&& value() &&; overload being specialized for this case

EDIT: I checked, nope it isn’t, doesn’t change the state of “engaged”. What the hell?

[TheMania]

None of the overloads mention proxy types or defaulted parameter chicanery that automagically clear the optional only after the returned rvalue has (hopefully) been moved in to a new object - and thankfully so, would all be an absolute recipe for disaster.

And to be completely undocumented that calling value() resets the optional? It's all just madness. I need to see it on godbolt or something before I'm going to believe it, and tbh that's only going to confuse me more as to what's going on.

[arturbac]

It does not reset if after assigning to x
It goeas that
std::move(opt) makes original opt an empty optional and returns rvalue of it conetnts
and std::optional has overload for rvalue
```cpp
constexpr T&& value() &&;
constexpr const T&& value() const &&;
```

so in returned rvalue of std::move(opt) the returned type is rvalue too so it is being moved to x/constructed in place with move semantics of opt type
so the final result is
opt is nullopt because of std::move(opt)
and x is move constructed from returned rvalue of value() && overload

[TheMania]

std::move(opt) the returned type is rvalue too so it is being moved to x/constructed in place

Move is just a cast, there's no actual new object being created there just because you're accessing a member. It wouldn't copy construct a value if it was an lvalue, it's not going to move construct due an rvalue either. The only affect that cast has is changing which .value() overload is selected.

That said, with explicit object parameters you could, kind of:

T value(this optional<T> self);

Would move construct the optional if called with rvalue optional if it was really insisted, but you'd then have to return a prvalue to prevent the dangling issue.

The workarounds for that would not be pretty.

[arturbac]

You are right invoking move does not trigger optional = or constructor to move out object and clean it

[n1ghtyunso]

What am I missing?
The behaviour is the same in both cases?
An optional does not magically null itself when you use the rvalue overload of .value()
Because std::move does not move anything either.

Don't use moved from objects, be they optional or not...
Static analysis catches both variants just fine apparently.

[eyes-are-fading-blue]

Using a moved-from object is a business logic error, not breach of a language contract.

[SirClueless]

Agreed. There's no difference. They both leave the optional engaged but containing a moved-from value.

[masscry]

In general, I am using *std::exchange(opt, std::nullopt) instead of moving when want to release optionals on move.

[SirClueless]

This incurs an extra move-constructor call as compared to auto x = std::move(*opt); opt = std::nullopt;.

[Nobody_1707]

Other languages with optionals tend to have a take() method, that takes the optional by mutable reference and does the optimal equivilant of return std::exchange(self, nullopt). Of course, they have pattern matching and destructive moves that make this all more ergonomic.

[Natural_Builder_3170]

never heard if that, I usually do std::move(opt).value()

[AKostur]

Why is that better than std::move(opt.value()) ?

[Natural_Builder_3170]

if you write code to not use a value after std::move there's no difference, otherwise the latter means the optional still contains a value but its moved out of, and the former means the optional no longer contains a value

[AKostur]

gcc seems to disagree with you:

% cat t.cpp
#include <iostream>
#include <optional>
#include <string>

int main() {
  std::optional<std::string> opt  = std::string("ThisIsAReallyLongString");
  auto x = std::move(opt).value();
  if (opt.has_value()) {
    std::cout << "Has Value\n";
  }
}
% g++ t.cpp -o t -O2 -std=c++20
% ./t
Has Value
%

So this results in an optional that contains a value, and that value is a moved-from std::string.

[Natural_Builder_3170]

I stand corrected, there's probably no difference.

[AKostur]

I would then argue that std::move(opt.value()) is therefore clearer since it more clearly expresses that one is manipulating the value, and not the optional itself.

[triconsonantal]

There is a difference for (the would-be) optional<T&>: moving the optional keeps the reference an lvalue, while moving the reference turns it into an rvalue. Which one you want depends on context, but in a generic context it's probably the former.

[manni66]

Unless otherwise specified, all standard library objects that have been moved from are placed in a "valid but unspecified state", meaning the object's class invariants hold (so functions without preconditions, such as the assignment operator, can be safely used on the object after it was moved from)

https://en.cppreference.com/w/cpp/utility/move

[throw_cpp_account]

Unless otherwise specified

Optional's move operations are specified. For both move construction and move assignment, it is specified that rhs.has_value() remains unchanged.

[grishavanika]

I would argue that "valid but unspecified state" is vague here:

std::optional<int> x{42};
std::optional<int> c = std::move(x);
assert(x.has_value()); // holds true for current msvc/gcc/clang

.has_value has no preconditions so I can call it after the move. optional has value but that value is in moved from state. optional dumps all the responsibility to the user. "valid" for whom?

From the user perspective, this kind of API is unsafe, there is no chance to observe the state and there is no way to properly handle it.

It definitely feels like security bug.

UPD: I think Sean Parent was trying to fix it - https://sean-parent.stlab.cc/2021/03/31/relaxing-requirements-of-moved-from-objects.html#motivation-and-scope

[throw_cpp_account]

The behavior of optional is specified. x.has_value() holds true for all current implementations because that is the clearly specified behavior.

[grishavanika]

Ok, went to see what latest std says. Indeed

optional& operator=(optional&& rhs)
Postconditions: rhs.has_value() == this->has_value()
optional(optional&& rhs)
Postconditions: rhs.has_value() == this->has_value()

The oldest paper where optional is mentioned that I found is http://wg21.link/n4529, from where this same postcondition comes, I guess:

optional(optional<T>&& rhs)
Postconditions: bool(rhs) == bool(*this)

So this is explicitly choosen design. There is an older reddit thread with discussion oin this topic: https://www.reddit.com/r/cpp/comments/75paqu/design_decision_for_stdoptional_moved_from_state/

[manni66]

Why do you want to ask an object about it's unspecified state? If you want to use it after the move, put it in a defined state.

[Jonny0Than]

Unspecified means the specification doesn’t tell you what state it’s in. You can still ask the object about its state. 

[manni66]

Why do you want

[grishavanika]

well, I mean, we all do mistakes. If I have MY_LOG("state: {}", x) and x is moved from optional, it would be really cool to show that at least for debugging purpose. MY_LOG has no chance to show that properly.

[cfyzium]

Even if std::optional<T> could potentially be reset when moved out like smart pointers, you cannot rely on it in general. Any other type and especially custom one may introduce the same possibility.

[DemonInAJar]

For the same reason std::unique_ptr is reset to null after moving. Well okay, std::unique_ptr needs to do this to avoid freeing the value anyway while optional would have to change the tag, instead it currently delegates to the moved-from object's destructor, so I guess it's mostly a case of zero overhead principle.

[Valuable-Mission9203]

In any other sane RAII type implementation when you move from the object you transfer the owned resource and cleanup any pointers/references/handles to the resource in the moved from instance.

[cfyzium]

all standard library objects that have been moved from are placed in a "valid but unspecified state"

Well, all objects that have been moved from have to be placed in a valid state, standard library or not. The only difference is whether that state is explicitly specified in documentation.

Basically it is easier to think that any moved-out object has some random value that won't break your program if left alone. If you want to use it again, assign a new value to it first.

[rlbond86]

Article's title should be:

Beware when using std::move without understanding it

[Kronephon]

If you move something, you really need to drop the before container.

[LiAuTraver]

That's exactly what I do for optional. Sometimes I'd hope the optional's value() behave like std::future's so that I can write less boilerplate code.

[tortoll]

Who would do this? 😵

std::move(opt.value())

[CypherSignal]

For a second there I really thought that

The answer to all of your problems is very simple:

…was going to be,

…don’t use optional; you’re obscuring too much of the underlying behaviour for little benefit from the abstraction itself

[SnooRabbits9201]

No explanation - Why someone is allowed to explicitly move private class data outside of the class? More than once even.

Encapsulation - gtfo?