-
Notifications
You must be signed in to change notification settings - Fork 221
Add support for reference_wrappers in make_xxx #179
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
base: develop
Are you sure you want to change the base?
Conversation
faceac0 to
01f0afc
Compare
|
Actually, For For |
01f0afc to
73d71ba
Compare
|
This PR is essentially feature complete, but I'm not sure about it. Actually, I'm not sure at all about the whole "storing references in containers" thing. Indeed, consider the following: #include <boost/hana/tuple.hpp>
#include <boost/hana/tail.hpp>
namespace hana = boost::hana;
int main() {
int i = 0, j = 1, k = 2;
hana::tuple<int&, int&, int&> xs{i, j, k};
hana::tuple<int, int> ys = hana::tail(xs);
// ^^^ ^^^ notice the decay
}The problem is that
Also note that this problem extends to basically any algorithm that returns a new sequence created with Hence, documenting that containers can hold references and making this easier by supporting the |
9d4e74d to
9c6b42c
Compare
81fe957 to
a7c50c5
Compare
99ff528 to
cf327f5
Compare
|
What if hana::type contained an additional type alias for the exact original template argument, without reference decay? Would this remove the need for reference wrapping? Would it be difficult to preserve this across algorithms? |
|
|
Ok, I just realized the "rationale for stripping references" section of the documentation is about decltype_ specifically. My mistake. |
|
Yes, it's about |
cf327f5 to
ff0fb7f
Compare
da2a3df to
37885cf
Compare
37885cf to
4031ba9
Compare
4031ba9 to
5e45f3d
Compare
|
Giving an explicit example of a use case where this issue comes up, for the purpose of a discussion currently going on at the Issaquah meeting. Consider the following: #include <tuple>
template <typename Tuple, typename U, std::size_t ...i>
auto push_back_impl(Tuple&& tuple, U&& u, std::index_sequence<i...>) {
// if we use deduction guides with unwrapping of reference_wrappers, this
// may end up unwrapping reference wrappers under our feet, in this totally
// generic context
return std::tuple{std::get<i>(std::forward<Tuple>(tuple))..., std::forward<U>(u)};
}
template <typename ...T, typename U>
auto push_back(std::tuple<T...>&& tuple, U&& u) {
return push_back_impl(std::move(tuple), std::forward<U>(u),
std::make_index_sequence<sizeof...(T)>{});
}
template <typename ...T, typename U>
auto push_back(std::tuple<T...> const& tuple, U&& u) {
return push_back_impl(tuple, std::forward<U>(u),
std::make_index_sequence<sizeof...(T)>{});
}
int main() {
int i = 1, j = 2, k = 3;
// no deduction guides used, we want a ref_wrapper here.
std::tuple<int, std::reference_wrapper<int>, int> tuple{i, std::ref(j), k};
auto more = push_back(tuple, 4); // oops, just made a copy of j!
// dangling reference to the temporary copy made inside push_back
int& dangling = std::get<1>(push_back(tuple, 4));
} |
This pull request attempts to gradually add support for
reference_wrapperin themake_xxxfamily of functions. This is in order to fix #176.As documented in #176, I decided to go with the "forward declare in the
std::namespace" approach, because I think this will be far better for users (let's not introduce yet anotherreference_wrapperclass). The following containers should supportreference_wrapper:tuplesetmappairoptionallazyWe also need to:
reference_wrappercan be used withmake_xxxto create a container of references.