On Thu, Aug 04, 2016 at 07:01:45PM +0100, Jonathan Wakely wrote:
> 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).
>
Leave it as is in the next submission? I am not too familiar with
Doxygen.
>
> > @@ -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.
>
static_cast should work just fine.
> 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.
>
>
--
Gleb.
