|
Boost : |
Subject: Re: [boost] [TypeIndex] Peer review period for library acceptance begins, ending Thurs 21st Nov
From: Niall Douglas (s_sourceforge_at_[hidden])
Date: 2013-11-15 16:09:38
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/
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk