[Bug libstdc++/77288] New: Std::experimental::optional::operator= implementation is broken in gcc 6.1

[dawid_jurek at vp dot pl]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77288

            Bug ID: 77288
           Summary: Std::experimental::optional::operator= implementation
                    is broken in gcc 6.1
           Product: gcc
           Version: 6.1.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: dawid_jurek at vp dot pl
  Target Milestone: ---

Created attachment 39470
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=39470&action=edit
Unit tests triggering bug in operator=

0. Gcc version.

./gcc -v

Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-pc-linux-gnu/6.1.1/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: /build/gcc-multilib/src/gcc/configure --prefix=/usr
--libdir=/usr/lib --libexecdir=/usr/lib --mandir=/usr/share/man
--infodir=/usr/share/info --with-bugurl=https://bugs.archlinux.org/
--enable-languages=c,c++,ada,fortran,go,lto,objc,obj-c++ --enable-shared
--enable-threads=posix --enable-libmpx --with-system-zlib --with-isl
--enable-__cxa_atexit --disable-libunwind-exceptions --enable-clocale=gnu
--disable-libstdcxx-pch --disable-libssp --enable-gnu-unique-object
--enable-linker-build-id --enable-lto --enable-plugin
--enable-install-libiberty --with-linker-hash-style=gnu
--enable-gnu-indirect-function --enable-multilib --disable-werror
--enable-checking=release
Thread model: posix
gcc version 6.1.1 20160707 (GCC) 


1. Issue.

C++ code snippet:
   
std::experimental::optional<std::experimental::optional<std::exception_ptr>>
nested_element;
    std::experimental::optional<std::exception_ptr> element = {};
    nested_element = element;
    assert(bool(nested_element));

In above snippet assertion passes for gcc 5.1 but fails for gcc 6.1. 
Analogous code passes with Boost.Optional as well.

From http://en.cppreference.com/w/cpp/experimental/optional:
"The optional object (of type T) contains a value in the following conditions:
 -   The object is initialized with a value of type T (...)"

so nested_element should contain a value which means operator= implementation
in gcc 6.1 is broken.

2. Analysis

According to http://en.cppreference.com/w/cpp/experimental/optional/operator%3D
there are 4 overloads of optional::operator=. 
We are interested in 2 versions here:
[2] optional& operator=( const optional& other );
[4] template< class U >
optional& operator=( U&& value );

Let's back to our problematic expression:
nested_element = element;

The root cause of problem is that in gcc 6.1 case version [2] is called but in
gcc 5.1 and Boost.Optional another version is called - [4] one.

Looking deeper on broken std::experimental::optional::operator= [4]
implementation there is:

      template<typename _Up,
               enable_if_t<__and_<
                           __not_<is_same<_Up, nullopt_t>>,
                           __not_<__is_optional<_Up>>>::value,
                         bool> = true>
        optional&
        operator=(_Up&& __u)
{
   ...
}

which means overload resolution ommits this candidate because of occurence
following expression in enable_if body:
__not_<__is_optional<_Up>> 
After substitution failure another overload candidate (version [2]) is choosen
at compilation level:

  template<typename _Up,
               enable_if_t<__and_<
                 __not_<is_same<_Tp, _Up>>>::value,
                           bool> = true>
        optional&
        operator=(const optional<_Up>& __u)
        {
            if (__u)
            {
                ...
            }
            else
            {
                this->_M_reset();
            }
            return *this;
        }

In runtime during nested_element = element expression evaluation flow enters to
operator=(const optional<_Up>& __u) 
and after that to this->_M_reset(); because __u is nullopt. Now it's clear that
this->_M_engaged field is not set and finally
bool(nested_element) returns _M_engaged content which is false.

3. Solution

As we saw version [2] of operator= is not ready for handling assignment when
_Tp (type of this) is nested optional.
IMO the best what we can is forcing overload resoultion to choose version [4]
in such case as it was done in gcc 5.1 implementation.
Basing on gcc 5.1 implementation we can relax rules for overload resoultion for
operator= [4] version.

It's enaugh to replace is_optional trait by decay trait in enable_if_t from: 
__not_<__is_optional<_Up>>>::value
to:
is_same< std::decay_t<_Up>, _Tp>>::value

Now behaviour is the same as in gcc 5.1 - operator=(_Up&& __u) is chosen in 2
situations:
1. _Up is NOT optional
2. _Up is optional AND _Up is same as _Tp modulo cv-qualifiers, references etc.

I prepared impl-fix.patch for gcc 6.1 containing above change. After applying
this patch code snippet from beginning of my email is fixed and assertion pass.

4. Tests

Unfotunately existing Libstdc++-v3 unit tests are not able to detect bug I
describe here. That's why I prepared another patch - extra-tests.patch. 
This patch just add 2 simple tests in form of extra 7.cc file under
libstdc++-v3/testsuite/experimental/optional/assignment/ path.
With those patches following workflow is recommended - apply extra-tests.patch
and run all tests. 
Only unit tests from 7.cc should fail. After that please apply impl-fix.patch
and run all unit tests again. Now everything should pass.

I believe that's all. Anyway if I missed something please ask me.

Regards,
Dawid