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

Subject: Re: [boost] [TypeIndex] Peer review period for library acceptance begins, ending Thurs 21st Nov
From: Andrey Semashev (andrey.semashev_at_[hidden])
Date: 2013-11-15 18:20:06

On Friday 15 November 2013 16:09:38 Niall Douglas wrote:
> 
> Here are my thoughts:
> 
> 1. I would be happy with the static upcast if no virtual function
> table were in play because it's single inheritance, and static_cast
> provides enough safeguards (e.g. erroring if the type's inheritance
> tree is unknown at the point of static_cast use). It's a very common
> pattern in C++ code and is used throughout (especially older parts
> of) Boost. It is sufficiently common that no future compiler could
> ever disable it or misoperate with it. I agree that could hinder
> significantly future C++ features, but that is not TypeIndex's
> problem because of so much preexisting usage. It's an established
> idiom, and that's that: in my opinion TypeIndex has the right to use
> established idioms if they are currently safe with all possible
> present compiler technologies as implied by the present C++ standard.

I disagree. Virtual functions or type inheritance are not relevant. The 
standard is quite clear on this case (see 5.2.9/2), and compilers are allowed 
to mess up that code. The actually stored object is different from what the 
reference is being cast to. If a compiler, say, has to adjust the pointer as 
the part of static_cast, the resulting pointer would be invalid and any access 
to it would be another UB. I know you rely on the fact of single inheritance 
and assume that the pointer doesn't need any adjustment. But that is not 
guaranteed by the standard, and such language implementation would be valid.

The preexisting practice is not an argument to me because that's not the kind 
of practice we'd like to preserve or advertise. That practice does cause a lot 
of headache from time to time. Gladly, such hacks are not that common nowdays.

> 2. Unfortunately, there IS a virtual function table, and therefore
> this line would fail:
> 
> dynamic_cast<const boost::type_info&>(typeid(no_cvr_t));
> 
> where this line does not fail:
> 
> static_cast<const boost::type_info&>(typeid(no_cvr_t));
> 
> That seems to me to be unacceptable. If a virtual function table is
> present, it MUST be correct with respect to the type the compiler is
> using it as.
> 
> 
> So, Antony, if you can get the virtual function table to correctly
> say it's a boost::type_info and not a std::type_info, I'll recommend
> in my report this is fine. I don't mind a certain amount of evil :)
> to achieve that given it's such an isolated case which can be easily
> wrapped in a single location.

Ignoring the problem is not acceptable for a new library, IMO. The fact that 
some select compilers currently bear with such code doesn't mean they won't in 
the future, let alone there may be compilers that break it already. Putting 
that kind of jeopardy in the base of the library design is not a good solution 
by any measure. More so, when a cleaner solution is available.