Re: [boost] Formal Review: Boost Cast Library

From: Ed Brey (brey_at_[hidden])
Date: 2000-12-07 10:13:41

From: "David Abrahams" <abrahams_at_[hidden]>
> >
> > polymorphic_cast and polymorphic_downcast are rather trivial and
seem to
> > mostly be rarely useful so I'm inclined to say that they should
not be
> part
> > of boost.
>
> I hope we don't make that decision, as it would require me and my
colleagues
> to change quite a lot of working code which depends on
polymorphic_downcast.
> I hope that is sufficient to speak for the utility of these
functions. As to
> triviality, the same could be said for std::pair<>. Sometimes a
trivial
> construct has great advantages in expressive power.

I agree with Dave that triviality is not a big concern: both
"polymorphic_*cast"s bundle two or more statements into a single
function call, saving the user 50% or more coding versus doing the
work by hand.

I do question whether there is a significant overlap between
polymorphic_cast and dynamic_cast, such that std and boost taken
together would no longer qualify for the "minimal" part of minimal and
complete.  I also am concerned that the polymorphic_cast could
discourage maximally type-safe code.

Specifically, I am concerned about there being two very similar ways
to doing the same thing:

void f(Base* b) {
  Derived* d1 = &dynamic_cast<Derived&>(*b);
  Derived* d2 = polymorphic_cast<Derived*>(b);
}

The only difference between polymorphic_cast and dynamic_cast with a
reference is that polymorphic_cast handles null pointers.  But do we
really want that?  The purpose of the cast is to convert between
objects in a hierarchy.  Having no object at all is a different
situation, which the program should handle separately, if allowed by
the function's preconditions.  Often it is not allowed, and so code
like this is more appropriate:

void f(Base& b) {
  Derived& d1 = dynamic_cast<Derived&>(b);
}

References are an excellent part of C++'s strong typing, a important
advantage over languages like Java and C#, and so I am hesitant about
including a feature that may discourage its use.

IMHO, the final call should be based on how common code like:
  Derived* d1 = &dynamic_cast<Derived&>(*b);
legitimately exists; that is, where you really need a pointer on both
sides of the cast.  If this is a common use case, then the argument
can be made that polymorphic_cast is a tool that provides a useful
service, and should be included, and the fact that it can be misused
shouldn't condemn it.

Polymorphic_downcast has no substitute and should be included.  I
would like to see it be able to work with references, even if only on
platforms that support partial specialization, if necessary.

Aside from those issues, I have a few other review comments:

1. Is there a performance impact by using strstream instead of
stringstream for lexical_cast?  Presumably, a library implementer will
put more optimization effort into a current class than a depreciated
one.  I'd hate to see performance drop for conforming platforms
because of a code change to appease a nonconforming one.  If this is
the case, I'd say conditional compilation is in order.

2. The classes bad_*_cast, don't have explicitly defined destructors.
Do they need them so that they can specify empty throw specifications,
so as to fit within the constraints of their base class?

3. type_wrapper should be declared only for VC.

4. In cast.htm, there is an extra period before the ODR warning.

In summary, I think that the cast library should be accepted into
boost.