Please let me know as soon as possible any errata, missing credits etc.
This report is the first edition, dated Sunday 24th November 2013. The review manager was Niall Douglas http://www.nedprod.com/.
My thanks to the Boost members whose peer review makes up this report: Robert Ramey, Vicente J. Botet Escriba, Gavin Lambert, Andrey Semashev, Steven Watanabe, pfultz2, Mathieu Champlon, Joël Lamotte Klaim, Rob Stewart, Jan Herrmann, Tony Van Eerd.
· Summary of consensus from peer review ......................................................................................................... 1
· My recommendation to the library author and the Review Wizards ................................................................ 3
Summary of consensus from peer review:
The consensus feedback was to accept TypeIndex after substantial modifications. There were two recommendations of rejection due to how substantial such modifications would need to be (i.e. a request for a second round of peer review with a new implementation).
Note that my personal opinion based on post-review reflection is written in [square bracketed italics].
· Many seemed to feel that a boost::type_info’s member function API, where identically named to that of std::type_info member function API, ought to have identical compile-time effects. This implied that missing member function APIs, or other compile time errors for missing or incomplete functionality, would be okay but if name() returns some const char *, then any const char * so long as it meets the C++ standard is okay.
[I think there are three use cases for a type_info like substitute: (i) as a lightest possible weight, non-RTTI requiring minimal type_info which can have any API it likes and ought to not be directly substitutable for std::type_info or std::type_index (ii) as a compile-time substitute for std::type_info in that it will compile without third party code modification, but may not produce reliable code due to not returning exactly the same values as std::type_info (iii) as a runtime substitute for std::type_info in that it will require code to be “ported” to it due to having a breaking API, but thereafter produces reliable code. I’ll be very blunt in saying that (ii) is a bad idea in my opinion, and (iii) is valuable but out of scope enhancement which could be added as a separate class after peer review. In my opinion, (i) is where we ought to focus, and I would recommend that the lightest possible weight type_info replacement be deliberately compile-time incompatible with std::type_info (where output is not identical to std::type_info) to force porting code to it so authors are not lazy.]
· A large minority seemed to feel that a boost::type_info’s member function API, where identically named to that of std::type_info member function API, ought to have identical run-time outcomes. Most specifically, this would mean that member functions e.g. name() would return exactly what std::type_info’s name() does, even if for example name() returns a std::string instead of a const char * as would be necessary in pre-C++11 compilers.
[This is really use case (iii) in the previous item.]
· It was mentioned that it should be possible to optionally specialise std::hash<> for type_info for optional seamless use in hash taking containers.
· Some felt that implementation-specific std::type_info quirks ought to be replicated. Others felt they should not, and a portable interface which works identically on all platforms is preferred.
[This is really use case (iii) in the previous item.]
· boost::type_info is currently initialised via a static cast from a std::type_info when RTTI is available. This is undefined behaviour, and some felt this to be a showstopper. I (Niall Douglas) looked into this more deeply due to its seriousness, and found that because most STL implementations define std::type_info with a virtual function table, the undefined behaviour static cast would cause dynamic_cast on a boost::type_info instance to misoperate.
[I believe the RTTI induced misoperation to indeed be a showstopper, and that this use of undefined behaviour to reduce code bloat is an unsafe optimisation. A much lighter weight non-direct-substitute for std::type_info ought to be as code bloat parsimonious as is possible, while its lack of non-explicit interoperation ability with std::type_info ought to allow avoidance of needing any UB tricks.]
· Some felt that any use of implied boost::type_info ought to be explicitly written as boost::type_info instead of relying on namespace lookup, as the name similarity to std::type_info may introduce confusion.
· It was mentioned that some function-local static data members are initialised in a thread unsafe way.
[For information I have to hand a very lightweight BOOST_BEGIN_MEMORY_TRANSACTION() implementation ideal for this purpose. We could submit it to Boost.Detail and then everyone could use it?]
· Magic macros such as __PRETTY_FUNCTION__ ought to not be referenced outside of a function as they don’t technically exist there. Checks for their existence ought to be within a function.
· It was requested that a boost::type_id<> which takes some unknown variable instance as a parameter and therefore allows the compiler to deduce the type of the variable as the boost::type_index<T> type would be valuable.
· A mechanism for programming compile-time logic with class inheritance trees such that inheritance can be determined at compile-time with RTTI disabled was requested.
My recommendation to the library author and the Review Wizards:
· In my opinion Antony ought to make TypeIndex v3 quite literally a very lightweight container of some unknown, but known to be uniquely identifying for some type, static const char * string. I think its class type and its list of member functions ought to be deliberately compile-time incompatible with std::type_info to force authors to upgrade their code. A conversion member function ought to be able to synthesise a corresponding std::type_info using typeid() from some boost::type_index<T>, but that’s about it. I would even, personally speaking, go so far as to only provide a boost::type_index and no corresponding boost::type_info, especially if the boost::type_id<T>() function can return a const boost::type_index<T>& and therefore can be used as a static const lref, or copy constructed from it etc.
A suggested name() member function replacement which correctly breaks out the multiple confounding uses of std::type_info::name() into each of their three use cases (and which intentionally causes any use of name() to fail to compile) might be:
Bear in mind that user code can always subclass boost::type_index and add their own name() implementation based on one or more of the above new member functions.
· In other words, here I think “less is more” in every way you look at it. I think a slimmer less heavy TypeIndex would be a superior solution.
· I request the Review Wizards to allow Antony some time to make the changes to produce a v3 of the library, and then to allow TypeIndex v3 a Fast Track Review of five days to ensure the Boost community is happy and that its feedback has been incorporated. It was asked during peer review that such a five day period please include a weekend. I would be happy to serve as review manager again if requested.
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost-announce
|Free forum by Nabble||Edit this page|