This is an archive of the discontinued LLVM Phabricator instance.

[libcxx] optional: Implement LWG 2900 and P0602
ClosedPublic

Authored by CaseyCarter on Apr 21 2017, 4:19 PM.

Details

Summary
  • Implement LWG 2900 "The copy and move constructors of optional are not constexpr" by injecting base classes that differentiate between trivial/non-trivial implementation of each copy/move constructor/assignment.
  • Implement P0602 "variant and optional should propagate copy/move triviality" with slightly modified / more correct constraints that I have "upstreamed" to the Zhihao for inclusion in the next revision of the paper.

Diff Detail

Repository
rL LLVM

Event Timeline

CaseyCarter created this revision.Apr 21 2017, 4:19 PM

Assigning an empty optional to a non-empty optional needs to destroy the contained value, so we need to also require is_trivially_destructible to implement trivial assignments.

CaseyCarter retitled this revision from [libcxx] Implement LWG 2900 "The copy and move constructors of optional are not constexpr" to [libcxx] optional: Implement LWG 2900 and P0602.
CaseyCarter edited the summary of this revision. (Show Details)
  • Define new macro _MSVC_STL_VER in msvc_stdlib_force_include.hpp to indicate that the VC++ standard library is being tested.
  • Implement test coverage for P0602 "variant and optional should propagate copy/move triviality".
CaseyCarter updated this revision to Diff 97203.EditedApr 29 2017, 8:46 PM

Fix comment typo in optional.object.ctor/move.pass.cpp

Update libc++-specific special member test for extended P0602 implementation.

CaseyCarter updated this revision to Diff 97366.May 1 2017, 4:46 PM

Fix a missing indent in the msvc_stdlib_force_include.hpp change.

CaseyCarter updated this revision to Diff 97769.May 3 2017, 9:23 PM

Polish: clarify static_assert messages, DeMorgan a clause so it more precisely reflects the static_assert message.

EricWF edited edge metadata.May 4 2017, 1:28 AM

So Itanium ABI has this quirk where trivial types are passed using different conventions than non-trivial types. This means changing the triviality of std::optional for any instantiation is potentially ABI breaking. I'll need to do more investigation to find out how this will affect libc++, and if we can take this change.

So Itanium ABI has this quirk where trivial types are passed using different conventions than non-trivial types. This means changing the triviality of std::optional for any instantiation is potentially ABI breaking. I'll need to do more investigation to find out how this will affect libc++, and if we can take this change.

libc++ committed to ABI stability for optional, a new feature of C++17, before the standard shipped?

EricWF added a comment.May 4 2017, 9:26 AM

libc++ committed to ABI stability for optional, a new feature of C++17, before the standard shipped?

Libc++ doesn't commit to anything, the vendors such as Apple and FreeBSD do. I don't think either of them
have shipped a release with std::optional yet, but I need to be sure they don't have ABI obligations before I break it.

I certainly agree that nobody should be expecting ABI stability for features not yet standardized, and I should
take steps to ensure vendors understand that.

Fix merge conflicts.

CaseyCarter edited the summary of this revision. (Show Details)

Incorporate the libcxx test tree special_member_gen test into the std test, making my per-SMF triviality tests unnecessary.

EricWF accepted this revision.Jul 7 2017, 10:28 PM

LGTM. I'll deal with ensuring all vendors are willing to take this ABI break (or that they avoid it).

This revision is now accepted and ready to land.Jul 7 2017, 10:28 PM
This revision was automatically updated to reflect the committed changes.
libcxx/trunk/include/optional