Hello Dave, first of all: thank you very, very much for these numerous hints on how to improve the documentation - especially for taking the time to rewrite several passages. Amazing work! Mind if I just "merge" some of this into the next revision? I'll cut down your post for the reply to highlight the points I want to answer to -- and add the others to my todo list. David Abrahams wrote:
[...]
All this stuff goes in namespace boost? I don't think I'm happy about that.
All names are actually defined inside local namespaces. They are injected to the boost namespace via a using directive.
Okay, that's a little better. It avoids some ADL problems but still brings many special-purpose names into the top-level namespace. Do the tag types belong there? Also "local namespace" isn't a well-accepted term. How about "sub-namespaces of boost?"
I don't think the synopsis should show them in namespace boost and then be amended by a footnote at the bottom that many people might not read. At least put a link to the note in the synopsis code.
I was not too sure about this myself. I did this because most libraries put their primary interface into namespace boost.
template<typename F> struct function_type_arity;
template<typename F> struct function_type_result;
template<typename F> struct function_type_class;
template<typename F, typename I> struct function_type_parameter;
template<typename F, long I> struct function_type_parameter_c;
template<typename F> struct function_type_parameters;
Yowch! I really hate to see a boost.whatever library with names like
whatever_foo whatever_bar whatever_baz
in it. Isn't this what namespaces are for?
Okay - if we don't put them in namespace boost, we can also use a different naming for these. I like it, especially because the names are currently too lengthy for my taste.
is_function_type
Members
value - Static constant of type bool, evaluating to true if T is
Uhm, the essential member of any metafunction is ::type, and I don't see it listed here. Value doesn't "evaluate to true", it "is true."
is_function_type<Tag,T> is an MPL Integral Constant. The type member is the identity of its specialization and not listed explicity because it's part of the concept. It won't hurt, I guess.
an element of the set of types specified by Tag. Member function pointers may be more const-volatile qualified than specified,
Hmm, have you looked carefully at the use cases for this? Member functions often have counterintuitive is-a relationships. For example, a base class member pointer is-a derived class member pointer, but not vice-versa. It's not obvious to me that any is-a relationships should hold here. What are the use cases?
I'm thinking about removing cv indication from the tags and cv-qualifying the class type, instead (see comments in the examples interpreter.hpp : line 99 and function_closure.hpp : line 120).
---------
Decomposition
Six class templates are provided to extract properties of a function type, such as arity, parameter-, class (where appropriate)- and result type.
"Six class templates are provided to extract properties of a function type, such as arity and the types of the result, parameters, and the class targets of member functions."
-------
template<typename T> struct function_type_arity;
Template parameters
T - The function type to be inspected. T must be a function type as defined at the beginning of this document (is_function_type<no_function,T>::value == false).
You shouldn't repeat that verbose description all over the place.
"T - Any function type <link to definition>"
is better and more concise.
Members
value - Static constant of type size_t, evaluating to the number of parameters taken by the type of function T describes. The hidden this-parameter of member functions is never counted.
That particular behavior does not match the needs of any use cases I've seen. I always want to treat the hidden this parameter as the function's first parameter (actually as a reference to the class, with cv-qualification given by that of the member function itself). What's the rationale?
The rationale is that there is no unified call syntax for member function pointers and non-member/static function pointers, anyway: Typically we use a cascade of template specializations that does the actual invocation for different function arities. Only counting "real" parameters here allows us to build such a cascade for arities ranging from zero to "OUR_MAX_ARITY_LIMIT" without having to deal with the two special cases that there are no nullary member functions and that we need to go up to OUR_ARITY_LIMIT+1 for member function invocations if the context reference is taken into account. Think about generating the (hand-written) cascade in libs/function_types/example/interpreter.hpp : line 299 with Boost.Preprocessor, for example. Even if completely binding a function (which I believe is a rather seldom use case), we still need a distinction between a member and non-member function pointer which won't go away no matter how we put it. Further we can still take the size of function_type_signature (well, in this case the result type is counted as well).
"I - An MPL Integral Constant describing the index of the parameter type to be returned. The index of the first parameter is zero."
What about the hidden "this" parameter? I think you know the semantics I want, but you don't say what you provide.
The same argumentation applies here. And parameters indices should be consistent with the function arity. Further it makes client code more expressive. I guess it's your turn here to prove my design is faulty ;-). If you can convince me, I'll happily change things, of course.
[...]
I don't know what you mean by this recommendation. If I have a specialization of function_type_signature, how am I supposed to get information about it, or use it? Am I supposed to pass it to the decomposition metafunctions (I don't think so!)?
Use it as an MPL sequence. I added this recommendation because it led to so much confusion discussing this with Jody Hagins (and I probably made things worse). The recommendation refers to using it as a traits bundle: e.g. function_type_signature<T>::arity instead of function_type_arity<T> which should be preferred.
template<typename Tag, typename Ts> struct function_type;
Needs a brief description; maybe all do. Example: "Returns a new function type built from a sequence of types."
Template parameters
Tag - Tag type specifying the kind of function type to be created. Abstract tag types (ones that describe abstract supersets of types such as any_function) are not allowed with the exception of no_function (see below for effect).
"Tag types that describe abstract supersets of types, such as any_function, are not allowed with the exception of no_function (see below<link>)."
Ts - Model of MPL Forward Sequence of sub types, starting with
Drop "Model of"
the result type optionally followed by the class type (if Tag specifies a member function pointer type)
That sounds like if Tag specifies a member function pointer you have the option to represent the class type. Just drop "optionally" and remove the parens.
So here you are using the form I like, where class types are treated just like any other (should be a reference, though). Why not do this uniformly.
See above.
followed by the parameter types, if any. If Ts is not a sequence (mpl::is_sequence<T>::value == false), a specialization of function_type_signature for this type is used.
This is unintelligible. You already said Ts is required to be a sequence. Now you're taking it back? A specialization of
I remember being rather unhappy about this writing it...
function_type_signature for *which* type is used? Used how?
... it needs to be rewritten.
This may fail if function_type_signature is an incomplete template due to forward-declaration without definition.
Why would that ever be the case? Shouldn't your library take care of it?
Both 'function_type_signature' (which is the backend for all higher-level inspection components) and function_type need quite a bit of code (and when used in preprocessing mode a significant ammount of compile time). That's why I decided to not let one depend upon the other, but to make them plug into each other when both are defined.
. If Tag is no_function, type is identical to Ts if Ts is an MPL Sequence other than a specialization of function_type_signature, in which case an type is an mpl::vector (this is primarily for internal use of the library but, since it may be useful in some situations, there is no reason to hide it from the user).
Oh yes there is!! I can't for the life of me understand these semantics. Guideline: anything whose specification is too complicated to explain easily probably needs to be redesigned.
It used to be a bit more understandable before I removed a rather strange feature to grab the signature from a class template's parameter list. I will just remove this as well.
Also, special-case "if" clauses, especially those that test whether something matches a concept (like "is_sequence") tend to destroy genericity.
Would you mind explaining this in some more detail?
----
Portability
Currently this library does not contain any workarounds for broken compilers. It was successfully tested with Microsoft Visual C++ Versions 7.1 to 8.0, Intel Linux Versions 7.0 to 8.1 and GCC Version 3.2 to 4.0. The library is expected to work with compiler using the Edison Design Group frontend other than Intel, although currently there is no proof for this.
Okay, I appreciate the desire to avoid broken compiler workarounds.
Okay. The desire to avoid broken compiler workarounds applies especially to the review version. I had hoped the review to answer how much portability is required for proper Boost integration. I definitely plan on extending the list of supported compilers.
However, many of the libraries in Boost that could use this library do support broken compilers, and therefore -- I think -- are not using idioms that can readily leverage metafunctions for this purpose. The true measure of whether this library should be accepted, IMO, is in whether it can be used to eliminate large amounts of code from other libs. Can it? The quickest way to a proof-of-concept is to apply it.
Good idea!
Incidentally, I think http://lists.boost.org/MailArchives/boost/msg81758.php makes it possible to implement almost any trait we previously thought was impossible in pre-7.1 Visual C++.
If the folks at Borland don't fix their compiler they should at least add this as a new bug...
I'm not ready to accept this library, and I'm not ready to reject it either. It has the potential to be extraordinarily useful, but that's unproven as far as I'm concerned. I have some substantial complaints with the interface, and I have some serious problems iwth the docs as you can see (not all of those remarks are trivial grammatical edits!)
Summary: - proof it by making some boost libraries use it - change naming - change docs I hope (but am not entirely sure) it's all doable within the review period. Any hints on a preferred prioritization? Regards, Tobias