Re: [boost] [CallableTraits] The formal review begins today

Subject: Re: [boost] [CallableTraits] The formal review begins today
From: Tim Song (t.canens.cpp_at_[hidden])
Date: 2017-04-03 11:06:54

I don't think I've participated on this list enough to do a formal
review, but here are some comments from a first look:

- The documentation misuses the term "implementation-defined"
throughout; "see below" seems to be closer to what is intended.

- Technically, the fallback definitions of foo_v are all ill-formed NDR.

- The description of behavior seems a bit awkward due to the use of
the passive tense: "std::false_type is inherited by
is_lvalue_reference_member<T> and is aliased by typename
is_lvalue_reference_member<T>::type, except when one of the following
criteria is met, in which case std::true_type would be similarly
inherited and aliased". Examining the implementation, it is unclear
why the repeated alias declaration is necessary when false/true_type
already define a member typedef 'type' that is itself.

- The traits do not, but should, handle top-level cv-qualification. I
see no reason why, say, is_volatile_member<int (foo::*)() volatile> is
true but not is_volatile_member<int (foo::* const)() volatile>.

- "reference collapsing" behavior on the add ref-qualifier traits is
subtle and arbitrary and I'm not aware of use cases calling for such
rules. The FAQ's argument - that a different set of rules would be
hard to memorize - does not sound very convincing to me. It sounds
like the problem came from the name of the trait: "adding" a &&
qualifier to a &-qualified function type doesn't make much sense. If,
say, the name is recast as "make the function type && qualified", then
the behavior would be obvious.

- Speaking of names, I'm not a big fan of names like
"is_reference_member" for a trait that's actually "has ref-qualifier".
Likewise for something like "is_volatile_member"; my first reaction is
"is it a pointer to member data of volatile-qualified type?". Maybe
the namespace helps, I don't know.

- Consider making the variable templates inline for implementations
that support it, to match C++17.

- apply_member_pointer seems to be trying to do too much. I can get a
pointer to member data of any type, unless that data member is itself
a pointer to member or a pointer to function, in which case I get
something else entirely. Additionally, the description doesn't mention
that PMDs also get special handling.

- transaction_safe is not a "C++17 feature".

- Is there an explanation for qualified_parent_class_of's design,
especially with respect to PMDs (the choice of const &)?

Tim