Are you knowledgeable about the problem domain? ----------------------------------------------- I have done some metaprogramming and made some use of SFINAE, but my knowledge of the program domain is not extensive. How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? -------------------------------------------------------------------------------------------- I have glanced over the documentation a few times over the last few days in order to give myself a chance to ponder it subconsciously, but I have probably only spent about an hour or two in total actively examining and thinking about the library, plus about an hour writing this review. Do you think the library should be accepted as a Boost library? --------------------------------------------------------------- I would say yes from what I have seen and understand about the library, but based on my relative lack of knowledge in the program domain and the relatively small amount of time I have spent looking at the library my vote should be weighted accordingly. What is your evaluation of the design? -------------------------------------- 1) I agree with others that it makes more sense to inject the metafunctions at the current scope rather at the absolute scope boost::tti. 2) I strongly think that "CREATE_METAFUNCTION_FOR" (or something similar) should be inserted into all of the macro names because otherwise they strike me as being misleading. If I were not familiar with this library and I saw the macro BOOST_TTI_HAS_TYPE(X) in someone else's code, I would be confused because the name of the macro makes it sound like it is asking if X has some type even though the result doesn't seem to be used. By contrast, if I saw the macro BOOST_TTI_CREATE_METAFUNCTION_FOR_HAS_TYPE(X), it would be immediately clear to me that the purpose of this macro is to create a metafunction. I say this based in part on my own experience. When I was learning how this library worked I started by skimming through the documentation to get a feel for what was going on, and the macro names confused me because I couldn't tell what they were actually doing. I did eventually figure this out when I read through the documentation more closely, but if they had names following the convention I am recommending then I would have immediately understood what they were doing. Although my proposed change would require additional typing (though unlikely not much, since I suspect most people are like me and just copy and paste from a template), I think that the benefit of improving the clarity would be well worth it. 3) I wonder if it would be better to instead of giving the metafunctions names such as has_type_X, it would make more sense to put them in their own namespace so they they would be referred to as for example has_type::X. This would save use from having to use macro pasting to compute the metafunction names, and would also have the advantage of addressing Lorenzo's concern that a double-underscore might inadvertently be put in a type name. However, I do not have enough experience here to get a full sense of the design implications of choosing this convention over the current convention. Another possibility is to hide the actual name of the metafunction by using a macro to access it. If my recommendation to insert "CREATE_METAFUNCTION_FOR" in all of the macro names is followed, it would free up the current macro names to be used to refer to the metafunction. That is, rather than code looking like BOOST_TTI_HAS_TYPE(X) boost::tti::has_type_X<T>::... it would look like BOOST_TTI_DECLARE_METAFUNCTION_FOR_HAS_TYPE(X) BOOST_TTI_HAS_TYPE(X,T)::... which would have the advantage of hiding the exact name and location of the metafunction as an implementation detail. To be clear, point #3 is just me pondering other possibilities to invite discussion, and should not be considered a necessary change that needs to be made. What is your evaluation of the implementation? ---------------------------------------------- I glanced at the code and it looked reasonable on the surface but I am not an expert in metaprogramming so I did not look at it closely. What is your evaluation of the documentation? --------------------------------------------- As others have pointed out, the documentation desperately needs more examples. In particular, unless I missed something I do not believe that there are any examples of actually using the library to accomplish something; all of the examples only show which macros should be used to create the metafunctions and what the resulting metafunction is called. In no example did I see an example where the metafunction was actually applied to accomplish something, such as disabling a function using SFINAE. I suspect that the author assumed that such in-depth examples would not be necessary since anyone reading the documentation would probably be very experienced with metaprogramming and so would already know what to do once they had the introspection metafunctions available to them, but not everyone thinking about using this library will be so experienced at metaprogramming (myself being one member of this ignorant group) and so some examples of how the library could be used to accomplish goals would be incredibly useful to us so that we could get a better sense of exactly how this library works and how we can leverage it to solve practical problems. What is your evaluation of the potential usefulness of the library? ------------------------------------------------------------------- It seems like this library could be incredibly useful to those who need introspection functionality, but since I do not generally do the type of metaprogramming that this library facilitates this should be considered as just my speculation. Did you try to use the library? With what compiler? Did you have any problem? ----------------------------------------------------------------------------- I played with it a little bit and compiled a random selection of the examples successfully. I used gcc version 4.2.1 on OSX. Cheers, Greg
On Sun, Jul 17, 2011 at 10:07 PM, Gregory Crosswhite < gcross@phys.washington.edu> wrote: [...]
2) I strongly think that "CREATE_METAFUNCTION_FOR" (or something similar) should be inserted into all of the macro names because otherwise they strike me as being misleading. If I were not familiar with this library and I saw the macro BOOST_TTI_HAS_TYPE(X) in someone else's code, I would be confused because the name of the macro makes it sound like it is asking if X has some type even though the result doesn't seem to be used. By contrast, if I saw the macro BOOST_TTI_CREATE_METAFUNCTION_FOR_HAS_TYPE(X), it would be immediately clear to me that the purpose of this macro is to create a metafunction.
I say this based in part on my own experience. When I was learning how this library worked I started by skimming through the documentation to get a feel for what was going on, and the macro names confused me because I couldn't tell what they were actually doing. I did eventually figure this out when I read through the documentation more closely, but if they had names following the convention I am recommending then I would have immediately understood what they were doing.
Although my proposed change would require additional typing (though unlikely not much, since I suspect most people are like me and just copy and paste from a template), I think that the benefit of improving the clarity would be well worth it.
I think this is a valid concern. For comparison, the introspection macros in Boost.MPL (which, I think, were part of the inspiration for TTI) are BOOST_MPL_HAS_XXX_TEMPLATE_DEF BOOST_MPL_HAS_XXX_TEMPLATE_NAMED_DEF BOOST_MPL_HAS_XXX_TRAIT_DEF BOOST_MPL_HAS_XXX_TRAIT_NAMED_DEF I kind of like the _DEF suffixes, e.g., maybe we could consider something like BOOST_TTI_HAS_TYPE_DEF, etc., for the metafunction-generating macros?
3) I wonder if it would be better to instead of giving the metafunctions names such as has_type_X, it would make more sense to put them in their own namespace so they they would be referred to as for example has_type::X. This would save use from having to use macro pasting to compute the metafunction names,
Agreed.
and would also have the advantage of addressing Lorenzo's concern that a double-underscore might inadvertently be put in a type name.
Agreed; but I'm not sure how much of a concern this is. How many class authors do you know use a leading underscore for any of their (logically) public members?
However, I do not have enough experience here to get a full sense of the design implications of choosing this convention over the current convention.
To be sure, it precludes generating metafunctions at class scope. AFAIK, the current metafunction-generating macros also must be invoked at namespace scope; and, separately, I believed someone thinks it is a valid use case to generate metafunctions at class scope (a workaround, though, would be to generate the metafunctions in a detail namespace and use metafunction forwarding within the class). Another possibility is to hide the actual name of the metafunction by using
a macro to access it. If my recommendation to insert "CREATE_METAFUNCTION_FOR" in all of the macro names is followed, it would free up the current macro names to be used to refer to the metafunction. That is, rather than code looking like
BOOST_TTI_HAS_TYPE(X) boost::tti::has_type_X<T>::...
it would look like
BOOST_TTI_DECLARE_**METAFUNCTION_FOR_HAS_TYPE(X) BOOST_TTI_HAS_TYPE(X,T)::...
which would have the advantage of hiding the exact name and location of the metafunction as an implementation detail.
I don't think we should be hiding the exact location of the metafunction entirely, as we want to prevent name collision when, say, 2 libraries attempt to generate the same metafunction from different namespaces. And, to be sure, it would probably be better to just generate the name only of the metafunction: BOOST_TTI_HAS_TYPE( X )<T>::... Otherwise, you quickly run into (unnessary) complications when T is a template instantiation with commas in it. I don't think this is a bad idea necessarily on any technical grounds (yet), but I do think it's a bad idea on aesthetic grounds :/
To be clear, point #3 is just me pondering other possibilities to invite discussion, and should not be considered a necessary change that needs to be made.
I think these are valid ideas to bring to the table. - Jeff
On 7/18/11 10:51 AM, Jeffrey Lee Hellrung, Jr. wrote:
I think this is a valid concern. For comparison, the introspection macros in Boost.MPL (which, I think, were part of the inspiration for TTI) are
BOOST_MPL_HAS_XXX_TEMPLATE_DEF BOOST_MPL_HAS_XXX_TEMPLATE_NAMED_DEF BOOST_MPL_HAS_XXX_TRAIT_DEF BOOST_MPL_HAS_XXX_TRAIT_NAMED_DEF
I kind of like the _DEF suffixes, e.g., maybe we could consider something like BOOST_TTI_HAS_TYPE_DEF, etc., for the metafunction-generating macros?
While I would personally prefer to have the prefix/suffix be a bit more verbose to make its meaning more explicit, if that is the convention that MPL uses then it probably makes the most sense to adopt it for TTI as well.
and would also have the advantage of addressing Lorenzo's concern that a double-underscore might inadvertently be put in a type name.
Agreed; but I'm not sure how much of a concern this is. How many class authors do you know use a leading underscore for any of their (logically) public members?
I agree that in practice it is unlikely to be much of a concern.
However, I do not have enough experience here to get a full sense of the design implications of choosing this convention over the current convention.
To be sure, it precludes generating metafunctions at class scope.
Good point; I hadn't thought of that.
Another possibility is to hide the actual name of the metafunction by using
a macro to access it. If my recommendation to insert "CREATE_METAFUNCTION_FOR" in all of the macro names is followed, it would free up the current macro names to be used to refer to the metafunction. That is, rather than code looking like
BOOST_TTI_HAS_TYPE(X) boost::tti::has_type_X<T>::...
it would look like
BOOST_TTI_DECLARE_**METAFUNCTION_FOR_HAS_TYPE(X) BOOST_TTI_HAS_TYPE(X,T)::...
which would have the advantage of hiding the exact name and location of the metafunction as an implementation detail.
I don't think we should be hiding the exact location of the metafunction entirely, as we want to prevent name collision when, say, 2 libraries attempt to generate the same metafunction from different namespaces.
And, to be sure, it would probably be better to just generate the name only of the metafunction: BOOST_TTI_HAS_TYPE( X )<T>::... Otherwise, you quickly run into (unnessary) complications when T is a template instantiation with commas in it.
I don't think this is a bad idea necessarily on any technical grounds (yet), but I do think it's a bad idea on aesthetic grounds :/
Fair enough. :-) Cheers, Greg
On Mon, Jul 18, 2011 at 11:22 AM, Gregory Crosswhite < gcross@phys.washington.edu> wrote:
On 7/18/11 10:51 AM, Jeffrey Lee Hellrung, Jr. wrote:
I think this is a valid concern. For comparison, the introspection macros in Boost.MPL (which, I think, were part of the inspiration for TTI) are
BOOST_MPL_HAS_XXX_TEMPLATE_DEF BOOST_MPL_HAS_XXX_TEMPLATE_**NAMED_DEF BOOST_MPL_HAS_XXX_TRAIT_DEF BOOST_MPL_HAS_XXX_TRAIT_NAMED_**DEF
I kind of like the _DEF suffixes, e.g., maybe we could consider something like BOOST_TTI_HAS_TYPE_DEF, etc., for the metafunction-generating macros?
While I would personally prefer to have the prefix/suffix be a bit more verbose to make its meaning more explicit, if that is the convention that MPL uses then it probably makes the most sense to adopt it for TTI as well.
Actually, now that I take a second look at it, putting TYPE and DEF together like this might not solve the naming problem you brought up... How about BOOST_TTI_GENERATE_HAS_TYPE BOOST_TTI_GENERATE_HAS_TYPE_NAMED or s/GENERATE/DEFINE/ or s/GENERATE/CREATE/ or...? - Jeff
On 7/18/11 11:43 AM, Jeffrey Lee Hellrung, Jr. wrote:
Actually, now that I take a second look at it, putting TYPE and DEF together like this might not solve the naming problem you brought up...
How about
BOOST_TTI_GENERATE_HAS_TYPE BOOST_TTI_GENERATE_HAS_TYPE_NAMED
or s/GENERATE/DEFINE/ or s/GENERATE/CREATE/ or...?
Ah, I like GENERATE! It describes exactly what is going on --- i.e., that the purpose of the macro is to GENERATE code for HAS_TYPE. Cheers, Greg
On 7/18/2011 1:07 AM, Gregory Crosswhite wrote:
Are you knowledgeable about the problem domain? -----------------------------------------------
I have done some metaprogramming and made some use of SFINAE, but my knowledge of the program domain is not extensive.
How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? --------------------------------------------------------------------------------------------
I have glanced over the documentation a few times over the last few days in order to give myself a chance to ponder it subconsciously, but I have probably only spent about an hour or two in total actively examining and thinking about the library, plus about an hour writing this review.
Do you think the library should be accepted as a Boost library? ---------------------------------------------------------------
I would say yes from what I have seen and understand about the library, but based on my relative lack of knowledge in the program domain and the relatively small amount of time I have spent looking at the library my vote should be weighted accordingly.
Appreciated.
What is your evaluation of the design? --------------------------------------
1) I agree with others that it makes more sense to inject the metafunctions at the current scope rather at the absolute scope boost::tti.
You are absolutely correct.
2) I strongly think that "CREATE_METAFUNCTION_FOR" (or something similar) should be inserted into all of the macro names because otherwise they strike me as being misleading. If I were not familiar with this library and I saw the macro BOOST_TTI_HAS_TYPE(X) in someone else's code, I would be confused because the name of the macro makes it sound like it is asking if X has some type even though the result doesn't seem to be used. By contrast, if I saw the macro BOOST_TTI_CREATE_METAFUNCTION_FOR_HAS_TYPE(X), it would be immediately clear to me that the purpose of this macro is to create a metafunction.
I can do that but I wonder if programmers really want to type that much <g>. I do not think they do even though I fully agree with you that your macro names are much clearer than the shorter ones.
I say this based in part on my own experience. When I was learning how this library worked I started by skimming through the documentation to get a feel for what was going on, and the macro names confused me because I couldn't tell what they were actually doing. I did eventually figure this out when I read through the documentation more closely, but if they had names following the convention I am recommending then I would have immediately understood what they were doing.
Although my proposed change would require additional typing (though unlikely not much, since I suspect most people are like me and just copy and paste from a template), I think that the benefit of improving the clarity would be well worth it.
It is a decent suggestion but I do explain after all that the macro metafunctions create metafunctions for introspecting an inner element with a given name.
3) I wonder if it would be better to instead of giving the metafunctions names such as has_type_X, it would make more sense to put them in their own namespace so they they would be referred to as for example has_type::X. This would save use from having to use macro pasting to compute the metafunction names, and would also have the advantage of addressing Lorenzo's concern that a double-underscore might inadvertently be put in a type name. However, I do not have enough experience here to get a full sense of the design implications of choosing this convention over the current convention.
Then we are back to injecting a namespace when a macro metafunction is invoked. I do not want to do this for all the reasons others cited.
Another possibility is to hide the actual name of the metafunction by using a macro to access it. If my recommendation to insert "CREATE_METAFUNCTION_FOR" in all of the macro names is followed, it would free up the current macro names to be used to refer to the metafunction. That is, rather than code looking like
BOOST_TTI_HAS_TYPE(X) boost::tti::has_type_X<T>::...
it would look like
BOOST_TTI_DECLARE_METAFUNCTION_FOR_HAS_TYPE(X) BOOST_TTI_HAS_TYPE(X,T)::...
which would have the advantage of hiding the exact name and location of the metafunction as an implementation detail.
To be clear, point #3 is just me pondering other possibilities to invite discussion, and should not be considered a necessary change that needs to be made.
I actually have a set of macros for generating the metafunction names. In the topic "General Functionality" look at the "Macro Metafunction Name Generation" section.
What is your evaluation of the implementation? ----------------------------------------------
I glanced at the code and it looked reasonable on the surface but I am not an expert in metaprogramming so I did not look at it closely.
What is your evaluation of the documentation? ---------------------------------------------
As others have pointed out, the documentation desperately needs more examples. In particular, unless I missed something I do not believe that there are any examples of actually using the library to accomplish something; all of the examples only show which macros should be used to create the metafunctions and what the resulting metafunction is called. In no example did I see an example where the metafunction was actually applied to accomplish something, such as disabling a function using SFINAE.
I suspect that the author assumed that such in-depth examples would not be necessary since anyone reading the documentation would probably be very experienced with metaprogramming and so would already know what to do once they had the introspection metafunctions available to them, but not everyone thinking about using this library will be so experienced at metaprogramming (myself being one member of this ignorant group) and so some examples of how the library could be used to accomplish goals would be incredibly useful to us so that we could get a better sense of exactly how this library works and how we can leverage it to solve practical problems.
I agree with you and intend to add some examples, and a section of the documentation which presents these examples inlined.
What is your evaluation of the potential usefulness of the library? -------------------------------------------------------------------
It seems like this library could be incredibly useful to those who need introspection functionality, but since I do not generally do the type of metaprogramming that this library facilitates this should be considered as just my speculation.
Did you try to use the library? With what compiler? Did you have any problem? -----------------------------------------------------------------------------
I played with it a little bit and compiled a random selection of the examples successfully.
I used gcc version 4.2.1 on OSX.
Cheers, Greg
Thanks very much for your review and suggestions. Eddie Diener
On 07/18/2011 05:41 PM, Edward Diener wrote:
2) I strongly think that "CREATE_METAFUNCTION_FOR" (or something similar) should be inserted into all of the macro names because otherwise they strike me as being misleading. If I were not familiar with this library and I saw the macro BOOST_TTI_HAS_TYPE(X) in someone else's code, I would be confused because the name of the macro makes it sound like it is asking if X has some type even though the result doesn't seem to be used. By contrast, if I saw the macro BOOST_TTI_CREATE_METAFUNCTION_FOR_HAS_TYPE(X), it would be immediately clear to me that the purpose of this macro is to create a metafunction.
I can do that but I wonder if programmers really want to type that much <g>. I do not think they do even though I fully agree with you that your macro names are much clearer than the shorter ones.
Elsewhere in the discussion it was proposed that "GENERATE" be used so that the macros would be called BOOST_TTI_GENERATE_HAS_TYPE etc, which is much less verbose than my original suggestion. I strongly support this option, and believe that is very little additional typing for a great deal of clarity. And I just want to emphasize that without this change (or something like it) the names of the macros are inherently confusing to anyone who hasn't grown use to them because they make the macros sound like they are doing one thing when they are actually doing another. I don't have a strong opinion about exactly how to fix this, but I do strongly believe that this is a problem that needs to be fixed.
I say this based in part on my own experience. When I was learning how this library worked I started by skimming through the documentation to get a feel for what was going on, and the macro names confused me because I couldn't tell what they were actually doing. I did eventually figure this out when I read through the documentation more closely, but if they had names following the convention I am recommending then I would have immediately understood what they were doing.
Although my proposed change would require additional typing (though unlikely not much, since I suspect most people are like me and just copy and paste from a template), I think that the benefit of improving the clarity would be well worth it.
It is a decent suggestion but I do explain after all that the macro metafunctions create metafunctions for introspecting an inner element with a given name.
Yes, but the less in-depth reading required to figure out something the better for the person who eventually has to use your library --- *especially* when they are in the position of unfamiliar reading code written by someone else and so want to have to read as little as possible from the documentation of the library to figure out what is going on. Cheers, Greg
On 7/18/2011 9:57 PM, Gregory Crosswhite wrote:
On 07/18/2011 05:41 PM, Edward Diener wrote:
2) I strongly think that "CREATE_METAFUNCTION_FOR" (or something similar) should be inserted into all of the macro names because otherwise they strike me as being misleading. If I were not familiar with this library and I saw the macro BOOST_TTI_HAS_TYPE(X) in someone else's code, I would be confused because the name of the macro makes it sound like it is asking if X has some type even though the result doesn't seem to be used. By contrast, if I saw the macro BOOST_TTI_CREATE_METAFUNCTION_FOR_HAS_TYPE(X), it would be immediately clear to me that the purpose of this macro is to create a metafunction.
I can do that but I wonder if programmers really want to type that much<g>. I do not think they do even though I fully agree with you that your macro names are much clearer than the shorter ones.
Elsewhere in the discussion it was proposed that "GENERATE" be used so that the macros would be called BOOST_TTI_GENERATE_HAS_TYPE etc, which is much less verbose than my original suggestion. I strongly support this option, and believe that is very little additional typing for a great deal of clarity.
I will consider your suggestion. Thanks ! Eddie
participants (3)
-
Edward Diener -
Gregory Crosswhite -
Jeffrey Lee Hellrung, Jr.