parameters library and rvalue pitfall
I just added an index_result metafunction that was requested during the review, for getting the result of a named parameter lookup, possibly with a default or a lazy default. Whereas previously, you would write the following to implement your function with named parameters, template<class Params> void f_impl(const Params& p) { g( p[name] , p[value || boost::bind(&value_default) ] , p[index | 999 ] ); } now you have the option to write something like this: template<class Params> void f_impl(const Params& p) { typename boost::parameter::index_result< Params, struct name_>::type& n = p[name]; typename boost::parameter::index_result< Params, struct value_, double >::type const& v = p[value || boost::bind(&value_default) ]; typename boost::parameter::index_result< Params, struct index_, int >::type const& i = p[index | 999]; g(n,v,i); } The problem, which you may have noticed, is that in the latter case you may be creating dangling references to destroyed rvalues. In the case of the index parameter above, the rvalue 999 gets bound to a const reference that then *passes through* the operator[] and initializes i. Since p[index | 999] returns a const reference and not a value, i is bound directly to the original rvalue 999 which is destroyed at the end of the full expression. The question is, what should we do about this? It turns out to be surprisingly hard to write the declarations of these temporary variables so they work in all cases. For example, v had to be a const& because in one case index_result<...>::type turned out to be char const[4] and when the default is used, it's double with p[value || ...] returning a double rvalue (by value). Thoughts? -- Dave Abrahams Boost Consulting www.boost-consulting.com
David Abrahams wrote: [...]
The question is, what should we do about this? It turns out to be surprisingly hard to write the declarations of these temporary variables so they work in all cases. For example, v had to be a const& because in one case index_result<...>::type turned out to be
char const[4]
and when the default is used, it's
double
with p[value || ...] returning a double rvalue (by value).
Thoughts?
Make the user add reference to the Default type instead of index_result<>::type? template<class Params> void f_impl(const Params& p) { typename boost::parameter::index_result< Params, struct name_>::type n = p[name]; typename boost::parameter::index_result< Params, struct value_, double >::type v = p[value || boost::bind(&value_default) ]; typename boost::parameter::index_result< Params, struct index_, int >::type i = p[index | 999]; g(n,v,i); } So index_result<Params, Keyword, Default>::type would be either Params[Keyword]::type& or Default. And maybe add another metafunction that returns Params[Keyword]::type without a reference. -- Daniel Wallin
Daniel Wallin <dalwan01@student.umu.se> writes:
David Abrahams wrote: [...]
The question is, what should we do about this? It turns out to be surprisingly hard to write the declarations of these temporary variables so they work in all cases. For example, v had to be a const& because in one case index_result<...>::type turned out to be
char const[4]
and when the default is used, it's
double
with p[value || ...] returning a double rvalue (by value).
Thoughts?
Make the user add reference to the Default type instead of index_result<>::type?
I don't fully understand what you're proposing or what it's supposed to address, but I think you might be missing the point. To make it safe to use, ideally index_result<...>::type whatever = p[x | some-expression]; would always cause a by-value return from p[...] when some-expression is an rvalue. By the same token, f( p[x | some-expression] ) should always return by-reference from p[...], for efficiency. This has a little less to do with how the user declares index_result<...>::type than with the result of indexing. -- Dave Abrahams Boost Consulting www.boost-consulting.com
David Abrahams wrote:
Daniel Wallin <dalwan01@student.umu.se> writes:
David Abrahams wrote: [...]
The question is, what should we do about this? It turns out to be surprisingly hard to write the declarations of these temporary variables so they work in all cases. For example, v had to be a const& because in one case index_result<...>::type turned out to be
char const[4]
and when the default is used, it's
double
with p[value || ...] returning a double rvalue (by value).
Thoughts?
Make the user add reference to the Default type instead of index_result<>::type?
I don't fully understand what you're proposing or what it's supposed to address, but I think you might be missing the point. To make it safe to use, ideally
index_result<...>::type whatever = p[x | some-expression];
would always cause a by-value return from p[...] when some-expression is an rvalue.
It's perfectly safe for p[...] to always return by reference, even if some-expression is an rvalue. The only problem is that it's hard for the user to determine if the result should be held by reference or not. I propose that the user tells index_result<> if the result of the default expression should be by reference, so that: index_result<.., int>::type whatever = p[x | 0]; whatever stores a copy of 0 in the default-case, and stores a reference to p[x] otherwise. int default_ = ...; index_result<.., int const&>::type whatever = p[x | default_]; See what I'm saying now? Am I missing the point? -- Daniel Wallin
Daniel Wallin <dalwan01@student.umu.se> writes:
David Abrahams wrote:
Daniel Wallin <dalwan01@student.umu.se> writes:
David Abrahams wrote: [...]
The question is, what should we do about this? It turns out to be surprisingly hard to write the declarations of these temporary variables so they work in all cases. For example, v had to be a const& because in one case index_result<...>::type turned out to be
char const[4]
and when the default is used, it's
double
with p[value || ...] returning a double rvalue (by value).
Thoughts?
Make the user add reference to the Default type instead of index_result<>::type?
I don't fully understand what you're proposing or what it's supposed to address, but I think you might be missing the point. To make it safe to use, ideally
index_result<...>::type whatever = p[x | some-expression];
would always cause a by-value return from p[...] when some-expression is an rvalue.
It's perfectly safe for p[...] to always return by reference, even if some-expression is an rvalue. The only problem is that it's hard for the user to determine if the result should be held by reference or not.
OK, that's one way to look at it: it's perfectly safe for the library to do X, as long as the user is really careful to do Y when necessary. I was looking at this from the user's eyes: she'd like the library to do something that's always safe and never require her to "determine if the result should be held by reference or not."
I propose that the user tells index_result<> if the result of the default expression should be by reference, so that:
index_result<.., int>::type whatever = p[x | 0];
whatever stores a copy of 0 in the default-case, and stores a reference to p[x] otherwise.
int default_ = ...; index_result<.., int const&>::type whatever = p[x | default_];
See what I'm saying now? Am I missing the point?
That's not too bad. I think we might be able to make it a bit safer by detecting when the default is a non-const rvalue and having p[...] return by value in that case. However, that might come at the cost of some efficiency if the user also happens to use a non-reference variable: index_result<.., heavyweight_type>::type = p[x | rvalue_expr() ] Of course, when it's heavy the user will probably be using || for lazy defaults, in which case, can we protect her? -- Dave Abrahams Boost Consulting www.boost-consulting.com
David Abrahams wrote:
Daniel Wallin <dalwan01@student.umu.se> writes:
David Abrahams wrote: I propose that the user tells index_result<> if the result of the default expression should be by reference, so that:
index_result<.., int>::type whatever = p[x | 0];
whatever stores a copy of 0 in the default-case, and stores a reference to p[x] otherwise.
int default_ = ...; index_result<.., int const&>::type whatever = p[x | default_];
See what I'm saying now? Am I missing the point?
That's not too bad.
I think we might be able to make it a bit safer by detecting when the default is a non-const rvalue and having p[...]
Can we really detect that?
return by value in that case. However, that might come at the cost of some efficiency if the user also happens to use a non-reference variable:
index_result<.., heavyweight_type>::type = p[x | rvalue_expr() ]
Of course, when it's heavy the user will probably be using || for lazy defaults, in which case, can we protect her?
There's only so much we can do. This case seems hard to get wrong for the user. -- Daniel Wallin
Daniel Wallin <dalwan01@student.umu.se> writes:
David Abrahams wrote:
Daniel Wallin <dalwan01@student.umu.se> writes:
David Abrahams wrote: I propose that the user tells index_result<> if the result of the default expression should be by reference, so that:
index_result<.., int>::type whatever = p[x | 0];
whatever stores a copy of 0 in the default-case, and stores a reference to p[x] otherwise.
int default_ = ...; index_result<.., int const&>::type whatever = p[x | default_];
See what I'm saying now? Am I missing the point?
That's not too bad.
Done now. I hate the name index_result, though. I was thinking maybe parameter::binding<p, key, default>::type ??
I think we might be able to make it a bit safer by detecting when the default is a non-const rvalue and having p[...]
Can we really detect that?
I'm pretty sure that for all practical purposes, we can. struct fu { template <class T> int operator|(T& x) const; template <class T> char* operator|(T) const volatile; }; int a = 1; int x = fu() | a; char* y = fu() | 999; Clearly you can also use a free function with enable_if to avoid the volatile interaction if you really care.
return by value in that case. However, that might come at the cost of some efficiency if the user also happens to use a non-reference variable:
index_result<.., heavyweight_type>::type = p[x | rvalue_expr() ]
Of course, when it's heavy the user will probably be using || for lazy defaults, in which case, can we protect her?
There's only so much we can do. This case seems hard to get wrong for the user.
Well I guess ultimately you want a macro (ick, in this case) with decltype: binding<p, k, decltype(some_expression)>::type x = p[x | some_expression]; right? -- Dave Abrahams Boost Consulting www.boost-consulting.com
David Abrahams wrote:
Daniel Wallin <dalwan01@student.umu.se> writes:
David Abrahams wrote:
Daniel Wallin <dalwan01@student.umu.se> writes:
Done now. I hate the name index_result, though. I was thinking maybe
parameter::binding<p, key, default>::type
??
Sure. Maybe we should have two, so people who want to do metaprogramming doesn't have to remove_reference<>? binding<p, key, default>::type -> T | default binding_result<p, key, default>::type -> T& | default ?
I think we might be able to make it a bit safer by detecting when the default is a non-const rvalue and having p[...]
Can we really detect that?
I'm pretty sure that for all practical purposes, we can.
struct fu { template <class T> int operator|(T& x) const;
template <class T> char* operator|(T) const volatile; };
int a = 1; int x = fu() | a; char* y = fu() | 999;
Clearly you can also use a free function with enable_if to avoid the volatile interaction if you really care.
OK, cool.
There's only so much we can do. This case seems hard to get wrong for the user.
Well I guess ultimately you want a macro (ick, in this case) with decltype:
binding<p, k, decltype(some_expression)>::type x = p[x | some_expression];
right?
Ultimately I would want to write: auto y = p[x | some_expression]; and have p[...] detect rvalues and return by value when appropriate. ;) -- Daniel Wallin
Daniel Wallin <dalwan01@student.umu.se> writes:
David Abrahams wrote:
Daniel Wallin <dalwan01@student.umu.se> writes:
David Abrahams wrote:
Daniel Wallin <dalwan01@student.umu.se> writes:
Done now. I hate the name index_result, though. I was thinking maybe
parameter::binding<p, key, default>::type
??
Sure. Maybe we should have two, so people who want to do metaprogramming doesn't have to remove_reference<>?
binding<p, key, default>::type -> T | default binding_result<p, key, default>::type -> T& | default
?
I don't understand why you'd want the one without the reference type... and -- I'm sure it's not what you want me to focus on -- but I don't like the name "binding_result" one bit. I'm inclined to vote "no" on this one. It isn't a flexibility that's known to be needed.
I think we might be able to make it a bit safer by detecting when the default is a non-const rvalue and having p[...]
Can we really detect that?
I'm pretty sure that for all practical purposes, we can.
struct fu { template <class T> int operator|(T& x) const;
template <class T> char* operator|(T) const volatile; };
int a = 1; int x = fu() | a; char* y = fu() | 999;
Clearly you can also use a free function with enable_if to avoid the volatile interaction if you really care.
OK, cool.
There's only so much we can do. This case seems hard to get wrong for the user.
Well I guess ultimately you want a macro (ick, in this case) with decltype:
binding<p, k, decltype(some_expression)>::type x = p[x | some_expression];
right?
Ultimately I would want to write:
auto y = p[x | some_expression];
and have p[...] detect rvalues and return by value when appropriate. ;)
Well, _ultimately_ p[...] can return an rvalue reference to some_expression and avoid a copy ;-) Anyway, I'm inclined that we not try to do this by-value return thing. Your thoughts? Also: 1. for the lazy case we should probably use result_of<F()>::type on the compilers that support it? 2. We probably also need lazy_binding<p, k, f>::type, right? -- Dave Abrahams Boost Consulting www.boost-consulting.com
David Abrahams <dave@boost-consulting.com> writes:
Daniel Wallin <dalwan01@student.umu.se> writes:
Sure. Maybe we should have two, so people who want to do metaprogramming doesn't have to remove_reference<>?
binding<p, key, default>::type -> T | default binding_result<p, key, default>::type -> T& | default
?
I don't understand why you'd want the one without the reference type... and -- I'm sure it's not what you want me to focus on -- but I don't like the name "binding_result" one bit.
I'm inclined to vote "no" on this one. It isn't a flexibility that's known to be needed.
So far I've just renamed index_result to binding.
Anyway, I'm inclined that we not try to do this by-value return thing. Your thoughts?
Still would like to hear them.
Also:
1. for the lazy case we should probably use result_of<F()>::type on the compilers that support it?
2. We probably also need lazy_binding<p, k, f>::type, right?
Now done. -- Dave Abrahams Boost Consulting www.boost-consulting.com
David Abrahams wrote:
David Abrahams <dave@boost-consulting.com> writes:
Anyway, I'm inclined that we not try to do this by-value return thing. Your thoughts?
Still would like to hear them.
I agree. I think the current system is good enough.
Also:
1. for the lazy case we should probably use result_of<F()>::type on the compilers that support it?
Yeah.
2. We probably also need lazy_binding<p, k, f>::type, right?
Now done.
Great! -- Daniel Wallin
participants (2)
-
Daniel Wallin -
David Abrahams