Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update signature of make_unique #5569

Merged
merged 2 commits into from
Jul 31, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 1 addition & 2 deletions Cython/Includes/libcpp/memory.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,7 @@ cdef extern from "<memory>" namespace "std" nogil:
# Smart pointer non-member operations
shared_ptr[T] make_shared[T](...) except +

# Temporaries used for exception handling break generated code
unique_ptr[T] make_unique[T](...) # except +
unique_ptr[T] make_unique[T](...) except +

# No checking on the compatibility of T and U.
cdef shared_ptr[T] static_pointer_cast[T, U](const shared_ptr[U]&)
Expand Down
3 changes: 1 addition & 2 deletions Cython/Utility/CppSupport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@ auto __Pyx_pythran_to_python(T &&value) -> decltype(to_python(

////////////// MoveIfSupported.proto //////////////////

#if __cplusplus >= 201103L || (defined(_MSC_VER) && _MSC_VER >= 1600)
// move should be defined for these versions of MSVC, but __cplusplus isn't set usefully
#if CYTHON_USE_CPP11_FEATURES
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seeing the comment, does _MSC_VER >= 1600 really imply the support for "C++11 features" in general, or just std::move? It seems that std::move is the only C++11 feature that we use internally, so why not call the feature flag CYTHON_USE_CPP_MOVE?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done that - c++11 support looks to be slowly implemented across multiple MSVC versions (of course this is probably true for every compiler...).

(I think we do use C++11 lambdas in one place, But we only do it if the user uses c++11 enums, so that doesn't really need a guard).

#include <utility>
#define __PYX_STD_MOVE_IF_SUPPORTED(x) std::move(x)
#else
Expand Down
10 changes: 10 additions & 0 deletions Cython/Utility/ModuleSetupCode.c
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,16 @@
# endif
#endif

#ifndef CYTHON_USE_CPP11_FEATURES
// msvc doesn't set __cplusplus to a useful value
#if defined(__cplusplus) && ( \
__cplusplus >= 201103L || (defined(_MSC_VER) && _MSC_VER >= 1600))
#define CYTHON_USE_CPP11_FEATURES 1
#else
#define CYTHON_USE_CPP11_FEATURES 0
#endif
#endif

#define __Pyx_void_to_None(void_result) ((void)(void_result), Py_INCREF(Py_None), Py_None)

#ifdef _MSC_VER
Expand Down
17 changes: 15 additions & 2 deletions tests/run/cpp_smart_ptr.pyx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# mode: run
# tag: cpp, werror, cpp11, no-cpp-locals
# tag: cpp, werror, cpp14, no-cpp-locals

from libcpp.memory cimport unique_ptr, shared_ptr, default_delete, dynamic_pointer_cast
from libcpp.memory cimport unique_ptr, shared_ptr, default_delete, dynamic_pointer_cast, make_unique
from libcpp cimport nullptr

cdef extern from "cpp_smart_ptr_helper.h":
Expand All @@ -11,6 +11,9 @@ cdef extern from "cpp_smart_ptr_helper.h":
cdef cppclass FreePtr[T]:
pass

cdef cppclass RaiseOnConstruct:
pass


ctypedef const CountAllocDealloc const_CountAllocDealloc

Expand Down Expand Up @@ -45,6 +48,16 @@ def test_unique_ptr():
x_ptr3.reset()
assert x_ptr3.get() == nullptr;

# Test that make_unique works
cdef unique_ptr[int] x_ptr4
x_ptr4 = make_unique[int](5)
assert(x_ptr4) != nullptr
cdef unique_ptr[RaiseOnConstruct] x_ptr5
try:
x_ptr5 = make_unique[RaiseOnConstruct]()
except RuntimeError:
pass # good - this is what we expect


def test_const_shared_ptr():
"""
Expand Down
9 changes: 9 additions & 0 deletions tests/run/cpp_smart_ptr_helper.h
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#include <stdexcept>

class CountAllocDealloc {
public:
CountAllocDealloc(int* alloc_count, int* dealloc_count)
Expand All @@ -22,3 +24,10 @@ struct FreePtr {
}
}
};

class RaiseOnConstruct {
public:
RaiseOnConstruct() {
throw std::runtime_error("Oh no!");
}
};