On 15 Nov 2013 at 19:45, Antony Polukhin wrote:
...and can be arbitrarily broken by (unknown) future optimizations, with no recourse. We wouldn't have all the problems with strict aliasing that we do today if programmers 10 years ago hadn't made exactly the same argument that you are, now. (Sure, it's undefined behavior, but we know what the compiler actually does, so it's really okay...).
You're absolutely right. I've liked the idea of "make it in a way that user won't notice a difference but typeid issues will be solved" so much, that turned a blind eye to a hack with UB.
This will be fixed (possibly code from v1.0 will be taken, which had no boost::type_info and contained type_index class that had a pointer to std::type_info).
Do you have other ideas of what can be fixed/improved?
Before you make any substantial changes here Antony, I have been busy pondering on this UB issue and on what to recommend to the review wizards. Here are the facts: 1. The problem code is like this: static_cast<const boost::type_info&>(typeid(no_cvr_t)); ... where typeid() returns a std::type_info and where boost::type_info singly inherits publicly from std::type_info and no other type. 2. Note that std::type_info contains a virtual function table on libstdc++, Dinkumware and probably any reasonable STL implementation. Therefore boost::type_info will also contain a virtual function table. 3. The problematic static cast is an upcast to a derived type, not a cast between unrelated types. No multiple or virtual inheritance is in play. 4. Which would be legal if and only if typeid originally returned a boost::type_info, because you can legally cast to a base type and back to a derived type. 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. 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. Niall -- Currently unemployed and looking for work. Work Portfolio: http://careers.stackoverflow.com/nialldouglas/