Compile failure on new test/optional for CCE with -std=c++2a

[Dan Bonachea]

Automated testing reports CCE 9.0.0 on cori KNL is failing the new test/optional:

+ /global/cscratch1/sd/hargrove/upcnightly-knl/EX-cori_knl-aries-cray_old/runtime/work/dbg/upcxx-inst/bin/upcxx -o optional-seq /global/cscratch1/sd/hargrove/upcnightly-knl/EX-cori_knl-aries-cray_old/runtime/work/dbg/upcxx/test/optional.cpp -Wall -Wno-unused
In file included from /global/cscratch1/sd/hargrove/upcnightly-knl/EX-cori_knl-aries-cray_old/runtime/work/dbg/upcxx/test/optional.cpp:17:
In file included from /opt/gcc/8.1.0/snos/lib/gcc/x86_64-suse-linux/8.1.0/../../../../include/g++/functional:61:
In file included from /opt/gcc/8.1.0/snos/lib/gcc/x86_64-suse-linux/8.1.0/../../../../include/g++/unordered_map:46:
In file included from /opt/gcc/8.1.0/snos/lib/gcc/x86_64-suse-linux/8.1.0/../../../../include/g++/bits/hashtable.h:37:
In file included from /opt/gcc/8.1.0/snos/lib/gcc/x86_64-suse-linux/8.1.0/../../../../include/g++/bits/node_handle.h:39:
/opt/gcc/8.1.0/snos/lib/gcc/x86_64-suse-linux/8.1.0/../../../../include/g++/optional:568:4: error: call to implicitly-deleted copy constructor of 'std::_Optional_payload<MoveAware<int>, true, false, false>'
        : _Optional_payload(__engaged
          ^                 ~~~~~~~~~
/opt/gcc/8.1.0/snos/lib/gcc/x86_64-suse-linux/8.1.0/../../../../include/g++/optional:734:4: note: in instantiation of member function 'std::_Optional_payload<MoveAware<int>, true, false, false>::_Optional_payload' requested here
        : _M_payload(__other._M_payload._M_engaged,
          ^
/opt/gcc/8.1.0/snos/lib/gcc/x86_64-suse-linux/8.1.0/../../../../include/g++/optional:980:11: note: in instantiation of member function 'std::_Optional_base<MoveAware<int>, false, false>::_Optional_base' requested here
    class optional
          ^
/global/cscratch1/sd/hargrove/upcnightly-knl/EX-cori_knl-aries-cray_old/runtime/work/dbg/upcxx/test/optional.cpp:262:40: note: in implicit move constructor for 'std::optional<MoveAware<int> >' first required here
  upcxx::optional<MoveAware<int>> ok = std::move(oi);
                                       ^
/opt/gcc/8.1.0/snos/lib/gcc/x86_64-suse-linux/8.1.0/../../../../include/g++/optional:574:7: note: explicitly defaulted function was implicitly deleted here
      _Optional_payload(const _Optional_payload&) = default;
      ^
/opt/gcc/8.1.0/snos/lib/gcc/x86_64-suse-linux/8.1.0/../../../../include/g++/optional:613:24: note: copy constructor of '_Optional_payload<MoveAware<int>, true, false, false>' is implicitly deleted because field '_M_payload' has a deleted copy constructor
          _Stored_type _M_payload;
                       ^
/global/cscratch1/sd/hargrove/upcnightly-knl/EX-cori_knl-aries-cray_old/runtime/work/dbg/upcxx/test/optional.cpp:221:3: note: 'MoveAware' has been explicitly marked deleted here
  MoveAware(MoveAware const&) = delete;
  ^
1 error generated.

Note this config is also defaulting to -std=c++2a, which means these errors are actually for our test/optional against std::optional provided by GCC 8.1.0. This is a combination that received very little pre-merge testing, so this problem might actually be much wider than CCE for configs passing -std=c++17 or newer (which activates use of std::optional).

Issue set to "Critical" priority pending triage

[Dan Bonachea]

Here is a GitLab pipeline run of test/optional for everything in our VMs while using -std=c++17 at app compile time.

The only failures appear to be due to the floor compilers which are using libstdc++ from GCC 6.4.0 which lacks the <optional> header entirely. So it's possible this defect is confined to CCE and the GCC 8 <optional> header.

[Dan Bonachea]

This is also failing in the same way using CCE 13.0.1 (still using -std=c++2a), so this problem is not confined to any one version of Cray C++.

[Amir Kamil]

I have not been able to figure out why the compiler is choking on this. This should be invoking the move ctor of _Optional_payload rather than the copy ctor. I didn’t find any issues with the library code – it looks correct to me. So my guess is a CCE bug.

That said, this test case is really intended to thoroughly test our (borrowed) implementation of optionals, not the vendor-provided one. I’ve already had to disable some pieces due to ICE’s on Intel. Should we just disable the test entirely in C++17+ mode?

[Dan Bonachea]

this test case is really intended to thoroughly test our (borrowed) implementation of optionals, not the vendor-provided one. I’ve already had to disable some pieces due to ICE’s on Intel. Should we just disable the test entirely in C++17+ mode?

I agree this sounds like a good approach - IOW if the vendor's own std::optional implementation is non-compliant and/or chokes their compiler, that's their problem not our problem. Please link this issue in a comment by the disable.

The only value I can see to keeping some coverage would be to helping us identify places where our test might rely on underspecified aspects of std::optional, but that would most likely reveal a defect in the test, not our optional implementation.

[Amir Kamil]

Proposed resolution in PR 450.

[Dan Bonachea]

Fixed in (pull request #450) merged at b98a386