Re: [boost] Formale Review: Callable Traits

Subject: Re: [boost] Formale Review: Callable Traits
From: Barrett Adair (barrettellisadair_at_[hidden])
Date: 2017-04-13 01:14:10

On Mon, Apr 10, 2017 at 1:48 PM, Peter Dimov via Boost
<boost_at_[hidden]> 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.