AMDG I'm not sure whether function_types is ready now or not. I don't really like the tags. Dividing the parameters into two groups (the type arguments and everything else) doesn't seem like the cleanest solution. Even with them it will sometimes be necessary to and_<> several predicates together mpl::and_<is_function<F>, mpl::equal_to<arity<F>, mpl::int_<1> > > so I think making certain properties should not be treated specially. I think that components should probably integrate all the properties a little better. Instead of making it the union of a tag and an mpl sequence would it be better to make it more like mpl metaobjects with appropriate mutators: typedef components<void(int, char)> c; typedef set_result<c, bool>::type c1 typedef set_param<c1, 0, const int&>::type c2; typedef make_variadic<c2>::type c3; typedef get<c3>::type f; // is bool(const int&, char, ...) (Although maybe set_param is asking too much since I don't see the corresponding operation in mpl.) and querying metafunctions: typedef is_variadic<c>::type b; //inherits from mpl::false_ These could be overloaded to take callable builtins too, but that might cause too much overhead translating back and forth between two representations. For both components and the synthesis metafunctions, having the result type as the first element of the sequence doesn't really make sense to me. I would expect that the return type would often need to be handled separately from the parameters in the cases where anything other than simply passing the result of components to function_type<...> is needed. I would like a way to find out what a tag is composed of. I saw represents<> in the code but it isn't documented (unless I missed it) I compiled the following on all the compilers I have with different values for N_TYPES. note that this only uses one partial specialization #include <boost/function_types/components.hpp> #include <boost/function_types/result_type.hpp> #include <boost/function_types/parameter_types.hpp> #include <boost/preprocessor/repetition/repeat.hpp> #include <boost/preprocessor/cat.hpp> #define N_TYPES ... namespace ft = boost::function_types; template<int N> struct Y {}; #define TYPE_AT(n) Y<n> #define CAT(a, b, c) BOOST_PP_CAT(a, BOOST_PP_CAT(b, BOOST_PP_CAT(_, c))) #define TYPE_NAME(n1, n2) CAT(type, n1, n2) #define FUNCTION_TYPE(z, n1, n2) typedef void TYPE_NAME(n2, n1)(TYPE_AT(n2), TYPE_AT(n1)); #define OUTER(z, n, data) BOOST_PP_REPEAT_##z(N_TYPES, data, n) #define NESTED_REPEAT(macro) BOOST_PP_REPEAT_1(N_TYPES, OUTER, macro) NESTED_REPEAT(FUNCTION_TYPE); #define USE_FT_COMPONENTS(z, n1, n2) typedef ft::parameter_types<TYPE_NAME(n2, n1)>::type CAT(component, n2, n1); NESTED_REPEAT(USE_FT_COMPONENTS); msvc 8.0 debug w/o debug N_TYPES time pp time pp 0 1 1 1 1 5 2 1 1 1 10 3 1 2 1 20 10 1 8 1 40 98 2 87 1 msvc 7.1* debug w/o debug N_TYPES time pp time pp 0 2 2 2 2 5 3 2 2 2 10 3 2 2 2 20 17 2 15 2 40 39** 2 37** 2 *I had to use -D"BOOST_FT_CONFIGURATION_OK" **fatal error C1204: compiler limit : internal structure overflow codewarrior 9.2 N_TYPES time pp 0 1 1 5 2 1 10 2 1 20 3 1 30 9 2 40 29 2 A few minor things. pp_tags/master.hpp BOOST_STATIC_CONSTANT(bits_t, combined_bits = LHS_bits ^ RHS_bits ^ (LHS_bits & RHS_mask) ); works but seems obscure to me would (LHS_bits & ~RHS_mask) | RHS_bits be clearer? Here are the mistakes I made when I looked at the documentation initially. If no one else runs into these don't worry about it. It's probably just me. I thought ClassTransform applied to all the parameters not just this. I didn't realize that components was a tag. I saw the capitalized and linked MPL <../../../../../mpl/index.html> - Front <../../../../../mpl/doc/refmanual/front-extensible-sequence.html> / Back <../../../../../mpl/doc/refmanual/back-extensible-sequence.html>Extensible <../../../../../mpl/doc/refmanual/extensible-sequence.html>Random Access Sequence <../../../../../mpl/doc/refmanual/random-access-sequence.html> of all component types and missed the trailing "... and property tag " In Christ, Steven Watanabe
Hi Steven, Steven Watanabe wrote:
AMDG
I'm not sure whether function_types is ready now or not. I don't really like the tags.
Hmm... If it isn't there might be something wrong with how we review things: <cite> [...] The function types library is accepted into Boost, subject to a further mini-review to be conducted at a later time convenient to Tobias. The "mini review" will be conducted independently to the full review process; it's purpose will not be to reopen issues already addressed at this review, but to give reviewers a chance to: * Check the reorganised/renamed classes: [...] * Check the revised documentation. [...] </cite> The previous version was pretty tag-heavy. I put a lot of work into reducing the tag-based interface down to a minimum. Please note that there is no need to touch the tag types for a vast majority of the library's use cases. Now folks complain more about the tags than before. Worse than that, I haven't been presented a single clearly superior alternative!
Dividing the parameters into two groups (the type arguments and everything else) doesn't seem like the cleanest solution. Even with them it will sometimes be necessary to and_<> several predicates together mpl::and_<is_function<F>, mpl::equal_to<arity<F>, mpl::int_<1> > > so I think making certain properties should not be treated specially.
The objective of the tags is to give some (less often required) properties an encapsulation as a type -- not to provide syntactic sugar for nicer queries. In fact, typical use cases will not require that a tag argument is ever given *by design*. Further, the tags are grouped by properties. Maybe the documentation does not reflect the grouping sufficiently, but there is no such thing like an "anything else group" ;-).
I think that components should probably integrate all the properties a little better. Instead of making it the union of a tag and an mpl sequence would it be better to make it more like mpl metaobjects with appropriate mutators: typedef components<void(int, char)> c; typedef set_result<c, bool>::type c1 typedef set_param<c1, 0, const int&>::type c2; typedef make_variadic<c2>::type c3; typedef get<c3>::type f; // is bool(const int&, char, ...) (Although maybe set_param is asking too much since I don't see the corresponding operation in mpl.) and querying metafunctions: typedef is_variadic<c>::type b; //inherits from mpl::false_ These could be overloaded to take callable builtins too, but that might cause too much overhead translating back and forth between two representations.
It seems a bit arguable to me whether we're talking about sugar or clutter, here.
For both components and the synthesis metafunctions, having the result type as the first element of the sequence doesn't really make sense to me. I would expect that the return type would often need to be handled separately from the parameters in the cases where anything other than simply passing the result of components to function_type<...> is needed.
I found it preferable to have a one-type representation, but I'm currently not feeling to strong about it anymore. Your proposal might be a good idea -- I'll think about it. Here is one scenario where the current design seems quite helpful, though: http://tinyurl.com/whkmd (message thread on g.c.l.boost.user)
I would like a way to find out what a tag is composed of. I saw represents<> in the code but it isn't documented (unless I missed it)
Unlike in the previous version, and as stated above, the tags intentionally play a very subordinate role in the interface, so I left out 'represents' and 'extract' for the lack of real world use cases.
I compiled the following on all the compilers I have with different values for N_TYPES.
note that this only uses one partial specialization
It doesn't matter much! The compiler has to instantiate over 3000 templates for N_TYPES == 40. During template instantiation the compiler has to check which partial specialization definition matches. You are using over 3000 distinct types so it will have to find out over 3000 times (even if it ends up picking the same definition, over and over again), looking at all specializations there are, of course. <snip code & benchmarks> I'm really glad to see that things compile with Codewarrior, since I don't have this compiler and thus never tested the library with it ;-)
A few minor things. pp_tags/master.hpp BOOST_STATIC_CONSTANT(bits_t, combined_bits = LHS_bits ^ RHS_bits ^ (LHS_bits & RHS_mask) ); works but seems obscure to me would (LHS_bits & ~RHS_mask) | RHS_bits be clearer?
Right. It's an artefact and used to be an optimization back when it read mpl::bitxor...
Here are the mistakes I made when I looked at the documentation initially. If no one else runs into these don't worry about it. It's probably just me.
I thought ClassTransform applied to all the parameters not just this.
I didn't realize that components was a tag. I saw the capitalized and linked MPL <../../../../../mpl/index.html> - Front <../../../../../mpl/doc/refmanual/front-extensible-sequence.html> / Back <../../../../../mpl/doc/refmanual/back-extensible-sequence.html>Extensible <../../../../../mpl/doc/refmanual/extensible-sequence.html>Random Access Sequence <../../../../../mpl/doc/refmanual/random-access-sequence.html> of all component types and missed the trailing "... and property tag "
This one got confused already, the documentation needs to be fixed at this point. Thank you for your review. Regards, Tobias
Tobias Schwinger wrote:
Now folks complain more about the tags than before. Worse than that, I haven't been presented a single clearly superior alternative!
What's wrong with separate traits for these? So far as I can see, you only need four (plus one per platform specific calling convention): is_variadic<T>::value is_const_member_function<T>::value is_volatile_member_function<T>::value is_default_cc<T>::value These seem much more in the spirit of the existing type traits in Boost. By your own admission, "typical use cases will not require that a tag argument is ever given *by design*", so requring a library user to use mpl::and_ (and perhaps mpl::not_) to fold these in on those atypical use cases that require this information can't be much of a burden. -- Richard Smith
Steven Watanabe wrote:
I think that components should probably integrate all the properties a little better. Instead of making it the union of a tag and an mpl sequence would it be better to make it more like mpl metaobjects with appropriate mutators: typedef components<void(int, char)> c; typedef set_result<c, bool>::type c1 typedef set_param<c1, 0, const int&>::type c2; typedef make_variadic<c2>::type c3; typedef get<c3>::type f; // is bool(const int&, char, ...)
(Although maybe set_param is asking too much since I don't see the corresponding operation in mpl.) and querying metafunctions: typedef is_variadic<c>::type b; //inherits from mpl::false_ These could be overloaded to take callable builtins too, but that might cause too much overhead translating back and forth between two representations.
It seems a bit arguable to me whether we're talking about sugar or clutter, here.
For both components and the synthesis metafunctions, having the result type as the first element of the sequence doesn't really make sense to me. I would expect that the return type would often need to be handled separately from the parameters in the cases where anything other than simply passing the result of components to function_type<...> is needed.
I found it preferable to have a one-type representation, but I'm currently not feeling to strong about it anymore. Your proposal might be a good idea -- I'll think about it.
Here is one scenario where the current design seems quite helpful, though:
http://tinyurl.com/whkmd (message thread on g.c.l.boost.user)
I would like a way to find out what a tag is composed of. I saw represents<> in the code but it isn't documented (unless I missed it)
Unlike in the previous version, and as stated above, the tags intentionally play a very subordinate role in the interface, so I left out 'represents' and 'extract' for the lack of real world use cases.
I compiled the following on all the compilers I have with different values for N_TYPES.
note that this only uses one partial specialization
It doesn't matter much! The compiler has to instantiate over 3000 templates for N_TYPES == 40. During template instantiation the compiler has to check which partial specialization definition matches. You are using over 3000 distinct types so it will have to find out over 3000 times (even if it ends up picking the same definition, over and over again), looking at all specializations there are, of course.
<snip code & benchmarks>
I'm really glad to see that things compile with Codewarrior, since I don't have this compiler and thus never tested the library with it ;-)
A few minor things. pp_tags/master.hpp BOOST_STATIC_CONSTANT(bits_t, combined_bits = LHS_bits ^ RHS_bits ^ (LHS_bits & RHS_mask) ); works but seems obscure to me would (LHS_bits & ~RHS_mask) | RHS_bits be clearer?
Right. It's an artefact and used to be an optimization back when it read mpl::bitxor...
Here are the mistakes I made when I looked at the documentation initially. If no one else runs into these don't worry about it. It's probably just me.
I thought ClassTransform applied to all the parameters not just this.
I didn't realize that components was a tag. I saw the capitalized and linked
MPL <../../../../../mpl/index.html> - Front <../../../../../mpl/doc/refmanual/front-extensible-sequence.html> / Back <../../../../../mpl/doc/refmanual/back-extensible-sequence.html>Extensible <../../../../../mpl/doc/refmanual/extensible-sequence.html>Random Access Sequence <../../../../../mpl/doc/refmanual/random-access-sequence.html> of all component types and missed the trailing "... and property tag "
This one got confused already, the documentation needs to be fixed at this point.
Thank you for your review.
Regards,
Tobias
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
-- Richard Smith
Richard Smith wrote:
Tobias Schwinger wrote:
Now folks complain more about the tags than before. Worse than that, I haven't been presented a single clearly superior alternative!
What's wrong with separate traits for these? So far as I can see, you only need four (plus one per platform specific calling convention):
is_variadic<T>::value is_const_member_function<T>::value is_volatile_member_function<T>::value is_default_cc<T>::value
These seem much more in the spirit of the existing type traits in Boost.
By your own admission, "typical use cases will not require that a tag argument is ever given *by design*", so requring a library user to use mpl::and_ (and perhaps mpl::not_) to fold these in on those atypical use cases that require this information can't be much of a burden.
Two things: 1. How to specify the properties for type synthesis? You proposed to strip it out, but I think that the compromising the symmetry of the library is far to high of a price for unspecified aesthetical reasons. 2. The primary interface would be "shape shifting" based on the configuration: is_fastcall_cc is_stdcall_cc is_*_cc which, is at least as ugly (not to say uglier) as tag types, IMO. Further: none of the reviewers (except Doug Gregor, who was talking about the issue in the very specific context of merger with another library) did answer the following question: What's wrong with the tag types (other than personal taste) in the first place? A "reject" vote for a matter of personal taste is pretty tough, IMO. But sadly it wouldn't surprise me much after reading some of the material from the GIL review... Regards, Tobias
Tobias Schwinger wrote:
Further: none of the reviewers (except Doug Gregor, who was talking about the issue in the very specific context of merger with another library) did answer the following question:
What's wrong with the tag types (other than personal taste) in the first place?
Having just read your other email, I think I now better understand why you want tags -- and why you don't want to go the traits route that I suggested. What I don't like about the current interface is that it seems to use an odd mixture of traits and tags. Let me try to explain. The library is supplying an interface to manipulate function types (and function pointer, function reference and member function pointer types). The two most obvious parts of a function type are the return type and the parameter types. These are queried via traits: result_type< Fn >::type mpl::at_c< parameter_types< Fn >, N >::type However, on top of this, there are other aspects: calling convention and variadic-ness (both of which are probably relatively rarely required), and cv-quals for member functions (which might be more frequently needed). These are all handled by the tags mechanism. is_callable_builtin< Fn, variadic >::value (I'm assuming that is_callable_builtin returns true for functions, references, pointers and members.) Finally, there's exactly what sort of function type we're dealing with -- function, reference, pointer or member. These are handled by separate traits: is_function_reference< Fn >::value is_member_function_pointer< Fn >::value That's three (or four if you include the at_c usage to get the nth parameter, but let's not go there again) subtly different interfaces. I'm not concerned about the difference between ::type and ::value -- as I can put ::type in both cases and get a true_type or false_type. But that still leaves two different interfaces, which IMO is one too many. My traits suggestion removed one interface by ditching the tags, but it wasn't the tag interface that I disliked /per se/ -- it was the inconsistency. What if you went the other way and used an exclusively tag-based interface: namespace bft = boost::function_types; bft::get< int (), result_type >::type // int bft::get< int (), parameter_types >::type // an empty MPL sequence bft::get< void (int*, float), parameter_types >::type // an MPL sequence containing (int*, float) bft::get< void (int*, float), parameter_type<0> >::type // int* -- optional syntactic sugar bft::get< void (...), is_variadic >::type // true_type bft::get< void (...), is_variadic >::value // true bft::get< void (), is_default_cc >::type // true_type bft::get< void (*)(), is_pointer >::type // true_type etc... This is not very dissimilar to the existing is_callable_scalar metafunction, but with addition is_pointer, is_reference, etc... tags folded in.
A "reject" vote for a matter of personal taste is pretty tough, IMO. But sadly it wouldn't surprise me much after reading some of the material from the GIL review...
Hey! I've never said I'm voting to reject ;-) I said in my original review | Although most of my comments are negative, I like this | library, and would like to see it in Boost. I would | prefer to see some modifications made first, but even in | the absense of these, I'd still like to see it added. ... and I still feel this way. Just to avoid any doubt, my vote is to ACCEPT. -- Richard Smith
Richard Smith wrote:
Tobias Schwinger wrote:
Further: none of the reviewers (except Doug Gregor, who was talking about the issue in the very specific context of merger with another library) did answer the following question:
What's wrong with the tag types (other than personal taste) in the first place?
Having just read your other email, I think I now better understand why you want tags -- and why you don't want to go the traits route that I suggested.
What I don't like about the current interface is that it seems to use an odd mixture of traits and tags. Let me try to explain.
The library is supplying an interface to manipulate function types (and function pointer, function reference and member function pointer types). The two most obvious parts of a function type are the return type and the parameter types. These are queried via traits:
result_type< Fn >::type mpl::at_c< parameter_types< Fn >, N >::type
However, on top of this, there are other aspects: calling convention and variadic-ness (both of which are probably relatively rarely required), and cv-quals for member functions (which might be more frequently needed).
Yeah. But cv-qualification for member functions is usually dealt with in form of a qualified class type. The tags are just needed to overide the default -- and for cv-qualified function types (which by the current standard make a program ill-formed).
These are all handled by the tags mechanism.
is_callable_builtin< Fn, variadic >::value
(I'm assuming that is_callable_builtin returns true for functions, references, pointers and members.)
Finally, there's exactly what sort of function type we're dealing with -- function, reference, pointer or member. These are handled by separate traits:
is_function_reference< Fn >::value is_member_function_pointer< Fn >::value
That's three (or four if you include the at_c usage to get the nth parameter, but let's not go there again) subtly different interfaces. I'm not concerned about the difference between ::type and ::value -- as I can put ::type in both cases and get a true_type or false_type.
But that still leaves two different interfaces, which IMO is one too many. My traits suggestion removed one interface by ditching the tags, but it wasn't the tag interface that I disliked /per se/ -- it was the inconsistency.
What if you went the other way and used an exclusively tag-based interface:
namespace bft = boost::function_types;
bft::get< int (), result_type >::type // int
bft::get< int (), parameter_types >::type // an empty MPL sequence
bft::get< void (int*, float), parameter_types >::type // an MPL sequence containing (int*, float)
bft::get< void (int*, float), parameter_type<0> >::type // int* -- optional syntactic sugar
bft::get< void (...), is_variadic >::type // true_type
bft::get< void (...), is_variadic >::value // true
bft::get< void (), is_default_cc >::type // true_type
bft::get< void (*)(), is_pointer >::type // true_type
etc... This is not very dissimilar to the existing is_callable_scalar metafunction, but with addition is_pointer, is_reference, etc... tags folded in.
I've been there. It's very close to the interface of the previous version. The descisions towards the current design and against consistency were a matter of practicability: <cite from yesterday's post> I came to the conclusion that simple and straightforward client code is more important -- and it really did cost me quite an effort to let go of the nagging perfectionism about consistency for consistency's sake. </cite from yesterday's post> Regards, Tobias
participants (3)
-
Richard Smith -
Steven Watanabe -
Tobias Schwinger