Hi Andrzej, thanks for your response. I do not get the same result as you using latest visual studio (as for today 15.6.4 with std=latest), I'm not sure on what would be the correct behavior but to me it seems erroneous that the unique_ptr has lost it's value. Note that using std::optional yields the same result as below. auto func0() { return boost::make_optional(std::make_unique<int>(42)); } const auto& x = func0().value(); assert(x); // true assert(*x = 42); // true I agree that declaring x as a reference rather than a value is sketchy, this is just how I happened to find the bug, I suppose there are other cases which might be errorous. While on the subject I think that boost::optional should add the member function has_value() in addition to is_initialized() to make it syntactically compatible with std::optional. best regards/Viktor On Fri, Mar 23, 2018 at 3:36 AM Andrzej Krzemienski <akrzemi1@gmail.com> wrote:

Hi Viktor, Thank you for reporting this. This is an omission (a bug) that I will need to fix. I have created an issue in GitHub to track it: https://github.com/boostorg/optional/issues/51.

On the other hand, I do not think that the bug is causing an UB any more than function `value()` in your example. Unless something has changed in the way temporaries' life is prolonged in C++17. I tested it for `int`s and all tests work fine. I tested it on `unique_ptr<int>`:

``` #include <memory> #include <iostream> #include <boost/optional.hpp>

using boost::optional;

optional<std::unique_ptr<int>> func() { return boost::make_optional(std::unique_ptr<int>(new int{3})); }

int main() { auto const& p = func().value(); std::cout << (bool)p << std::endl; // outputs: 0 } ```

And even when I use `value()` I do not get the intuitive result. I would recommend catching the result by value rather than by reference, and all surprises will be gone, including the one from your example.

Regards, &rzej;

2018-03-23 2:04 GMT+01:00 Viktor Sehr via Boost <boost@lists.boost.org>:

...

Both *value()* and *operator** has an overload to handle if the *optional<T>* object is an r-value. However *get()* does not have the r-value overload.

This leads to undefined behavior using *get()*, but not with *operator** and *value()*, in the following example:

...

auto func0() { return optional<int>{3}; } auto func1() { const auto& x = func0().get(); // Undefined behavior const auto& y = func0().value(); // Correct const auto& z = *func0(); // Correct }

Is this intended, I can't see any reason why the r-value overload of .get() would be missing? Otherwise, the following overload of .get() should be added:

reference_type_of_temporary_wrapper get() && { BOOST_ASSERT(this->is_initialized()); return boost::move(this->get_impl()); }

/Viktor Sehr, Software Developer at Toppluva

_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Andrzej Krzemienski wrote:

...

const auto& x = func0().value(); ... I am convinced that you are hitting an UB. Temporary returned by func0() is destroyed in the first line.

Interesting question. value() && returns T&&. The auto const& reference can't bind directly to T&&, because that's not an lvalue. So a temporary value of type T is created, and the auto const& binds to that, extending its life. That's of course assuming I read http://eel.is/c++draft/dcl.init.ref correctly.

Matt Calabrese wrote:

On Fri, Mar 23, 2018 at 12:01 PM, Peter Dimov via Boost <boost@lists.boost.org> wrote:

...

Interesting question. value() && returns T&&. The auto const& reference can't bind directly to T&&, because that's not an lvalue.

An lvalue-reference-to-const can bind to an rvalue (it's why you can pass an rvalue to a function that takes a "const T&").

You're right, it does bind directly. http://eel.is/c++draft/dcl.init.ref#5.3.1

Viktor Sehr wrote:

Clang (6.0) emits "ud2" instructions for both auto&& and const auto&. If the const auto& can bind directly to an r-value, this must mean Clang is wrong, or am I missing something?

"ud2" is Clang's way of encoding undefined behavior. This is an invalid instruction that crashes. Both auto&& and auto const& do the same thing - bind to the rvalue reference returned by value() &&.

GCC on the other hand emits code for both functions, although with slight difference (I'm not skilled enough in assembler to analyze the GCC output in further detail)

Code example: https://godbolt.org/g/7fjszE <https://godbolt.org/g/uNJoZB>

GCC crashes too, in another creative way. It returns *p, where the pointer p is [rsp+8], which is NULL. If you change the functions to int * func1() { const auto& x = func0().value(); return &*x; } you'll see that Clang returns NULL and GCC returns the value in [rsp+8] which is also NULL. So both compilers deliberately dereference a NULL pointer because they see that the reference `x` is no longer valid.

Viktor Sehr wrote:

Thanks for the answer, but just so I understand, you said a const-reference binds directly to a r-value as when passing an r-value to a const reference argument. Doesn't this mean that both Clang and GCC compiles this erroneously?

No, both are correct; optional::value()&& returns an rvalue reference to the value inside the optional, to which the auto&& or auto const& binds. When the optional temporary is destroyed, this reference becomes dangling.

auto temp_uptr = std::move(temp_optional.value());

There is no temp_uptr. For there to have been, value()&& would have needed to return T, not T&&. But it doesn't. It's roughly template<class T> class optional { T t_; publlic: T&& value()&& { return std::move(t_); } }; so in auto temp_optional = boost::make_optional(std::make_unique<int>(42)); const auto& x = temp_optional.value(); x binds to temp_optional.t_.

On Sun, Mar 25, 2018, 18:12 Gavin Lambert via Boost <boost@lists.boost.org> wrote:

On 25/03/2018 13:50, Peter Dimov wrote:

...

No, both are correct; optional::value()&& returns an rvalue reference to the value inside the optional, to which the auto&& or auto const& binds. When the optional temporary is destroyed, this reference becomes dangling.

Doesn't this problem go away if value()&& is removed entirely, or at least changed to return by value instead?

Granted I haven't done much delving into the darker corners of C++11, but I've usually found that outside of implementing std::move itself, anything that returns a T&& is probably a bug waiting to happen.

...

T&& value()&& { return std::move(t_); }

In this case, this should be just as efficient:

T value()&& { return std::move(t_); }

Especially with improved elision guarantees in more recent standards and compilers.

I do not recommend doing that. It has different semantics from the other overloads and can be a pessimization for certain types. I think the correct answer here, as harsh as it sounds, is "use the API correctly". I do not see a defect here.

2018-03-26 0:12 GMT+02:00 Gavin Lambert via Boost <boost@lists.boost.org>:

On 25/03/2018 13:50, Peter Dimov wrote:

...

No, both are correct; optional::value()&& returns an rvalue reference to the value inside the optional, to which the auto&& or auto const& binds. When the optional temporary is destroyed, this reference becomes dangling.

Doesn't this problem go away if value()&& is removed entirely, or at least changed to return by value instead?

Granted I haven't done much delving into the darker corners of C++11, but I've usually found that outside of implementing std::move itself, anything that returns a T&& is probably a bug waiting to happen.

T&& value()&& { return std::move(t_); }

...

In this case, this should be just as efficient:

T value()&& { return std::move(t_); }

Especially with improved elision guarantees in more recent standards and compilers.

However, note that the only problem it would solve is the problem that does not exist when you declare objects rather than references in your scope (in cases you are not interested in object's identity): ``` auto p = func().value(); ``` And everything is correct, shorter, an reflects you intentions more clearly ("I am interested in value, not in address"). Regards, &rzej;