This is an archive of the discontinued LLVM Phabricator instance.

[libcxx] add missing constexpr to optional::value_or
ClosedPublic

Authored by cpplearner on Dec 16 2016, 7:30 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

cpplearner updated this revision to Diff 81754.Dec 16 2016, 7:30 AM
cpplearner retitled this revision from to [libcxx] add missing constexpr to optional::value_or.
cpplearner updated this object.
cpplearner added a subscriber: cfe-commits.
EricWF requested changes to this revision.Dec 23 2016, 10:42 AM
EricWF edited edge metadata.

Tests please! They should be somewhere under test/std/utilities/optional/optional.object/optional.object.observe

This revision now requires changes to proceed.Dec 23 2016, 10:42 AM
cpplearner edited edge metadata.

test updated

cpplearner edited edge metadata.
EricWF accepted this revision.Dec 27 2016, 11:42 AM
EricWF edited edge metadata.

The changes to <optional> LGTM but not the ones to <experimental/optional>

Unfortunately libc++'s <experimental/optional> doesn't yet implement LFTS v2 so all of the other non-const observers are also not constexpr in our implementation. I think updating <experimental/optional> to LFTS v2 should be done as a separate patch. Please back out the changes to <experimental/optional> before committing.

test/std/experimental/optional/optional.object/optional.object.observe/value_or.pass.cpp
48 ↗(On Diff #82551)

This test doesn't compile since the non-const version of std::experimental::optional::operator*() isn't constexpr.

This revision is now accepted and ready to land.Dec 27 2016, 11:42 AM
cpplearner edited edge metadata.

backed out the changes to <experimental/optional>

@cpplearner Do you need somebody to commit this for you?

Yes, I do. Thank you.

This revision was automatically updated to reflect the committed changes.