[boost] thoughts about casts.hpp (a bit long)

From: Darin Adler (darin_at_[hidden])
Date: 1999-09-17 22:12:52

First, a few thoughts about the polymorphic_downcast template in the 
casts.hpp file. I have considerable experience with this template, although
I have always called my version called checked_static_cast (not as good a
name). I think that including the word polymorphic in the name is superior.
I also had polymorphic_cast, but I called it checked_dynamic_cast and had it
assert rather than throwing an exception.

----------------

The name polymorphic_downcast is not so great because the cast is useful for
upcasts as well as as downcasts. You just can't use it for a crosscast.

The current boost implementation checks only that dynamic_cast returns
something other than 0. In my implementation, I've always checked that the
static_cast and dynamic_cast return the same result. There are some cases
with multiple inheritance where dynamic_cast and static_cast will both work,
but static_cast would get a different result.

Here's my alternate implementation, with a new suggested name. I call the
parameters CastTo and CastFrom to emphasize that it's useful for upcasts as
well as downcasts.

    template <class CastTo, class CastFrom>
    inline CastTo polymorphic_static_cast(CastFrom* from)
    {
        CastTo cast_result(static_cast<CastTo>(from));
        assert(dynamic_cast<CastTo>(from) == cast_result);
        return cast_result;
    }

Here's an example of what's less powerful about the current implementation.
An error that my suggested implementation catches that the current
implementation misses.

    struct B {
        virtual ~B() { }
    };

    struct D1 : B { };
    struct D2 : B { };
    struct D3 : D1, D2 { };

    int main()
    {
        D3 od3;
        D1* pd1(&od3);
        B* pb(pd1);

        // The following succeeds, because the dynamic_cast does
        // a successful cross-cast, even though the static_cast gets
        // it wrong. You don't get an assert.
        polymorphic_downcast<D2*>(pb);

        // The following fails, because it notices that dynamic_cast
        // has returned something different from what static_casts
        // returned. It's good that it fails because the result
        // returned above from polymorphic_downcast is bad.
        polymorphic_static_cast<D2*>(pb);
    }

Another difference about the proposed polymorphic_static_cast is that it
works fine with 0; 0 in, 0 out. A variant version could be written that also
checks for 0 as well (and either asserts or throws bad_cast).

----------------

It seems that a version of polymorphic_cast that uses an assert might be
useful for some people. Someone who uses polymorphic_downcast /
polymorphic_static_cast in some parts of their program might want a
consistent philosophy of compiling out runtime checks that are known to
never fail, due to the structure of the program. Someone who knows that a
dynamic cast could fail and prefers to use a bad_cast exception rather than
a check for 0 for that case is different from someone who is certain that
the cast won't fail, but wants a runtime check in debug versions to enhance
testing.

Similarly, some might prefer a version of numeric_cast that uses an assert
rather than an exception.

Perhaps there could be two different versions of the header for the two
different styles of programming, one that throws exceptions and another that
uses asserts instead.

----------------

It would be nice to factor out the part of numeric_cast that checks if
something is in range, like this (there may be some typos):

    template<typename Target, typename Source>
    inline bool number_in_range(Source arg)
    {
        // typedefs abbreviating respective trait classes
        typedef std::numeric_limits<Source> arg_traits;
        typedef std::numeric_limits<Target> result_traits;

        // typedefs that act as compile time assertions
        // (to be replaced by boost compile time assertions
        // as and when they become available and are stable)
        typedef bool argument_must_be_numeric[arg_traits::is_specialized];
        typedef bool result_must_be_numeric[result_traits::is_specialized];

        return (result_traits::is_signed || arg >= 0) &&
                // loss of negative range
            (!arg_traits::is_signed ||
                 arg >= result_traits::min()) &&    // underflow
            arg <= result_traits::max();           // overflow
    }

    template<typename Target, typename Source>
    inline Target numeric_cast(Source arg)
    {
        if (!number_in_range<Target>(arg))
            throw bad_numeric_cast();
        return static_cast<Target>(arg);
    }

The number_in_range template can be useful for asserts and perhaps for code
that's deciding which algorithm to use based on whether a number fits in a
particular type. It can also be used to build a version of numeric_cast that
asserts instead of throws.

----------------

I don't like having an implicit_cast that disables the warnings that some
compilers issue for implicit numeric conversions. When I use implicit_cast,
I'm trying to get the implicit conversion that would not cause warnings to
happen. I don't want to disable the warnings. To me, that's what the
"implicit" in implicit_cast means -- just do the same thing you'd do if I
was assigning to the type I mention.

To eliminate the implicit numeric conversion warnings, I would use a version
of numeric_cast that used asserts instead. In the past I've used a cast
called narrow_cast and another called unsigned_cast that narrow things and
convert from signed to unsigned without warning. I use static_cast in the
implementation to make sure they work as intended. For an implementation, I
use a specialization for each legal pair of numeric types like this:

    template <class T, class U> inline T narrow_cast(U);
    template <class T, class U> inline T unsigned_cast(U);

    template <> inline long narrow_cast(unsigned long n)
        { return static_cast<long>(n); }
    // ... and more specializations for combinations of built-in types

    template <> inline unsigned long unsigned_cast(long n)
        { return static_cast<unsigned long>(n); }
    // ... and more specializations for combinations of built-in types

If runtime tests are desired, a version of these casts could be made that
uses the number_in_range template above:

    template <> inline long narrow_cast(unsigned long n)
    {
        assert(number_in_range<long>(n));
        return static_cast<long>(n);
    }

I'd prefer to have at least one cast that won't disable any warnings, but is
there to reduce the need to temporary variables. A separate cast can be used
to disable implicit numeric conversion warnings.

This has the side benefit of completely eliminating the "Implementation
details we wish were hidden" section of the current cast.hpp.

----------------

Thanks for reading this long message.

Thoughts?

    -- Darin