-
Notifications
You must be signed in to change notification settings - Fork 68
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ranges::to #307
base: master
Are you sure you want to change the base?
ranges::to #307
Conversation
... a zero-instantiation version of `std::conditional_t`.
include/stl2/detail/to.hpp
Outdated
_Cont __c(static_cast<_Args&&>(__args)...); | ||
auto const __size = size(__r); | ||
using __capacity = decltype(__c.capacity()); | ||
// if (__can_represent<__capacity>(__size)) ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incomplete thought?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is a potential means of dealing with "What happens if the container's capacity type cannot represent the range's size?" (which is what the comment should really say).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is now tagged FIXME
, and needs to be fixed before merging.
include/stl2/detail/to.hpp
Outdated
|
||
template<class _IRange, class _Cont> | ||
requires __class_convertible<_Cont, _IRange> | ||
constexpr auto operator|(_IRange&& __r, __class_closure<_Cont>(&)() noexcept) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think to<std::vector<int>>
has the right associated namespaces in order to find this overload in all cases. In range-v3, I needed to have a defaulted function argument (of non-template class type) on to
and put the operator|
within that type's associated namespace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the reminder, and more importantly the twitter link. I'd intended to dig the thread out to refresh my memory on which parts of the problem are in the spec and which are compiler bugs. (Notably the workaround doesn't work with MSVC.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh. I'm certain I got it to work with MSVC on Godbolt. 🤔
include/stl2/detail/to.hpp
Outdated
|
||
template<template<class...> class _ContT, class _IRange> | ||
requires __template_convertible<_ContT, _IRange> | ||
constexpr auto operator|(_IRange&& __r, __template_closure<_ContT>(&)() noexcept) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same feedback here. I have it on good authority[*] that this doesn't work in general.
[*]: https://twitter.com/ericniebler/status/1085600729656549377
Per P1206 and Kona LEWG discussion.