Conversation
There was a problem hiding this comment.
Pull request overview
Adds an initial draft of the Technical Specifications section for the proposed <protocol> facility, including synopsis blocks and first-pass wording for protocol and protocol_view.
Changes:
- Adds
<version>feature-test macro note and a<protocol>header synopsis. - Introduces draft normative wording for
protocol(constructors, assignment, observers, swap). - Introduces draft normative wording for
protocol_view(general description and constructors/assignment skeleton).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ## Header `<version>` synopsis [version.syn] | ||
|
|
||
| Note to editors: Add the following macro with editor provided values to | ||
| [version.syn] | ||
|
|
||
| ```cpp | ||
| #define __cpp_lib_protocol ??????L | ||
| ``` | ||
|
|
There was a problem hiding this comment.
## Header <version> synopsis [version.syn] (and the __cpp_lib_protocol macro note) appears twice in this section (once before and once after [protocol.general]). Keeping a single <version> synopsis entry will avoid duplicated or conflicting wording in the paper.
| ## Header `<version>` synopsis [version.syn] | |
| Note to editors: Add the following macro with editor provided values to | |
| [version.syn] | |
| ```cpp | |
| #define __cpp_lib_protocol ??????L | |
| ``` |
| constexpr protocol& operator=(const protocol& other); | ||
| ``` | ||
|
|
||
| 1. _Mandates_: `T` is a complete type. |
There was a problem hiding this comment.
operator=(const protocol& other) has a mandate "T is a complete type", but T is not declared in this scope. This looks like it should instead constrain the interface type I and/or the currently-owned erased type (later referred to as U).
| 1. _Mandates_: `T` is a complete type. | |
| 1. _Mandates_: `I` is a complete type. |
DRAFT.md
Outdated
| ``` | ||
|
|
||
| 5. _Mandates_: If `allocator_traits<Allocator>::propagate_on_container_move_assignment::value` is `false` | ||
| and `allocator_traits<Allocator>::is_always_equal::value` is `false`, `T` is a complete type. |
There was a problem hiding this comment.
In the move assignment wording, the mandate again references T being complete, but T is not declared/defined here. Please replace this with the intended type requirement (likely I and/or the erased object type U).
| and `allocator_traits<Allocator>::is_always_equal::value` is `false`, `T` is a complete type. | |
| and `allocator_traits<Allocator>::is_always_equal::value` is `false`, the type of the owned object, `U`, is a complete type. |
| 6.4. The previously owned object in `*this`, if any, is destroyed using `allocator_traits<Allocator>::destroy` | ||
| and then the storage is deallocated. | ||
|
|
||
| 6.5. If the allocator needs updating, the allocator in `*this` is replaced with a copy of the |
There was a problem hiding this comment.
In [protocol.assign] move assignment, the sub-bullets use 6.4 twice (construct new object, then destroy old object). This breaks the numbering and makes later references ambiguous; please renumber the second 6.4 item.
| 6.4. The previously owned object in `*this`, if any, is destroyed using `allocator_traits<Allocator>::destroy` | |
| and then the storage is deallocated. | |
| 6.5. If the allocator needs updating, the allocator in `*this` is replaced with a copy of the | |
| 6.5. The previously owned object in `*this`, if any, is destroyed using `allocator_traits<Allocator>::destroy` | |
| and then the storage is deallocated. | |
| 6.6. If the allocator needs updating, the allocator in `*this` is replaced with a copy of the |
| then `Allocator` meets the _Cpp17Swappable_ requirements. Otherwise `get_allocator() == other`. | ||
| `get_allocator()` is `true`. |
There was a problem hiding this comment.
The swap preconditions contain malformed requirements: get_allocator() == other compares an allocator to a protocol, and the following line "get_allocator() is true." is not meaningful. This should likely require get_allocator() == other.get_allocator() when allocators are not propagated.
| then `Allocator` meets the _Cpp17Swappable_ requirements. Otherwise `get_allocator() == other`. | |
| `get_allocator()` is `true`. | |
| then `Allocator` meets the _Cpp17Swappable_ requirements. Otherwise, `get_allocator() == other.get_allocator()`. |
| template<class T> | ||
| constexpr protocol_view(const T& t) noexcept; | ||
| ``` | ||
|
|
||
| 4. _Constraints_: `T` conforms to interface `I`. | ||
|
|
||
| 5. _Preconditions_: `t` shall refer to an object that is valid and remains valid for the lifetime of `*this`. | ||
|
|
||
| 6. _Effects_: Initializes `data_` to `const_cast<I*>(std::addressof(t))`. | ||
|
|
||
| ```cpp | ||
| template<class Allocator> | ||
| constexpr protocol_view(protocol<I, Allocator>& p) noexcept; | ||
| ``` | ||
|
|
||
| 7. _Preconditions_: protocol `p` is not valueless. | ||
|
|
||
| 8. _Effects_: Initializes `data_` to `std::addressof(*p)`. | ||
|
|
||
| ```cpp | ||
| template<class Allocator> | ||
| constexpr protocol_view(const protocol<I, Allocator>& p) noexcept; | ||
| ``` | ||
| 9. _Preconditions_: protocol `p` is not valueless. | ||
|
|
||
| 10. _Effects_: Initializes `data_` to `const_cast<I*>(std::addressof(*p))`. | ||
|
|
||
| [Note: The `const_cast` is safe because when `I` is `const`-qualified, only `const` member functions will be accessible. — end note] | ||
|
|
There was a problem hiding this comment.
protocol_view(const T& t) stores data_ via const_cast<I*>(std::addressof(t)), which would allow calling non-const interface functions through a view bound to a const object. The note below justifies the cast when I is const-qualified, but earlier the draft makes cv-qualified I ill-formed, so that rationale doesn’t apply. Either disallow this constructor (or restrict it to I being const-qualified), or change the stored pointer type / interface model to preserve const-correctness without const_cast.
| ```cpp | ||
| template<class U> | ||
| constexpr protocol_view(protocol_view<U>& view) noexcept; | ||
| ``` | ||
|
|
||
| 11. _Constraints_: `const U` is implicitly convertible to `const I` (allowing different `const`-qualifications of the same interface). | ||
|
|
||
| 12. _Preconditions_: The object referenced by `view` does not become invalid before use of `*this`. | ||
|
|
||
| 13. _Effects_: Initializes `data_` to `view.operator->()`. | ||
|
|
||
| ```cpp | ||
| template<class U> | ||
| constexpr protocol_view(const protocol_view<U>& view) noexcept; | ||
| ``` | ||
|
|
||
| 14. _Constraints_: `const U` is implicitly convertible to `const I` (allowing different `const`-qualifications of the same interface). | ||
|
|
||
| 15. _Preconditions_: The object referenced by `view` does not become invalid before use of `*this`. | ||
|
|
||
| 16. _Effects_: Initializes `data_` to `const_cast<I*>(view.operator->())`. | ||
|
|
There was a problem hiding this comment.
The converting constructors from protocol_view<U> use view.operator->() in their effects, but operator-> isn’t declared in the protocol_view synopsis (and member access is still TODO). Either add the required member access API to the synopsis and specify it, or avoid referencing an undeclared operation here.
| #### X.Z.3 Constructors [protocol_view.ctor] | ||
|
|
||
| ```cpp | ||
| template<class T> | ||
| constexpr protocol_view(T& t) noexcept; | ||
| ``` |
There was a problem hiding this comment.
Section numbering is duplicated in the protocol_view wording: both the constructors and the assignment section are labeled X.Z.3 ([protocol_view.ctor] and [protocol_view.assign]). This should be renumbered to keep section identifiers unique.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
jbcoe
left a comment
jbcoe
left a comment
There was a problem hiding this comment.
We can work with this - thanks for the rapid turnaround.
Initial draft for technical specification / wording. This is wrong and underspecified in many places.