Temporaries cause broken code for std::make_unique<>(), any hints?

[Yury V. Zaytsev]

Hi,

I've implemented a wrapper for std::make_unique<>() as follows:

from libcpp.memory cimport unique_ptr

cdef extern from "<memory>" namespace "std" nogil:
unique_ptr[T] make_unique[T]() except +
unique_ptr[T] make_unique[T](T&) except +

Very unfortunately, this forces Cython to generate a temporary for
handling the exceptions, which, obviously breaks the whole thing:

try {
__pyx_t_10 = std::make_unique<XXX>(__pyx_v_uv);
} catch(...) {
__Pyx_CppExn2PyErr(); /* ... */
}
__pyx_v_pyuv->thisptr = __pyx_t_10;

When I remove "except +", then the generated code is correct, but, of
course, possible exceptions (std::bad_alloc or any exception thrown by the
constructor of T) are not handled.

Is it somehow possible to force Cython to generate a move, as in

__pyx_v_pyuv->thisptr = std::move(__pyx_t_10);

Maybe it actually makes sense to always generate a move if the compiler
supports it? I can't see any obvious disadvantages to not doing so, at
least for the specific case of exception handling temporaries...

Thanks for considering this!

--
Sincerely yours,
Yury V. Zaytsev

[Kevin Thornton]

Hi,

The developers will have to weigh in, but I don't think that this is straightforward.  Cython's C++ support appears to be trying to remain agnostic about C++98/11/14 at the moment.

Have you tried simply creating a unique_ptr without make_unique?   Would that be a sufficient workaround?  

I'm not sure if it matters, but make_unique is defined as a variadic, and should take an rvalue reference instead of a non-const reference.  Currently, Cython supports neither of those C++11 idioms.

[Yury V. Zaytsev]

Hi Kevin,

On Wed, 20 Jan 2016, Kevin Thornton wrote:

> The developers will have to weigh in, but I don't think that this is
> straightforward. Cython's C++ support appears to be trying to remain
> agnostic about C++98/11/14 at the moment.

Sure, but one can simply define a macro along the following lines:

#if __cplusplus > 199711L
#include <utility>
#define __Pyx_std_move(x) std::move(x)
#else
#define __Pyx_std_move(x) (x)
#endif

We can also use ">= 201103L" to stay on the safe side, but probably any
compiler that claims to support anything better than 199711L does have
support for move semantics (wild guess).

Maybe you are right though, and I should have sent this to the developer
list; not sure if this one is checked by Robert & Stefan and how often.

Also, I've noticed that there was some recent activity by Ian Henriksen
regarding exception handling for overloaded C++ operators. I'm not sure
what was the bottom line though, is it better to use the move hack, or
rather teach Cython that it shouldn't introduce temporaries for
non-copyiable objects? If that's the latter, then I'm out :-/ otherwise, I
could even try to come up with a pull request.

Orthogonally to that, some generators that I'm working with, like
CodeSynthesis XSD, for example, are getting options like `-std=c++14`, so
maybe that's yet another way to go, although, again, I don't know what the
core developers would favor.

> Have you tried simply creating a unique_ptr without make_unique? Would
> that be a sufficient workaround?

For now I've just left out the "except +" from my definition, the
downside, of course, is that if the exception is thrown from the type
constructor, the interpreter will crash.

> I'm not sure if it matters, but make_unique is defined as a variadic,
> and should take an rvalue reference instead of a non-const reference.
> Currently, Cython supports neither of those C++11 idioms.

Sure, I do know about that, but my declarations simply allow to make a
unique pointer out of a copy-constructable or default-constructable class,
which is all I currently need it for.

I've seen that Cython allows ellipsis (...) in function declarations, and
in theory I could have used that instead, but I couldn't find anything in
the documentation, so I thought I'd rather not do it.

[Kevin Thornton]

Just a random thought:

Could you declare your own template function with an internal try/catch that wraps std::make_unique, and returns a unique_ptr containing nullptr if an exception is thrown?

This way, you could have a function that is not "except +" as far as Cython  is concerned, yet still handles exceptions.

On Tuesday, January 19, 2016 at 7:45:56 AM UTC-8, Yury V. Zaytsev wrote:

[Yury V. Zaytsev]

Hi,

> Could you declare your own template function with an internal try/catch
> that wraps std::make_unique, and returns a unique_ptr containing nullptr
> if an exception is thrown?

Yes, this certainly gonna work, but I'd rather prefer the interpreter to
crash upon exception to checking the pointer manually and raising an
exception if it's empty.

I've had a look at the code, and, unfortunately, it seems that it's not as
simple to insert the moves as I thought, unless you do this for all
temporaries, because temporaries used for exception handling are allocated
via the normal temporary machinery.

However, if you start sticking the moves all over the place, you should
also make sure that the temporaries aren't used after they've been moved,
and I'm not familiar with the code, so that's probably too much for me.

Nevertheless, I've just got another idea. Maybe it's relatively easy to
teach Cython that it shouldn't introduce temporaries when doing exception
handling for classes with overloaded operator=() ?!

If I get it right, Ian did something like that for operator+(), so maybe
looking closer at his work will enlighten me...