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<foo&, float, char, int> void(foo::*)(float, char, int) & -> tuple<foo&, float, char, int> 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<foo, float, char, int> and void(float, ...) -> tuple<float, ct::varargs_t> which would be fully reversible but could possibly be less convenient in practice. 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. 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.) 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&. 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. All in all, a solid library that solves a real problem, even though the problem remains somewhat of a mystery for the uninitiated. :-)
On Mon, Apr 10, 2017 at 2:48 PM, Peter Dimov via Boost <boost@lists.boost.org> wrote:
The obvious theoretically consistent approach here is
void(foo::*)(float, char, int) -> tuple<foo, float, char, int>
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 <boost@lists.boost.org> wrote:
The obvious theoretically consistent approach here is
void(foo::*)(float, char, int) -> tuple<foo, float, char, int>
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<args_t<PMF>, return_type_t<PMF>> > and here we do want foo&. We'd prefer a reversible tuple if we wanted to manipulate the argument list (assuming that the built-in facilities are removed) and then go from the tuple back to a member pointer. Although in this case it would probably be more convenient if we just drop the class and keep the original member pointer to retain it (along with the return type and noexcept which can't go in the tuple anyway.)
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<foo&, float, char, int> void(foo::*)(float, char, int) & -> tuple<foo&, float, char, int>
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<foo, float, char, int>
and
void(float, ...) -> tuple<float, ct::varargs_t>
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 <boost@lists.boost.org> wrote:
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<foo&, float, char, int> void(foo::*)(float, char, int) & -> tuple<foo&, float, char, int>
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<foo, float, char, int>
and
void(float, ...) -> tuple<float, ct::varargs_t>
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<args_t<T>>>, but maybe that's not ideal?
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 <type_traits>. Would this also remove noexcept/transaction_safe? I'm conflicted about copy_member_cv_ref, because I'm not sure whether it would be more useful to copy them to a function type or value type.
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