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

Subject: Re: [boost] [typeindex v3.0] Peer review begins Mon 21st ends Wed 30th
From: Andrey Semashev (andrey.semashev_at_[hidden])
Date: 2014-05-01 05:07:03

On Wednesday 30 April 2014 19:40:54 Antony Polukhin wrote:
> 2014-04-30 18:56 GMT+04:00 Andrey Semashev <andrey.semashev_at_[hidden]>:
> > 
> > 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.