Re: [boost] [TypeIndex] Peer review period for library acceptance begins, ending Thurs 21st Nov

2013/11/14 Andrey Semashev

On Thu, Nov 14, 2013 at 9:26 AM, Antony Polukhin wrote:

...

2013/11/14 Steven Watanabe

...

AMDG

On 11/12/2013 11:34 AM, Niall Douglas wrote:

...

Boost community feedback is requested for the formal peer review of the TypeIndex library by Antony Polukhin. Feedback requested includes:

1. Should this library be accepted into Boost?

No. This library should not be accepted into Boost in its current form.

type_info.hpp:139: return static_cast(typeid(type));

This is undefined behavior, and there is no way to make it correct.

This is a necessary evil and it is harmless: * all the methods of std::type_info (except destructor) are non virtual, so for example calls to name() will call boost::type_info::name() * it is *undefined* which destructor (std::type_info or boost::type_info) will be called *but*: * boost::type_info contains no members, it's size must be equal to the size of std::type_info, so no harm if boost::type_info destructor won't be called * destructor of std::type_info will be called anyway

I can add assert to tests to ensure that sizeof(std::type_info) == sizeof(boost::type_info).

I think the undefined behavior can be removed with the following trick:

1. boost::type_info should not derive from std::type_info (it would have no base classes or data members). Its destructor should be deleted, as well as its constructors and assignment. 2. The above cast should be replaced with reinterpret_cast:

return reinterpret_cast(typeid(type));

3. All methods of boost::type_info should perform the reverse reinterpret_cast of *this before accessing std::type_info.

The standard warrants that such two-way casting would yield the original reference to std::type_info.

However, this turns out to be quite a lot of hackery and makes me wonder if boost::type_info is needed at all. Why not just have boost::type_index that will work directly with std::type_info?

Unfortunately, boost::type_info must derive from std::type_info for compatibility reasons. Without it the following code won't compile: std::type_index ti = boost:type_id<int>(); This may look like a rare case, however will be quite common if other Boost libraries adopt the TypeIndex. Is my favorite example with Boost.Any: boost::any a = 10; // Imagine that Boost.Any uses boost:;type_info instead of std::type_info class_that_constructs_from_std_type_info = a.type(); // operator const std::type_info&() won't help std::type_index ti = a.type(); In v1.0 of TypeIndex library there was no boost::type_info, there was only boost::type_index. version 2 was started because v1.0 was hard to use in existing Boost libraries that that return std::type_info to the user. Version 1.0 is not a drop-in replacement. It can be used only if we have full control over the code that uses it. Why there is a such need in drop-in replacement? Here are the reasons: * some compilers fail to compare std::type_info across modules (surprise!) * some libraries fail to write a sane hashing function for std::type_info * users want to use Boost without RTTI and fail: http://stackoverflow.com/questions/14557313/is-it-possible-to-use-boost-prog... Here are some more issues in Boost: * there is a mess with stripping/not stripping const, volatile, reference * there is a mess with name() calls, plenty of places where name() used for debug/user output without demangling * some of the Boost libraries demangle names by themselfs and forget to dealocate memory (at least valgrind says so) In three words: std::type_info is broken. There is a need to give users an ability to write portable code, but do not break existing users code (that do fail on some compilers). There is a need to put all the std::type_info workarounds all around the Boost in s single place, which will be easy to maintain. It is not good to see in different libraries code like that: // Borrowed from Boost.Python library: determines the cases where we // need to use std::type_info::name to compare instead of operator==. #if defined( BOOST_NO_TYPEID ) # define BOOST_FUNCTION_COMPARE_TYPE_ID(X,Y) ((X)==(Y)) #elif (defined(__GNUC__) && __GNUC__ >= 3) \ || defined(_AIX) \ || ( defined(__sgi) && defined(__host_mips)) # include <cstring> # define BOOST_FUNCTION_COMPARE_TYPE_ID(X,Y) \ (std::strcmp((X).name(),(Y).name()) == 0) # else # define BOOST_FUNCTION_COMPARE_TYPE_ID(X,Y) ((X)==(Y)) #endif Or like this: // See boost/python/type_id.hpp // TODO: add BOOST_TYPEID_COMPARE_BY_NAME to config.hpp # if (defined(__GNUC__) && __GNUC__ >= 3) \ || defined(_AIX) \ || ( defined(__sgi) && defined(__host_mips)) \ || (defined(__hpux) && defined(__HP_aCC)) \ || (defined(linux) && defined(__INTEL_COMPILER) && defined(__ICC)) # define BOOST_AUX_ANY_TYPE_ID_NAME #include <cstring> # endif Did you note that macro *differ*? And by the way, in both cases it is not optimal! -- Best regards, Antony Polukhin