Update optional<T&> paper with new Wording based on Beman Optional26 #53
Conversation
Header is in sync with synopsis.
Make class declaration the synopsis version.
Move the assignment operators out of class synopsis declaration.
Define swap out of body
Definitions of begin and end moved out of line.
Move definitions out of the class declaration.
and_then etc out of line
Move reset out of the body.
Move the free functions out of the namespace. Make sure no declarations are unintended from a slightly different definition.
optional_map_impl and monostate are dead.
Standardese lives in std and avoids naming the namespace, except for move and forward.
No extra definitions snuck in, all back now inside the namespace so are not noise.
Make change tracking back to std a little easier.
Apply all the changes to optional to the wording section of P2988.
Replace by expos or implementation defined.
optional<T&&>
The type of the u parameter must be convertible to a T, and T must be copy constructible.
A few additional std:: names. Also remove the anon namespace. Doesn't protect much as the std:: names are still available if someone uses namespace optional26.
Have to return std::move(i) to return an int&& in GCC 11 and 12.
Make style consistent with existing optional wording.
Fix issues
|
clang-format added by mistake? |
| } | ||
|
|
||
| TEST(OptionalRefRefTest, Triviality) { | ||
| EXPECT_TRUE(std::is_trivially_copy_constructible<beman::optional26::optional<int&&>>::value); |
There was a problem hiding this comment.
Maybe use an alias using Type = beman::optional26::optional<int&&>; and then EXPECT_TRUE(std::is_trivially_copy_constructible<Type>::value);
Btw, we can put a follow-up commit/PR to remove duplication across *.t.cpp. We can create a new issue. for that.
| = default; | ||
|
|
||
| constexpr optional(nullopt_t) noexcept {} | ||
| using iterator = detail::contiguous_iterator<T, optional>; // see~\ref{optional.iterators} |
There was a problem hiding this comment.
| using iterator = detail::contiguous_iterator<T, optional>; // see~\ref{optional.iterators} | |
| using iterator = detail::contiguous_iterator<T, optional>; // ~\ref{optional.iterators} |
?
| requires is_copy_constructible_v<T> && (!is_trivially_copy_constructible_v<T>); | ||
| constexpr optional(const optional&) | ||
| requires std::is_copy_constructible_v<T> && std::is_trivially_copy_constructible_v<T> | ||
| requires is_copy_constructible_v<T> && is_trivially_copy_constructible_v<T> |
There was a problem hiding this comment.
why do we prefer this convention ? using multiple using std::something at the top of the file
| @@ -950,209 +1106,466 @@ auto optional_map_impl(Opt&& opt, F&& f) | |||
| template <class T> | |||
| class optional<T&> { | |||
There was a problem hiding this comment.
What do you think to split this file? Maybe:
- move
class optional<T>intodetails/optional_impl.hpp - move
class optional<T&>intodetails/optional_ref_impl.hpp - include both in
optional.hpp
If we agree with such approach, I'm OK to defer the work into a new issue.
| template <class U> | ||
| requires(!detail::is_optional<decay_t<U>>) | ||
| constexpr optional<T&>::optional(U&& u) noexcept : value_(addressof(u)) { | ||
| static_assert(is_constructible_v<add_lvalue_reference_t<T>, U>, "Must be able to bind U to T&"); |
There was a problem hiding this comment.
Why are add_lvalue_reference_t and add_rvalue_reference_t used given T& must be well-formed in these partial specializations?
I'll double check, but I think this one is probably dealing with formatting for papers and the standard, which has some tradeoffs made decades ago, and has to deal with narrow margins. |
neatudarius
left a comment
neatudarius
left a comment
There was a problem hiding this comment.
LGTM 👍
I'm going in PTO for a week. If not merged until I come back, I will do one more round of review. But feel free to proceed from my side.
|
Superseded by #70 |
3 participants
Update the paper for optional<T&> based on the updates to Optional26. Reorganize optional26 so that the header and class synopsis in the standard can be quoted mostly unmodified from the tested code. This involves moving the definitions of functions out of line from the class declaration and pulling std:: functions into scope via using declarations, as well as making sure non standard proposed helpers are in a detail namespace. In the paper itself, the synopsis and definitions are slightly modified to indicate exposition only members and types, changing some explicit and throw specifications to see-below, and moving the expression to the function definition, and, since is quite old at this point, rendering requires clauses in declarations as constraint clauses in text.
Also add two new "top level" papers that only produce the before and after changes for the wording, in order to simplify cross checking differences for review, and to later produce a better change request for the draft standard, in addition to the edit instructions in the paper.
This also adds a partial specialization for optional<T&&> which is nearly identical to optional<T&> and would be even more so if all optional specializations used optional::value_type throughout its definition where that is the actual intent.