C++11 implementation of bslstl_optional

bloomberg / bde

Basic Development Environment - a set of foundational C++ libraries used at Bloomberg.

Apache License 2.0

1.67k stars 316 forks source link

C++11 implementation of bslstl_optional #261

Closed NinaRanns closed 2 years ago

NinaRanns commented 4 years ago

P2047 'An allocator-aware optional type'

C++11 Implementation of P2047 'An allocator-aware optional type'. The implementation differs from the paper in a couple of ways (bsl::allocator is used instead of pmr::polymorphic_allocator, value_or is disabled until the decision is made on how to handle compilers in which copy elision isn't guaranted).

New tests specific to bsl::optional have been implemented.

C++03 support is not implemented yet.

phalpern commented 4 years ago

I thoroughly reviewed this PR before it was submitted. It is good for merging.

RMGiroux commented 4 years ago

Copied this as Bloomberg internal PR 2755

mversche commented 4 years ago

I thoroughly reviewed this PR before it was submitted. It is good for merging.

Merging where? master??

I think there is a disconnect between what we think is "ready for merge". "Ready for merge" would suggest it has been built and tested our production platforms. A cursory scan of the changes suggests a few things to do before merge:

Verify support for non gcc compilers (clang, MSVC 2015,17,19) then C++03 Sun OS, AIX XLC
Running bde_verify and cleaning up the obvious stuff
TODO lists

phalpern commented 4 years ago

This has been tested on several other C++11 compilers at Bloomberg (Linux gcc and clang, MacOS Clang, and MSVC 2015). Merging is, of course at Bloomberg's discretion. Work is ongoing to make this component completely BDE compliant, including support for the C++03 compilers. It probably makes sense, therefore, to delay merging until that work is complete.

mversche commented 2 years ago

Closing. bsl::optional, a version of this change-set, was merged to our master branch a year ago or so.