Please let me know as soon as possible any errata, missing credits etc. Niall Boost Peer Review Report for proposed Boost.TypeIndex v2.1 Nov 12th – 21st 2013 This report is the first edition, dated Sunday 24th November 2013. The review manager was Niall Douglas http://www.nedprod.com/. Source: https://github.com/apolukhin/type_index/zipball/master Documentation: http://apolukhin.github.com/type_index/index.html Peer review discussion detail: http://boost.2283326.n4.nabble.com/TypeIndex-Peer-review-period-for-library-... 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. Contents: · 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. [I agree] · 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: Text Box: /*! Returns a static const char string of unknown format uniquelyidentifying this type.The only guarantee is that this string will beunique to the type within this process lifetime. */const char *unique_name() const noexcept;/*! Returns a representation of this type suitable for printing.This call may take some time as its storage may not be cached. */std::string pretty_name() const;class enum mangling{Native, //!< Whatever the native mangling used by this toolset isMSVC, //!< The Microsoft C++ mangling formatItanium //!< The Itanium C++ mangling format};/*! Returns the mangled form of the string representation of the type.After the calculation the value is cached statically such that the c_str()function can be used to convert the returned string to a const char *format identical to what may be returned by std::type_info::name() (orraw_name() on MSVC).This function may throw an exception if it does notsupport mangled type string calculation, including when mangling=Native onsome 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. Niall Douglas Waterloo, Canada, November 2013
