[boost] [Review] Function types library

From: Doug Gregor (dgregor_at_[hidden])
Date: 2006-11-09 12:01:11

My review of the function types library follows.

I've only been able to spend an hour or so reviewing this library,  
but I've read through all of the documentation, browsed the code and  
examples, and run the testsuite.

I've written and maintained several libraries that deal with function  
types, and we end up reinventing the functionality in this library  
over and over. We definitely need these facilities in Boost, if only  
to have all of the dumb compiler workarounds in a single place. I  
have a few comments, in no particular order:

        - This library should be merged into the type traits library, both  
the code (headers go in boost/type_traits) and documentation (there  
are already some function-type facilities there that should probably  
be replaced). The facilities in this library are type traits, and the  
library itself is small enough that it does not need to stand alone.

        - I'm not a huge fan of the tag types facilities. Part of the  
problem with them is that there are so many new names (11), many of  
which are very close to existing type traits: there's a tag  
const_qualified, for instance, which basically means is_const on the  
underlying function type. At the very least, these tags will need to  
go in a sub-namespace of "boost" ("boost::type_traits" is fine). At  
best, a crazy idea: is there a way to re-use existing type traits  
like "is_const" instead of introducing these new names? Or, perhaps,  
the "non_" versions could be mpl::not_, and "tag" could be  
"mpl::and_". Interface reuse is really important!

        - I'm rather concerned about the portability of this library. Andy  
Little has reported some failures on MSVC 7.1, and I'm seeing a huge  
number of failures on Apple GCC 4.0.1. Function types will be a core  
part of the type traits library: they absolutely must be portable.

        - If the library is accepted, I hope the author will help us clean  
up other facilities in Boost (say, result_of <g>) to use this library  
rather than their own hacks. Part of introducing infrastructure into  
Boost is helping to make sure that infrastructure actually replaces  
what it's meant to replace.

At this point, I don't know whether to accept or reject. The  
portability problem can be solved, of course, and it will be solved  
with time. The issue of tag types really bothers me, because the  
interface is so large and seems so redundant with what we already  
have in type traits. If tag types could be simplified, my vote would  
be an enthusiastic "accept." Right now... I'm on the fence.

        Cheers,
        Doug