Re: [PATCH] do not throw in std::make_exception_ptr

[Jonathan Wakely <jwakely at redhat dot com>]

On 04/08/16 20:01 +0300, Gleb Natapov wrote:
> Instead of throwing an exception allocate its memory and initialize it
> explicitly. Makes std::make_exception_ptr more efficient since no stack
> unwinding is needed.
>
> In this version I hopefully addressed all Jonathan comments.
>
> * libsupc++/exception (std::exception): Move...
> * libsupc++/exception.h: ...here; New.
> * libsupc++/cxxabi.h (__cxa_allocate_exception): Move...
> * libsupc++/cxxabi_init_exception.h: ...here and add
> __cxa_init_primary_exception; New.
> * config/abi/pre/gnu-versioned-namespace.ver: add
> __cxa_init_primary_exception and std::exception_ptr(void*)
> * config/abi/pre/gnu.ver (CXXABI_1.3.11) : add
> __cxa_init_primary_exception and std::exception_ptr(void*)
> (CXXABI_1.3.11): New.
> * include/Makefile.am: add exception.h and cxxabi_init_exception.h
> * include/Makefile.in: Likewise.
> * libsupc++/Makefile.am: add exception.h and cxxabi_init_exception.h
> * libsupc++/Makefile.in: Likewise.
> * libsupc++/eh_throw.cc(__cxa_throw): add __cxa_init_primary_exception
> and use it
> * libsupc++/exception_ptr.h(std::make_exception_ptr): use
> __cxa_allocate_exception and __cxa_init_primary_exception to create
> exception pointer
> * libsupc++/typeinfo: include bits/exception.h instead of exception

This version is *much* easier to review :-)

> +namespace __cxxabiv1
> +{
> +  struct __cxa_refcounted_exception;
> +
> +  extern "C"
> +    {
> +      // Allocate memory for the primary exception plus the thrown object.
> +      void*
> +        __cxa_allocate_exception(size_t) _GLIBCXX_NOTHROW;

Please align __cxa_allocate_exception with the return type on the
previous line.

> +
> +      // Initialize exception

Please add "(this is a GNU extension)" to the comment.

> +      __cxa_refcounted_exception*
> +      __cxa_init_primary_exception(void *object, std::type_info *tinfo,
> +                void (_GLIBCXX_CDTOR_CALLABI *dest) (void *)) _GLIBCXX_NOTHROW;
> +
> +    }
> +} // namespace __cxxabiv1
> +
> +#endif
> +
> +#pragma GCC visibility pop
> +
> +#endif // _CXXABI_INIT_EXCEPTION_H
> index 63631f6..8be903b 100644
> --- a/libstdc++-v3/libsupc++/exception
> +++ b/libstdc++-v3/libsupc++/exception
> @@ -36,39 +36,12 @@
>
> #include <bits/c++config.h>
> #include <bits/atomic_lockfree_defines.h>
> +#include <bits/exception.h>
>
> extern "C++" {
>
> namespace std
> {
> -  /**
> -   * @defgroup exceptions Exceptions
> -   * @ingroup diagnostics
> -   *
> -   * Classes and functions for reporting errors via exception classes.
> -   * @{
> -   */

The Doxygen group that starts with @{ ends later in this file, but you
haven't moved the corresponding @} to the new file.

I can take care of fixing that up though (we want the contents of the
new file and this one to all be in the group).


> @@ -162,8 +169,12 @@ namespace std
>     swap(exception_ptr& __lhs, exception_ptr& __rhs)
>     { __lhs.swap(__rhs); }
>
> -  } // namespace __exception_ptr
> +    template<typename _Ex>
> +      static inline void
> +      __dest_thunk(void* x)
> +      { reinterpret_cast<_Ex*>(x)->~_Ex(); }

This is still 'static' so we get a separate instantiation in every
translation unit that uses make_exception_ptr. It should non 'inline'
but not 'static'.

Looking at it again now, is there any reason to use reinterpret_cast
rather than static_cast? I think they're identical in this context,
and I prefer not to only use reinterpret_cast as a last resort.

Other than that, I think this is good to commit.

I can't think of any way to test the changes beyond the existing tests
for exception_ptr, so we don't need new test files.