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
