Formale Review: Callable Traits
I think that the Callable Traits library should be ACCEPTED.
The design of the library is sound, although I'm not fond of the ad-hoc
metaprogramming part in "parameter list features". This is no fault of the
author though, as without a modern type-metaprogramming library in Boost,
everyone has to solve this problem in some way. Still, I think that most of
these should be removed.
Getting the arguments as a tuple that includes the class type as a first
parameter raises the obvious question of
void(foo::*)(float, char, int) -> tuple
On Mon, Apr 10, 2017 at 2:48 PM, Peter Dimov via Boost
The obvious theoretically consistent approach here is
void(foo::*)(float, char, int) -> tuple
It depends on what the theory is. If the model is "type of the implicit object parameter during overload resolution", then foo& is the correct choice (see [over.match.funcs]/4). If the model is "type of the class, decorated to reflect the cv-qualifier-seq and ref-qualifier, if any", then foo would be the natural result. The first model seems more useful to me, even if lossy.
Tim Song wrote:
On Mon, Apr 10, 2017 at 2:48 PM, Peter Dimov via Boost
wrote: The obvious theoretically consistent approach here is
void(foo::*)(float, char, int) -> tuple
It depends on what the theory is.
What I wanted (and failed) to express by "theoretically consistent" wasn't "consistent with a theory" but "consistent, in theory". That is, consistent, but not necessarily a better choice in practice.
If the model is "type of the implicit object parameter during overload resolution", then foo& is the correct choice (see [over.match.funcs]/4).
That would be useful if you want to perform overload resolution on the
tuple, which you probably don't.
I acknowledged that foo& is more useful. One example is when given a member
pointer pmf you want for some reason to derive its corresponding
std::function. That would be
function< apply_return_t
On 11/04/2017 06:48, Peter Dimov via Boost wrote:
Getting the arguments as a tuple that includes the class type as a first parameter raises the obvious question of
void(foo::*)(float, char, int) -> tuple
void(foo::*)(float, char, int) & -> tuple I understand why that is, but it still makes args lossy and the original can't be recreated from the tuple. Trailing varargs also have the same problem, as (float) and (float, ...) map to the same tuple.
The obvious theoretically consistent approach here is
void(foo::*)(float, char, int) -> tuple
and
void(float, ...) -> tuple
which would be fully reversible but could possibly be less convenient in practice.
Perhaps in the first case it should specify a pointer type instead? That seems reasonably natural to me as the actual type of the "this" parameter, though I don't have any standardese in particular to back that up.
On Mon, Apr 10, 2017 at 1:48 PM, Peter Dimov via Boost
I think that the Callable Traits library should be ACCEPTED.
The design of the library is sound, although I'm not fond of the ad-hoc metaprogramming part in "parameter list features". This is no fault of the author though, as without a modern type-metaprogramming library in Boost, everyone has to solve this problem in some way. Still, I think that most of these should be removed.
Getting the arguments as a tuple that includes the class type as a first parameter raises the obvious question of
void(foo::*)(float, char, int) -> tuple
void(foo::*)(float, char, int) & -> tuple I understand why that is, but it still makes args lossy and the original can't be recreated from the tuple. Trailing varargs also have the same problem, as (float) and (float, ...) map to the same tuple.
The obvious theoretically consistent approach here is
void(foo::*)(float, char, int) -> tuple
and
void(float, ...) -> tuple
which would be fully reversible but could possibly be less convenient in practice.
I decided to make these "lossy" because I think doing otherwise would make them less convenient to use in common forwarding scenarios.
I've a feeling that it might also be useful to provide an extractor that, for member pointers, returns just the arguments without the class.
I had this implemented at one point, but I thought it polluted the
interface. There is pop_args_front_t
In "member qualifier features", I'm missing remove_member_cv_ref (obvious semantics) and copy_member_cv_ref (copies cv+ref from one member pointer to another.)
I'm slightly hesitant to add remove_member_cv_ref (remove_member_qualifiers?),
because it doesn't quite mirror
In "member pointer features" we have the same problem in qualified_parent_class_of as in args,
void(foo::*)() -> foo& void(foo::*)() & -> foo&
It makes more sense for the first to yield 'foo', although this again may make the trait less convenient in practice.
It would also _probably_ make sense to provide apply_qualified_member_pointer, which, given void(float) and foo const&, would yield void (foo::*)(float) const&.
Hmm, I will consider adding this.
I did not look at the implementation.
I ran the test suite on Windows under Cygwin g++ 5 in 11/14/1z, Cygwin clang 3.9 11/14/1z, msvc-14.0, msvc-14.1. The tests passed. Took a look at the tests themselves, the coverage looks excellent.
The reference documentation is very good, although as already noted, it uses the terms of art "implementation defined" and "undefined behavior" incorrectly, this must be fixed.
The non-reference portion is a bit thin. It would be improved considerably by presenting a few real-world use cases.
Agreed, I will add some better examples.
All in all, a solid library that solves a real problem, even though the problem remains somewhat of a mystery for the uninitiated. :-)
Thanks a lot for taking the time to review this.
participants (4)
-
Barrett Adair
-
Gavin Lambert
-
Peter Dimov
-
Tim Song