Hi, Here's my review of the function types submission. I'm sorry I waited until the last minute to submit my review. I vote to accept the library, even though I have some problems with the design.
What is your evaluation of the potential usefulness of the library?
The library is potentially very useful. For my interfaces library ( http://www.kangaroologic.com/interfaces/) I needed a facility for constructing and manipulating function types. Because nothing was available, I wrote this: http://www.kangaroologic.com/function_traits/libs/function_traits/doc/ However, as Tobias politely pointed out, my implementation was a bit naive. It would be valuable to have a single correct and efficient implementation which could be shared by various current and proposed boost libraries.
What is your evaluation of the design?
My main criticism is that the metafunctions taking tags are given too prominent a place in the library. I would rather have the library provide a collection metafunctions to handle the most common combinations of tags and provide the more generic metafunctions for 'advanced' use. For example, I'd like to have metafunctions 'is_[plain_]function_type,' 'is_function_pointer,' 'is_function_reference' and 'is_const_member_function_pointer,' like I provided in the above mentioned library, in addition to 'is_function_type.' Also, I'd like to see specialized metafunctions for constructing (plain) function types, function pointers and function references, in addition to the general purpose metafunction 'function_type.' Next, function_type_signature is too complex; it has too many members, and their specifications are hard to understand. I would like to see separate metafunctions which take a function type and return 1. the sequence of arguments; you might call this function_type::arguments 2. the sequence of arguments, including ant implicit 'this' pointer; you might call this function_type::effective_arguments 3. the sequence (return_type, [class,] arg0, ... argN). You could call this function_type::signature. Alternatively, the result of instantiating the templates might themselves be sequences. Next, it's too hard too keep track of all the different tags, esp. remembering the order of the components of the names. Why not have a few atomic tags, and let user's combine them via inheritance? E.g., struct const_tag { }; struct volatile_tag { }; struct reference_tag { }; struct variadic_tag { }; struct stdcall_tag { }; struct my_tag : reference_tag, stdcall_tag, ... { }; typedef is_function_type<my_tag, T>::type result; You could define a bunch of convenience tags for common cases, but people wouldn't be required to learn them. This is what the iostreams library does. Finally, I don't like the fact that the term 'function type' is used to refer to pointers and references. This is bound to cause confusion. Also, it means that you have to write 'plain' all over the place. In my library, I used the term 'signature type,' which I wasn't too happy with, but at least it was clear that I wasn't referring to any pre-existing category of types.
What is your evaluation of the implementation?
I studied an early implementation that didn't define any MPL sequences. It was good.
What is your evaluation of the documentation?
The documentation needs major improvement. This has been addressed thoroughly in other reviews.
Did you try to use the library? With what compiler? Did you have any problems?
I used previous versions of the library, but not the current version.
How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
I've spent much time considering the problem domain, but only about two hours during this review period.
Are you knowledgeable about the problem domain?
Yes. Jonathan
Hi Jonathan, Jonathan Turkanis wrote:
Hi,
Here's my review of the function types submission. I'm sorry I waited until the last minute to submit my review.
I vote to accept the library, even though I have some problems with the design.
Thank you for your review and for the positive vote.
What is your evaluation of the design?
My main criticism is that the metafunctions taking tags are given too prominent a place in the library. I would rather have the library provide a collection metafunctions to handle the most common combinations of tags and provide the more generic metafunctions for 'advanced' use.
For example, I'd like to have metafunctions 'is_[plain_]function_type,' 'is_function_pointer,' 'is_function_reference' and 'is_const_member_function_pointer,' like I provided in the above mentioned library, in addition to 'is_function_type.' Also, I'd like to see specialized metafunctions for constructing (plain) function types, function pointers and function references, in addition to the general purpose metafunction 'function_type.'
Basically a set of wrappers... I like it, especially in conjunction with Dave's request to put things into a sub namespace of boost. These expressive forms could go into boost (given they don't collide with TypeTraits) then.
Next, function_type_signature is too complex; it has too many members, and their specifications are hard to understand. I would like to see separate metafunctions which take a function type and return
This impression come from the fact that the documentation is currently way too weak, especially at this point. The primary interface of this class template is not to use most of these type member and use this class as an MPL Sequence (I'll cite a section of previous review discussion, which I hope explains this in more detail). Most of these members are for optimization (saving template instantiations) so I vote for efficieny and not putting them into wrappers. Even though using the members of 'function_type_signature' directly is pretty close to the gory details, I don't think it should be an implementation detail. <CITE> Tobias Schwinger wrote:
John Maddock wrote:
Reading the docs, I don't understand what function_type_signature is for, especially when it's members are not recomended for general use. Should this be an implementation detail?
No, but the docs need to be fixed, here.
'function_type_signature' a model of MPL Random Access sequence (even an "extensible" one if the synthesis part has been enabled by #including 'function_type.hpp').
The 'kind' member is the "missing piece of information" to fully describe the encapsulated type (in combination with the subtypes stored in the sequence).
The 'representee' member is neccessary to get the type which it (e.g. a modified sequence) describes.
Further (what follows is a preliminary try to improve this recommendation):
'function_type_signature' can be used to achieve similar functionality as provided by the decomposition components.
This application, however, is not recommended (*), because there is no implied assertion that its template argument really is a function type and because its type members form a less expressive interface for decomposistion.
(*) It can be an opportunity for optimization in heavily repetitive situations to reduce the number of template instantiations required or to avoid to depend on the synthesis part of the library if an extensible sequence is needed which does not have to be reassembled to a function type.
</CITE>
1. the sequence of arguments; you might call this function_type::arguments
It already exists and is currently called 'function_type_parameters'. Btw. I prefer the term 'parameter' over 'argument' in this context, because a (formal) parameter constraints the argument (which is also called an actual parameter). I.e. (translated Standardese) the argument is what is passed for a parameter. I know e.g. 'std::unary_function::argument_type' -- but this is not exactly the same (I interpret it like: "please give me an argument of this type").
2. the sequence of arguments, including ant implicit 'this' pointer; you might call this function_type::effective_arguments
I've been thinking about this too already... This functionaliy has been requested by David Abrahams, already. Proably with a second parameter (MPL lambda expression ? enum ?) to say whether you want a "this pointer" or a "this reference" or the undecorated type or even stripped cv (currently cv is handled via tags, this will be changed so it is done with a cv-qualified class type, instead). Could be practical for 'function_type_class' as well.
3. the sequence (return_type, [class,] arg0, ... argN). You could call this function_type::signature.
Alternatively, the result of instantiating the templates might themselves be sequences.
It's like this, already. You can either use e.g: mpl::at< function_type_parameter<T>, I > mpl::at< function_type_signature<T>, I > or mpl::at< typename function_type_parameter<T>::type, I > mpl::at< typename function_type_signature<T>::type, I > In fact, the MPL Sequence concept requires this.
Next, it's too hard too keep track of all the different tags, esp. remembering the order of the components of the names. Why not have a few atomic tags, and
I like the idea of atomic tags to query aspects instead of full combinations...
let user's combine them via inheritance? E.g.,
struct const_tag { }; struct volatile_tag { }; struct reference_tag { }; struct variadic_tag { }; struct stdcall_tag { };
struct my_tag : reference_tag, stdcall_tag, ... { };
..making this work with inheritance, in particular, might be inefficient. though (internally these are just MPL Integral Constant bit masks).
typedef is_function_type<my_tag, T>::type result;
You could define a bunch of convenience tags for common cases, but people wouldn't be required to learn them. This is what the iostreams library does.
This is a bit opposite to the suggestion to remove the dominant role of tags...
Finally, I don't like the fact that the term 'function type' is used to refer to pointers and references. This is bound to cause confusion. Also, it means that you have to write 'plain' all over the place. In my library, I used the term 'signature type,' which I wasn't too happy with, but at least it was clear that I wasn't referring to any pre-existing category of types.
OK - a standard reader's issue ;-). I'm not too happy about it either (especially the plain_ prefix sucks), but 'signature term' is not very expressive, which is why I don't like it. Someone out there with a better term for us ?! Regards, Tobias
Tobias Schwinger wrote:
Hi Jonathan,
Next, function_type_signature is too complex; it has too many members, and their specifications are hard to understand. I would like to see separate metafunctions which take a function type and return
This impression come from the fact that the documentation is currently way too weak, especially at this point.
The primary interface of this class template is not to use most of these type member and use this class as an MPL Sequence (I'll cite a section of previous review discussion, which I hope explains this in more detail).
Most of these members are for optimization (saving template instantiations) so I vote for efficieny and not putting them into wrappers. Even though using the members of 'function_type_signature' directly is pretty close to the gory details, I don't think it should be an implementation detail.
<snip citation> I still don't quite understand it. Even if all the work is done by function_type_signature, can't you give it a more user-friendly interface -- perhaps a simple wrapper which hides the 'advanced' members? I will admit I haven't put a long time into studying 'function_type_signature'; my comments basically just reflect my impression that 'function_type_signature' is strange and complex.
1. the sequence of arguments; you might call this function_type::arguments
It already exists and is currently called 'function_type_parameters'.
Okay, sorry I missed it.
Btw. I prefer the term 'parameter' over 'argument' in this context, because a (formal) parameter constraints the argument (which is also called an actual parameter). I.e. (translated Standardese) the argument is what is passed for a parameter. I know e.g. 'std::unary_function::argument_type' -- but this is not exactly the same (I interpret it like: "please give me an argument of this type").
I, too, try to maintain the distinction between parameters and arguments. I'm not sure it's decisive here, though, since I usually think of a parameter as comprising a type *and* a formal name.
2. the sequence of arguments, including ant implicit 'this' pointer; you might call this function_type::effective_arguments
I've been thinking about this too already... This functionaliy has been requested by David Abrahams, already.
Alternatively, the result of instantiating the templates might themselves be sequences.
It's like this, already. You can either use e.g:
mpl::at< function_type_parameter<T>, I > mpl::at< function_type_signature<T>, I >
I understand; I was just saying that I thought function_type_signature was too complicated, and listing the functionality that I thought should be available one way or another.
Next, it's too hard too keep track of all the different tags, esp. remembering the order of the components of the names. Why not have a few atomic tags, and
I like the idea of atomic tags to query aspects instead of full combinations...
let user's combine them via inheritance? E.g.,
struct const_tag { }; struct volatile_tag { }; struct reference_tag { }; struct variadic_tag { }; struct stdcall_tag { };
struct my_tag : reference_tag, stdcall_tag, ... { };
..making this work with inheritance, in particular, might be inefficient. though (internally these are just MPL Integral Constant bit masks).
It involves an extra level of indirection. However, I think it could be done in a way that adds only a miniscule amount of compile time when using the built-in tags.
typedef is_function_type<my_tag, T>::type result;
You could define a bunch of convenience tags for common cases, but people wouldn't be required to learn them. This is what the iostreams library does.
This is a bit opposite to the suggestion to remove the dominant role of tags...
I don't see how this is opposite. Jonathan
Hi Jonathan, thanks for your response! My second reply to your review was sent almost simultanously... Jonathan Turkanis wrote:
Tobias Schwinger wrote:
Hi Jonathan,
Next, function_type_signature is too complex; it has too many members, and their specifications are hard to understand. I would like to see separate metafunctions which take a function type and return
This impression come from the fact that the documentation is currently way too weak, especially at this point.
The primary interface of this class template is not to use most of these type member and use this class as an MPL Sequence (I'll cite a section of previous review discussion, which I hope explains this in more detail).
Most of these members are for optimization (saving template instantiations) so I vote for efficieny and not putting them into wrappers. Even though using the members of 'function_type_signature' directly is pretty close to the gory details, I don't think it should be an implementation detail.
<snip citation>
I still don't quite understand it. Even if all the work is done by function_type_signature, can't you give it a more user-friendly interface -- perhaps a simple wrapper which hides the 'advanced' members? I will admit I haven't put a long time into studying 'function_type_signature'; my comments basically just reflect my impression that 'function_type_signature' is strange and complex.
I really believe it's bad documentation that gives you this impression. Some members can't be hidden because they make up the MPL-Sequence concept ('type', 'tag'). Other members provide essential information: 'kind' and 'representee'. 'types' is for optimization (not going through the decoration). So the only member I could hide is 'arity', I guess. Probably I should clearly point this out in the docs. By coloring, for example.
Btw. I prefer the term 'parameter' over 'argument' in this context, because a (formal) parameter constraints the argument (which is also called an actual parameter). I.e. (translated Standardese) the argument is what is passed for a parameter. I know e.g. 'std::unary_function::argument_type' -- but this is not exactly the same (I interpret it like: "please give me an argument of this type").
I, too, try to maintain the distinction between parameters and arguments. I'm not sure it's decisive here, though, since I usually think of a parameter as comprising a type *and* a formal name.
An interesting way to look at it! Doesn't entirely convince me since the argument only exists at the call site... And because it's all about types, anyway - my thinking was something like: "You can't know the argument's type - you can only know the one of the parameter".
Next, it's too hard too keep track of all the different tags, esp. remembering the order of the components of the names. Why not have a few atomic tags, and
I like the idea of atomic tags to query aspects instead of full combinations...
let user's combine them via inheritance? E.g.,
struct const_tag { }; struct volatile_tag { }; struct reference_tag { }; struct variadic_tag { }; struct stdcall_tag { };
struct my_tag : reference_tag, stdcall_tag, ... { };
..making this work with inheritance, in particular, might be inefficient. though (internally these are just MPL Integral Constant bit masks).
It involves an extra level of indirection. However, I think it could be done in a way that adds only a miniscule amount of compile time when using the built-in tags.
Not entirely convinced design-wise either because it's quite error prone. I like your idea of banning the tags from the primary interface, entirely - and adding "aspect tags" only for querying. Most cases of synthesis can work "tagless" by swapping the 'Tag' and 'Types' parameter of 'function_type', defaulting to 'plain_function', as described in my second reply to your review.
You could define a bunch of convenience tags for common cases, but people wouldn't be required to learn them. This is what the iostreams library does.
This is a bit opposite to the suggestion to remove the dominant role of tags...
I don't see how this is opposite.
We already put the convenience cases in seperate metafunctions, don't we? Having "atomic tags" (or let's call them "aspect tags") for querying, cv-qualification as part of the class type, common case metafunctions we'ld barely ever need to use them. My next post will be a summary of what I think could be a reasonable synopsis (based on review comments and on my own experience from having used the library in several different places now). Regards, Tobias
Tobias Schwinger wrote:
Hi Jonathan,
Jonathan Turkanis wrote:
Hi,
Here's my review of the function types submission. I'm sorry I waited until the last minute to submit my review.
I vote to accept the library, even though I have some problems with the design.
Thank you for your review and for the positive vote.
What is your evaluation of the design?
My main criticism is that the metafunctions taking tags are given too prominent a place in the library. I would rather have the library provide a collection metafunctions to handle the most common combinations of tags and provide the more generic metafunctions for 'advanced' use.
For example, I'd like to have metafunctions 'is_[plain_]function_type,' 'is_function_pointer,' 'is_function_reference' and 'is_const_member_function_pointer,' like I provided in the above mentioned library, in addition to 'is_function_type.' Also, I'd like to see specialized metafunctions for constructing (plain) function types, function pointers and function references, in addition to the general purpose metafunction 'function_type.'
Basically a set of wrappers... I like it, especially in conjunction with Dave's request to put things into a sub namespace of boost. These expressive forms could go into boost (given they don't collide with TypeTraits) then.
Some more thoughts on this: Having this sort of interface, I'ld like to put the tag after the other parameter for the "advanced" functions taking a tag. The "metafunctions for most common cases" (as proposed in the review) can be implemented directly (rather than wrapping is_function_type) to save a template instantiation, of course. These would be candidates to be merged with TypeTraits. Something like this: // Pseudo code #if BOOST_FT_CLASSIFICATION_WORKS_ON_CURRENT_COMPILER using boost::function_types::is_member_function_pointer // for example #else // highly portable code #endif would allow a smooth transistion. I'm not really sure on the "fine grained metafunctions for type synthesis" part of your suggestion, though. It's a pretty "advanced" use case, isn't it? Putting the tag at the end of the template parameter list allows a default parameter like 'plain_function', so we wouldn't need a tag in most cases: function_type<Sig>::type function_type<Sig>::type & function_type<Sig>::type * Comments welcome! Regards, Tobias
"Jonathan Turkanis" <technews@kangaroologic.com> writes:
Finally, I don't like the fact that the term 'function type' is used to refer to pointers and references. This is bound to cause confusion. Also, it means that you have to write 'plain' all over the place. In my library, I used the term 'signature type,' which I wasn't too happy with, but at least it was clear that I wasn't referring to any pre-existing category of types.
How about "callable scalar [type]?" -- Dave Abrahams Boost Consulting www.boost-consulting.com
David Abrahams wrote:
"Jonathan Turkanis" <technews@kangaroologic.com> writes:
Finally, I don't like the fact that the term 'function type' is used to refer to pointers and references. This is bound to cause confusion. Also, it means that you have to write 'plain' all over the place. In my library, I used the term 'signature type,' which I wasn't too happy with, but at least it was clear that I wasn't referring to any pre-existing category of types.
How about "callable scalar [type]?"
The direction is pretty good - seems it excludes the "plain" ones, though. Perhaps "functional scalar [type]" ?
Tobias Schwinger <tschwinger@neoscientists.org> writes:
David Abrahams wrote:
"Jonathan Turkanis" <technews@kangaroologic.com> writes:
Finally, I don't like the fact that the term 'function type' is used to refer to pointers and references. This is bound to cause confusion. Also, it means that you have to write 'plain' all over the place. In my library, I used the term 'signature type,' which I wasn't too happy with, but at least it was clear that I wasn't referring to any pre-existing category of types.
How about "callable scalar [type]?"
The direction is pretty good - seems it excludes the "plain" ones,
Sorry, which ones are those? Plain functions are surely callable.
though. Perhaps "functional scalar [type]" ?
No worse, but is it better? -- Dave Abrahams Boost Consulting www.boost-consulting.com
David Abrahams wrote:
Tobias Schwinger <tschwinger@neoscientists.org> writes:
David Abrahams wrote:
"Jonathan Turkanis" <technews@kangaroologic.com> writes:
Finally, I don't like the fact that the term 'function type' is used to refer to pointers and references. This is bound to cause confusion. Also, it means that you have to write 'plain' all over the place. In my library, I used the term 'signature type,' which I wasn't too happy with, but at least it was clear that I wasn't referring to any pre-existing category of types.
How about "callable scalar [type]?"
The direction is pretty good - seems it excludes the "plain" ones,
Sorry, which ones are those? Plain functions are surely callable.
*LOL* - you're right, of course...
though. Perhaps "functional scalar [type]" ?
No worse, but is it better?
Not sure. Are plain functions scalar, BTW. ? Regards, Tobias
participants (3)
-
David Abrahams -
Jonathan Turkanis -
Tobias Schwinger