Formale Review: Callable Traits

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Formale Review: Callable Traits

Boost - Dev mailing list
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. :-)


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Formale Review: Callable Traits

Boost - Dev mailing list
On Mon, Apr 10, 2017 at 2:48 PM, Peter Dimov via Boost
<[hidden email]> 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.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Formale Review: Callable Traits

Boost - Dev mailing list
Tim Song wrote:
> On Mon, Apr 10, 2017 at 2:48 PM, Peter Dimov via Boost
> <[hidden email]> 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.)


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Formale Review: Callable Traits

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
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.



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Formale Review: Callable Traits

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On Mon, Apr 10, 2017 at 1:48 PM, Peter Dimov via Boost
<[hidden email]> 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.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost