[cfe-commits] [libcxx] r104850 - in /libcxx/trunk: include/ src/ test/language.support/support.exception/except.nested/

[Howard Hinnant]

Thanks for the review Sebastian!

On May 28, 2010, at 5:09 AM, Sebastian Redl wrote:

> 
> On Thu, 27 May 2010 17:06:52 -0000, Howard Hinnant <hhinnant at apple.com>
> wrote:
>> Author: hhinnant
>> Date: Thu May 27 12:06:52 2010
>> New Revision: 104850
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=104850&view=rev
>> Log:
>> [except.nested]
>> 
>> +template <class _E>
>> +inline
>> +void
>> +rethrow_if_nested(const _E& __e, typename enable_if<
>> +                                   !is_same<_E,
> nested_exception>::value
>> &&
>> +                                   is_convertible<_E*,
>> nested_exception*>::value
>> +                                                   >::type* = 0)
>> +{
>> +    static_cast<const nested_exception&>(__e).rethrow_nested();
>> +}
>> +
>> +template <class _E>
>> +inline
>> +void
>> +rethrow_if_nested(const _E& __e, typename enable_if<
>> +                                   is_same<_E, nested_exception>::value
> ||
>> +                                   !is_convertible<_E*,
>> nested_exception*>::value
>> +                                                   >::type* = 0)
>> +{
>> +}
> 
> You don't seem to have any tests for this function.

It is here:

http://llvm.org/svn/llvm-project/libcxx/trunk/test/language.support/support.exception/except.nested/rethrow_if_nested.pass.cpp

> Anyway, unless I'm
> missing something major, this is definitely wrong as implemented. Not only
> does it only do a static switch on the type where the standard requires a
> dynamic switch, the condition of the static switch is strange, too. Why is
> the is_same case handled in the second function? It should be in the first.
> 
> But anyway, it doesn't work this way. What you need is:
> 1) One function to handle polymorphic types. In this one, you do a
> dynamic_cast to nested_exception const* and call rethrow_nested() if it
> succeeds. If nested_exception is in the direct inheritance path of _E, the
> dynamic_cast is performed at compile time, so you don't have any efficiency
> issues there (as if it matters here ...).
> 2) One function to handle non-polymorphic types. This needs to be a
> separate function to avoid compile errors when applying dynamic_cast to
> these types. Since nested_exception is polymorphic, any type that derives
> from it is polymorphic too, so this function won't apply. If _E is not
> directly derived from nested_exception and not polymorphic, then you
> couldn't cross-cast to a nested_exception anyway, even if there is one in
> the complete object. So this function is a no-op.

I missed the single word "dynamic" in [except.nested]/8.  Take that word out and my implementation was fine. ;-)  Thanks for catching my mistake.

-Howard