Re: [boost] [typeindex v3.0] Peer review begins Mon 21st ends Wed 30th

On Wednesday 30 April 2014 19:40:54 Antony Polukhin wrote:

2014-04-30 18:56 GMT+04:00 Andrey Semashev :

...

The ABI incompatibilities between release and debug (and different flavors of debug in case of MSVC) libraries have caused a lot of grief already, even though Boost.Build tags debug and release binaries differently. Adding yet another source of incompatibilities certainly doesn't make things better.

Boost.Log has a compiled part. In some cases it needs to pass RTTI between the compiled part an the user's application. If the application defines BOOST_TYPE_INDEX_USER_TYPEINDEX then Boost.Log becomes broken. Hoping that the user never does that is not what I'm prepared to do (because he will, and will report a bug about weird crashes in Boost.Log). That is why I won't be able to use your library in Boost.Log.

I see your point. Let's try to resolve this somehow without throwing away the BOOST_TYPE_INDEX_USER_TYPEINDEX macro.

Is in your case type_index one of the function arguments? If yes - then user won't be able to link modules with and without BOOST_TYPE_INDEX_USER_TYPEINDEX. Linker won't find functions.

Currently, in some cases std::type_info is the argument. If type_index is the argument in those cases, linker errors will protect from crashes in run time. But these functions are not always used by the application, so it's not a 100% protection. In other cases std::type_info const* is stored in structures and containers thereof, and do not affect mangled type and function names. I'll have to inspect the code to see if Boost.Log actually has these cases but it certainly does not prohibit them; they may appear in user's code as well. I know there are workarounds for this. Like add yet another tag to Boost.Log internal namespace. (BTW, using a function is not really a good solution because this function needs to be called in many places of the code; it reduces efficiency, introduces false dependency on the binary, and is easy to miss). But these are workarounds that are not needed now and required to support a controversial feature. With Boost.Log's internal namespace the workaround may seem easy to implement, but other libraries don't have such a namespace, and there the fix would not be so easy.

...

...

...

I think it is safe to assume that the compiler supports template specialization (at least, full specialization). Otherwise

Boost.TypeTraits

...

...

you already use (as well as pretty much all Boost) wouldn't work. In any

case,

...

...

the current implementation is not correct.

The only problem is that I'm not sure that such compiler will distinguish signed int from int. We have no regression testers that can check this

out.

Then perhaps you shouldn't bother with that check in the first place. Add a workaround when someone with such a compiler appears.

In both cases we get code that does not work on ancient EDG like compilers. But it is better to notify user that library does not work at compile time. Otherwise user will get a hard to debug runtime error.

Information about that issue is taken from here: https://github.com/boostorg/python/blob/master/include/boost/python/type_id. hpp Looks like it is not only ints related. If I change the static assert message to more generic will it be OK for you?

In that header, I can see the code that works around the problem by using template specializations. As I understand, the compiler matches specializations for int when the template is instantiated for signed int. Why not make the same workaround in Boost.TypeIndex? The case is very much the same.