diff --git a/src/cpyrt/CPPOverload.cxx b/src/cpyrt/CPPOverload.cxx index 5639bde..f23ed0d 100644 --- a/src/cpyrt/CPPOverload.cxx +++ b/src/cpyrt/CPPOverload.cxx @@ -445,36 +445,23 @@ static int mp_setcreates(CPPOverload* pymeth, PyObject* value, void*) { return set_flag(pymeth, value, CallContext::kIsCreator, "__creates__"); } -//---------------------------------------------------------------------------- -static PyObject* mp_getmempolicy(CPPOverload* pymeth, void*) { - // Get '_mempolicy' enum, which determines ownership of call arguments. - if (pymeth->fMethodInfo->fFlags & CallContext::kUseHeuristics) - return PyInt_FromLong(CallContext::kUseHeuristics); - - if (pymeth->fMethodInfo->fFlags & CallContext::kUseStrict) - return PyInt_FromLong(CallContext::kUseStrict); +constexpr const char* mempolicy_error_message = + "The __mempolicy__ attribute can't be used, because in the past it was " + "reserved to manage the local memory policy. " + "If you want to do that now, please implement a pythonization for your " + "class that uses SetOwnership() to manage the " + "ownership of arguments according to your needs."; - return PyInt_FromLong(-1); +//---------------------------------------------------------------------------- +static PyObject* mp_getmempolicy(CPPOverload*, void*) { + PyErr_SetString(PyExc_RuntimeError, mempolicy_error_message); + return nullptr; } //---------------------------------------------------------------------------- -static int mp_setmempolicy(CPPOverload* pymeth, PyObject* value, void*) { - // Set '_mempolicy' enum, which determines ownership of call arguments. - long mempolicy = PyLong_AsLong(value); - if (mempolicy == CallContext::kUseHeuristics) { - pymeth->fMethodInfo->fFlags |= CallContext::kUseHeuristics; - pymeth->fMethodInfo->fFlags &= ~CallContext::kUseStrict; - } else if (mempolicy == CallContext::kUseStrict) { - pymeth->fMethodInfo->fFlags |= CallContext::kUseStrict; - pymeth->fMethodInfo->fFlags &= ~CallContext::kUseHeuristics; - } else { - PyErr_SetString(PyExc_ValueError, - "expected kMemoryStrict or kMemoryHeuristics as value for " - "__mempolicy__"); - return -1; - } - - return 0; +static int mp_setmempolicy(CPPOverload*, PyObject*, void*) { + PyErr_SetString(PyExc_RuntimeError, mempolicy_error_message); + return -1; } //---------------------------------------------------------------------------- @@ -541,9 +528,7 @@ static PyGetSetDef mp_getset[] = { (char*)"For ownership rules of result: if true, objects are python-owned", nullptr}, {(char*)"__mempolicy__", (getter)mp_getmempolicy, (setter)mp_setmempolicy, - (char*)"For argument ownership rules: like global, either heuristic or " - "strict", - nullptr}, + (char*)"Unused", nullptr}, {(char*)"__set_lifeline__", (getter)mp_getlifeline, (setter)mp_setlifeline, (char*)"If true, set a lifeline from the return value onto self", nullptr}, {(char*)"__release_gil__", (getter)mp_getthreaded, (setter)mp_setthreaded, @@ -575,9 +560,6 @@ static PyObject* mp_vectorcall(CPPOverload* pymeth, PyObject* const* args, CallContext ctxt{}; const auto mflags = pymeth->fMethodInfo->fFlags; - const auto mempolicy = - (mflags & (CallContext::kUseHeuristics | CallContext::kUseStrict)); - ctxt.fFlags |= mempolicy ? mempolicy : (uint64_t)CallContext::sMemoryPolicy; ctxt.fFlags |= (mflags & CallContext::kReleaseGIL); ctxt.fFlags |= (mflags & CallContext::kProtected); if (IsConstructor(pymeth->fMethodInfo->fFlags)) @@ -982,10 +964,19 @@ void cpyrt::CPPOverload::Set(const std::string& name, fMethodInfo->fFlags |= (CallContext::kIsCreator | CallContext::kIsConstructor); - // special case, in heuristics mode also tag *Clone* methods as creators - if (CallContext::sMemoryPolicy == CallContext::kUseHeuristics && - name.find("Clone") != std::string::npos) - fMethodInfo->fFlags |= CallContext::kIsCreator; + // special case, in heuristics mode also tag *Clone* methods as creators. Only + // check that Clone is present in the method name, not in the template + // argument list. + if (CallContext::GlobalPolicyFlags() & CallContext::kUseHeuristics) { + std::string_view name_maybe_template = name; + auto begin_template = name_maybe_template.find_first_of('<'); + if (begin_template <= name_maybe_template.size()) { + name_maybe_template = name_maybe_template.substr(0, begin_template); + } + if (name_maybe_template.find("Clone") != std::string_view::npos) { + fMethodInfo->fFlags |= CallContext::kIsCreator; + } + } fVectorCall = (vectorcallfunc)mp_vectorcall; } diff --git a/src/cpyrt/CallContext.cxx b/src/cpyrt/CallContext.cxx index f8d077e..4a9e646 100644 --- a/src/cpyrt/CallContext.cxx +++ b/src/cpyrt/CallContext.cxx @@ -4,15 +4,11 @@ using namespace cppjit; #include "CallContext.h" -//- data _____________________________________________________________________ -namespace cppjit::cpyrt { - -CallContext::ECallFlags CallContext::sMemoryPolicy = CallContext::kUseStrict; -// this is just a data holder for linking; actual value is set in -// cpyrtModule.cxx -CallContext::ECallFlags CallContext::sSignalPolicy = CallContext::kNone; - -} // namespace cppjit::cpyrt +//----------------------------------------------------------------------------- +uint32_t& cpyrt::CallContext::GlobalPolicyFlags() { + static uint32_t flags = 0; + return flags; +} //----------------------------------------------------------------------------- void cpyrt::CallContext::AddTemporary(PyObject* pyobj) { @@ -41,21 +37,12 @@ void cpyrt::CallContext::Cleanup() { } //----------------------------------------------------------------------------- -bool cpyrt::CallContext::SetMemoryPolicy(ECallFlags e) { - // Set the global memory policy, which affects object ownership when objects - // are passed as function arguments. - if (kUseHeuristics == e || e == kUseStrict) { - sMemoryPolicy = e; - return true; - } - return false; -} - -//----------------------------------------------------------------------------- -bool cpyrt::CallContext::SetGlobalSignalPolicy(bool setProtected) { - // Set the global signal policy, which determines whether a jmp address - // should be saved to return to after a C++ segfault. - bool old = sSignalPolicy == kProtected; - sSignalPolicy = setProtected ? kProtected : kNone; +bool cpyrt::CallContext::SetGlobalPolicy(ECallFlags toggleFlag, bool enabled) { + auto& flags = GlobalPolicyFlags(); + bool old = flags & toggleFlag; + if (enabled) + flags |= toggleFlag; + else + flags &= ~toggleFlag; return old; } diff --git a/src/cpyrt/CallContext.h b/src/cpyrt/CallContext.h index b52915b..f8000a0 100644 --- a/src/cpyrt/CallContext.h +++ b/src/cpyrt/CallContext.h @@ -49,20 +49,16 @@ struct CallContext { kProtected = 0x008000, // if method should return on signals kUseFFI = 0x010000, // not implemented kIsPseudoFunc = 0x020000, // internal, used for introspection - kUseStrict = 0x040000, // if method applies strict memory policy }; - // memory handling - static ECallFlags sMemoryPolicy; - static bool SetMemoryPolicy(ECallFlags e); + // Policies about memory handling and signal safety + static bool SetGlobalPolicy(ECallFlags e, bool enabled); + + static uint32_t& GlobalPolicyFlags(); void AddTemporary(PyObject* pyobj); void Cleanup(); - // signal safety - static ECallFlags sSignalPolicy; - static bool SetGlobalSignalPolicy(bool setProtected); - Parameter* GetArgs(size_t sz) { if (sz != (size_t)-1) fNArgs = sz; @@ -134,13 +130,19 @@ inline bool ReleasesGIL(CallContext* ctxt) { return ctxt ? (ctxt->fFlags & CallContext::kReleaseGIL) : false; } -inline bool UseStrictOwnership(CallContext* ctxt) { - if (ctxt && (ctxt->fFlags & CallContext::kUseStrict)) - return true; - if (ctxt && (ctxt->fFlags & CallContext::kUseHeuristics)) - return false; +inline bool UseStrictOwnership() { + using CC = cppjit::cpyrt::CallContext; + return !(CC::GlobalPolicyFlags() & CC::kUseHeuristics); +} - return CallContext::sMemoryPolicy == CallContext::kUseStrict; +// kImplicitSmartPtrConversion is a global policy (set through +// cppyy.SetImplicitSmartPointerConversion), but it can also be requested for a +// single call through the call context, so check both words - as is done for +// kProtected in CPPMethod::Execute. +inline bool AllowImplicitSmartPtrConversion(CallContext* ctxt) { + using CC = cppjit::cpyrt::CallContext; + return (CC::GlobalPolicyFlags() & CC::kImplicitSmartPtrConversion) || + (ctxt && (ctxt->fFlags & CC::kImplicitSmartPtrConversion)); } template class CallContextRAII { diff --git a/src/cpyrt/Converters.cxx b/src/cpyrt/Converters.cxx index cd6d6cf..48bf47c 100644 --- a/src/cpyrt/Converters.cxx +++ b/src/cpyrt/Converters.cxx @@ -1615,14 +1615,14 @@ bool cpyrt::VoidArrayConverter::GetAddressSpecialCase(PyObject* pyobject, //---------------------------------------------------------------------------- bool cpyrt::VoidArrayConverter::SetArg(PyObject* pyobject, Parameter& para, - CallContext* ctxt) { + CallContext* /*ctxt*/) { // just convert pointer if it is a C++ object CPPInstance* pyobj = GetCppInstance(pyobject); para.fValue.fVoidp = nullptr; if (pyobj) { // depending on memory policy, some objects are no longer owned when passed // to C++ - if (!fKeepControl && !UseStrictOwnership(ctxt)) + if (!fKeepControl && !UseStrictOwnership()) pyobj->CppOwns(); // set pointer (may be null) and declare success @@ -1688,7 +1688,7 @@ bool cpyrt::VoidArrayConverter::ToMemory(PyObject* value, void* address, if (pyobj) { // depending on memory policy, some objects are no longer owned when passed // to C++ - if (!fKeepControl && CallContext::sMemoryPolicy != CallContext::kUseStrict) + if (!fKeepControl && !UseStrictOwnership()) pyobj->CppOwns(); // set pointer (may be null) and declare success @@ -2308,7 +2308,7 @@ bool cpyrt::InstancePtrConverter::SetArg(PyObject* pyobject, if (oisa && (oisa == fClass || interop::IsSubclass(oisa, fClass))) { // depending on memory policy, some objects need releasing when passed into // functions - if (!KeepControl() && !UseStrictOwnership(ctxt)) + if (!KeepControl() && !UseStrictOwnership()) pyobj->CppOwns(); // calculate offset between formal and actual arguments @@ -2369,7 +2369,7 @@ bool cpyrt::InstancePtrConverter::ToMemory(PyObject* value, if (interop::IsSubclass(pyobj->ObjectIsA(), fClass)) { // depending on memory policy, some objects need releasing when passed into // functions - if (!KeepControl() && CallContext::sMemoryPolicy != CallContext::kUseStrict) + if (!KeepControl() && !UseStrictOwnership()) ((CPPInstance*)value)->CppOwns(); *(void**)address = pyobj->GetObject(); @@ -2532,9 +2532,8 @@ bool cpyrt::InstanceMoveConverter::SetArg(PyObject* pyobject, Parameter& para, //---------------------------------------------------------------------------- template -bool cpyrt::InstancePtrPtrConverter::SetArg(PyObject* pyobject, - Parameter& para, - CallContext* ctxt) { +bool cpyrt::InstancePtrPtrConverter::SetArg( + PyObject* pyobject, Parameter& para, CallContext* /*ctxt*/) { // convert to C++ instance**, set arg for call CPPInstance* pyobj = GetCppInstance(pyobject); if (!pyobj) { @@ -2551,7 +2550,7 @@ bool cpyrt::InstancePtrPtrConverter::SetArg(PyObject* pyobject, if (interop::IsSubclass(pyobj->ObjectIsA(), fClass)) { // depending on memory policy, some objects need releasing when passed into // functions - if (!KeepControl() && !UseStrictOwnership(ctxt)) + if (!KeepControl() && !UseStrictOwnership()) pyobj->CppOwns(); // set pointer (may be null) and declare success @@ -2593,7 +2592,7 @@ bool cpyrt::InstancePtrPtrConverter::ToMemory( if (interop::IsSubclass(pyobj->ObjectIsA(), fClass)) { // depending on memory policy, some objects need releasing when passed into // functions - if (!KeepControl() && CallContext::sMemoryPolicy != CallContext::kUseStrict) + if (!KeepControl() && !UseStrictOwnership()) pyobj->CppOwns(); // register the value for potential recycling @@ -3111,7 +3110,7 @@ bool cpyrt::SmartPtrConverter::SetArg(PyObject* pyobject, Parameter& para, if (interop::IsSubclass(tsmart, fSmartPtrType)) { // depending on memory policy, some objects need releasing when passed // into functions - if (!fKeepControl && !UseStrictOwnership(ctxt)) + if (!fKeepControl && !UseStrictOwnership()) ((CPPInstance*)pyobject)->CppOwns(); // calculate offset between formal and actual arguments @@ -3142,7 +3141,8 @@ bool cpyrt::SmartPtrConverter::SetArg(PyObject* pyobject, Parameter& para, } // for the case where we have an ordinary object to convert - if (!pyobj->IsSmart() && interop::IsSubclass(oisa, fUnderlyingType)) { + if (AllowImplicitSmartPtrConversion(ctxt) && !pyobj->IsSmart() && + interop::IsSubclass(oisa, fUnderlyingType)) { // create the relevant smart pointer and make the pyobject "smart" CPPInstance* pysmart = (CPPInstance*)ConvertImplicit( fSmartPtrType, pyobject, para, ctxt, false); diff --git a/src/cpyrt/Dispatcher.cxx b/src/cpyrt/Dispatcher.cxx index 1fafb48..9017ae6 100644 --- a/src/cpyrt/Dispatcher.cxx +++ b/src/cpyrt/Dispatcher.cxx @@ -553,7 +553,12 @@ bool cpyrt::InsertDispatcher(CPPScope* klass, PyObject* bases, PyObject* dct, // is not a base of the Python class to keep the inheritance tree intact) for (const auto& name : protected_names) { PyObject* disp_dct = PyObject_GetAttr(disp_proxy, PyStrings::gDict); +#if PY_VERSION_HEX < 0x30d00f0 PyObject* pyf = PyMapping_GetItemString(disp_dct, (char*)name.c_str()); +#else + PyObject* pyf = nullptr; + PyMapping_GetOptionalItemString(disp_dct, (char*)name.c_str(), &pyf); +#endif if (pyf) { PyObject_SetAttrString((PyObject*)klass, (char*)name.c_str(), pyf); Py_DECREF(pyf); diff --git a/src/cpyrt/cpyrtModule.cxx b/src/cpyrt/cpyrtModule.cxx index dd54099..6293389 100644 --- a/src/cpyrt/cpyrtModule.cxx +++ b/src/cpyrt/cpyrtModule.cxx @@ -752,39 +752,24 @@ static PyObject* AddTypeReducer(PyObject*, PyObject* args) { Py_RETURN_NONE; } -//---------------------------------------------------------------------------- -static PyObject* SetMemoryPolicy(PyObject*, PyObject* args) { - // Set the global memory policy, which affects object ownership when objects - // are passed as function arguments. - PyObject* policy = nullptr; - if (!PyArg_ParseTuple(args, const_cast("O!"), &PyInt_Type, &policy)) - return nullptr; - - long old = (long)CallContext::sMemoryPolicy; - - long l = PyInt_AS_LONG(policy); - if (CallContext::SetMemoryPolicy((CallContext::ECallFlags)l)) { - return PyInt_FromLong(old); - } - - PyErr_Format(PyExc_ValueError, "Unknown policy %ld", l); - return nullptr; -} - -//---------------------------------------------------------------------------- -static PyObject* SetGlobalSignalPolicy(PyObject*, PyObject* args) { - // Set the global signal policy, which determines whether a jmp address - // should be saved to return to after a C++ segfault. - PyObject* setProtected = 0; - if (!PyArg_ParseTuple(args, const_cast("O"), &setProtected)) - return nullptr; - - if (CallContext::SetGlobalSignalPolicy(PyObject_IsTrue(setProtected))) { - Py_RETURN_TRUE; +#define DEFINE_CALL_POLICY_TOGGLE(name, flagname) \ + static PyObject* name(PyObject*, PyObject* args) { \ + PyObject* enabled = 0; \ + if (!PyArg_ParseTuple(args, const_cast("O"), &enabled)) \ + return nullptr; \ + \ + if (CallContext::SetGlobalPolicy(CallContext::flagname, \ + PyObject_IsTrue(enabled))) { \ + Py_RETURN_TRUE; \ + } \ + \ + Py_RETURN_FALSE; \ } - Py_RETURN_FALSE; -} +DEFINE_CALL_POLICY_TOGGLE(SetHeuristicMemoryPolicy, kUseHeuristics); +DEFINE_CALL_POLICY_TOGGLE(SetImplicitSmartPointerConversion, + kImplicitSmartPtrConversion); +DEFINE_CALL_POLICY_TOGGLE(SetGlobalSignalPolicy, kProtected); //---------------------------------------------------------------------------- static PyObject* SetOwnership(PyObject*, PyObject* args) { @@ -867,11 +852,20 @@ static PyMethodDef gcpyrtMethods[] = { (char*)"Install a type pinning."}, {(char*)"_add_type_reducer", (PyCFunction)AddTypeReducer, METH_VARARGS, (char*)"Add a type reducer."}, - {(char*)"SetMemoryPolicy", (PyCFunction)SetMemoryPolicy, METH_VARARGS, - (char*)"Determines object ownership model."}, + {(char*)"SetHeuristicMemoryPolicy", (PyCFunction)SetHeuristicMemoryPolicy, + METH_VARARGS, + (char*)"Set the global memory policy, which affects object ownership when " + "objects are passed as function arguments."}, + {(char*)"SetImplicitSmartPointerConversion", + (PyCFunction)SetImplicitSmartPointerConversion, METH_VARARGS, + (char*)"Enable or disable the implicit conversion to smart pointers in " + "function calls (on by default)."}, {(char*)"SetGlobalSignalPolicy", (PyCFunction)SetGlobalSignalPolicy, METH_VARARGS, - (char*)"Trap signals in safe mode to prevent interpreter abort."}, + (char*)"Set the global signal policy, which determines whether a jmp " + "address should be saved to return to after a " + "C++ segfault. In practical terms: trap signals in safe mode to " + "prevent interpreter abort."}, {(char*)"SetOwnership", (PyCFunction)SetOwnership, METH_VARARGS, (char*)"Modify held C++ object ownership."}, {(char*)"AddSmartPtrType", (PyCFunction)AddSmartPtrType, METH_VARARGS, @@ -1003,12 +997,6 @@ extern "C" PyObject* PyInit_libcppjit() { PyErr_NewException((char*)"cppjit.ll.AbortSignal", cppfatal, nullptr); PyModule_AddObject(gThisModule, (char*)"AbortSignal", gAbrtException); - // policy labels - PyModule_AddObject(gThisModule, (char*)"kMemoryHeuristics", - PyInt_FromLong((int)CallContext::kUseHeuristics)); - PyModule_AddObject(gThisModule, (char*)"kMemoryStrict", - PyInt_FromLong((int)CallContext::kUseStrict)); - // gbl namespace is injected in cppjit.py // create the memory regulator diff --git a/test/test_conversions.py b/test/test_conversions.py index 65b1600..354f87e 100644 --- a/test/test_conversions.py +++ b/test/test_conversions.py @@ -154,3 +154,30 @@ def test07_mutable_voidp_reference(self): o = ns.Obj() assert ns.is_same(o, o) + + +class TestSMARTPTRPOLICY: + def test01_implicit_smartptr_conversion_policy(self): + """Wrapping an object into a smart-pointer argument is off by default""" + + import cppjit + from pytest import raises + + cppjit.cppdef("""\ + #include + namespace SPPolicy { + struct Payload { int x = 5; }; + int take(std::shared_ptr p) { return p ? p->x : -1; } + }""") + + obj = cppjit.gbl.SPPolicy.Payload() + with raises(TypeError): + cppjit.gbl.SPPolicy.take(obj) + + # a smart pointer passes regardless of the policy + sp = cppjit.gbl.std.make_shared["SPPolicy::Payload"]() + assert cppjit.gbl.SPPolicy.take(sp) == 5 + + # the toggle is exposed and returns the prior setting + assert cppjit.libcppjit.SetImplicitSmartPointerConversion(True) is False + assert cppjit.libcppjit.SetImplicitSmartPointerConversion(False) is True diff --git a/test/test_regression.py b/test/test_regression.py index 980cbf3..ac0ff02 100644 --- a/test/test_regression.py +++ b/test/test_regression.py @@ -1385,9 +1385,7 @@ def test44_heuristic_mem_policy(self): try: # The scope with the heuristic memory policy is in a try-except-finally block # to ensure the memory policy is always reset. - old_memory_policy = cppjit._backend.SetMemoryPolicy( - cppjit._backend.kMemoryHeuristics - ) + old_memory_policy = cppjit._backend.SetHeuristicMemoryPolicy(True) # Validate the intended behavior for different argument types: # const ref : caller keeps ownership @@ -1414,7 +1412,7 @@ def test44_heuristic_mem_policy(self): except: raise # rethrow the exception finally: - cppjit._backend.SetMemoryPolicy(old_memory_policy) + cppjit._backend.SetHeuristicMemoryPolicy(old_memory_policy) @mark.xfail(condition=IS_MAC, reason="Fails on OS X") def test45_typedef_resolution(self):