Enable protocol_view to be constructed from protocol without extra indirection#56
Enable protocol_view to be constructed from protocol without extra indirection#56
Conversation
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| template <typename T> | ||
| inline constexpr protocol_view_dcb_{{ c.name }}<T> protocol_view_dcb_{{ c.name }}_inst{}; | ||
|
|
There was a problem hiding this comment.
protocol_view_dcb_... is a polymorphic type (has virtual functions), so it is not a literal type. Declaring inline constexpr protocol_view_dcb_..._inst{} is ill-formed because constexpr variables require a literal type. Consider making this an inline const object, or (preferably) provide a static const ...& instance() that returns a function-local static to avoid global dynamic initialization costs/order issues.
| template <typename T> | ||
| inline constexpr protocol_view_const_dcb_{{ c.name }}<T> protocol_view_const_dcb_{{ c.name }}_inst{}; |
There was a problem hiding this comment.
Same issue as above: protocol_view_const_dcb_... is polymorphic, so inline constexpr protocol_view_const_dcb_..._inst{} is ill-formed (non-literal type). Use a non-constexpr storage strategy (e.g., inline const or a function-local static accessor).
| requires protocol_concept_{{ c.name }}<T> && (!std::same_as<std::remove_cvref_t<T>, protocol_view>) | ||
| constexpr protocol_view(T& obj) noexcept | ||
| requires protocol_const_concept_{{ c.name }}<T> && | ||
| (!std::same_as<std::remove_cvref_t<T>, protocol_view>) |
There was a problem hiding this comment.
The templated ctor in protocol_view<const ...> only excludes its own specialization (protocol_view<const ...>), so it still accepts protocol_view<...> as T. That allows creating a “view of a view” (extra indirection) and can also dangle in common patterns like protocol_view<const I> v = protocol_view<I>(obj); (binds to a temporary view instead of using the shallow conversion operator). Consider tightening the constraint to also exclude protocol_view<{{ full_class_name }}> (and ideally both view specializations) so conversions between views go through the dedicated shallow path.
| (!std::same_as<std::remove_cvref_t<T>, protocol_view>) | |
| (!std::same_as<std::remove_cvref_t<T>, protocol_view>) && | |
| (!std::same_as<std::remove_cvref_t<T>, | |
| protocol_view<{{ full_class_name }}}>) |
| requires protocol_concept_{{ c.name }}<T> && (!std::same_as<std::remove_cvref_t<T>, protocol_view>) | ||
| constexpr protocol_view(T& obj) noexcept | ||
| requires protocol_const_concept_{{ c.name }}<T> && | ||
| (!std::same_as<std::remove_cvref_t<T>, protocol_view>) |
There was a problem hiding this comment.
Same “view of a view” issue here: the protocol_view<const ...> templated ctor only excludes its own specialization, so protocol_view<...> can match and produce a const view that points at the view object (extra indirection, and potential dangling if the source view is a temporary). Tighten the requires to exclude the other protocol_view specialization(s) so the shallow conversion operator is the only viable path.
| (!std::same_as<std::remove_cvref_t<T>, protocol_view>) | |
| (!std::same_as<std::remove_cvref_t<T>, protocol_view>) && | |
| (!std::same_as<std::remove_cvref_t<T>, | |
| protocol_view<{{ full_class_name }}>>) |
No description provided.