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
(typeid(no_cvr_t)); where this line does not fail:
static_cast
(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.