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

Get some of the limited API working with Py_LIMITED_API defined #5550

Merged
merged 41 commits into from
Aug 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
658b9dc
Get a some of the limited API working with Py_LIMITED_API defined
da-woods Jul 20, 2023
41807c0
Try to fix setup.py
da-woods Jul 20, 2023
5349cd9
Small fixes
da-woods Jul 21, 2023
037c0f3
Move guards to "assume safe macros"
da-woods Jul 21, 2023
13aab71
Make arg spliting code handle avoid borrowed references
da-woods Jul 22, 2023
743a2ba
Reorder imports to allow monkey patching
da-woods Jul 22, 2023
8d7da21
Fix syntax error in regular mode
da-woods Jul 22, 2023
b9cb9d6
Merge remote-tracking branch 'real_origin/master' into limited-api-wi…
da-woods Jul 22, 2023
bb97acb
More fixes
da-woods Jul 22, 2023
b2227ab
Fix code style
da-woods Jul 22, 2023
335a718
Fix a few more tests
da-woods Jul 22, 2023
63a3963
Slightly improve error message
da-woods Jul 22, 2023
edc8242
ITEM and GetItem were the wrong way round
da-woods Jul 22, 2023
0c3eba8
Fix struct definition
da-woods Jul 22, 2023
516d5a7
Changes CyFunction to compile with Py_LIMITED_API
da-woods Jul 22, 2023
77829d9
Remove some spurious internal includes
da-woods Jul 22, 2023
5b743a1
Switch orders
da-woods Jul 23, 2023
0a3163c
Fixup
da-woods Jul 23, 2023
7fdefa2
Merge conditions
scoder Jul 23, 2023
a7262dc
Initialize weakref
da-woods Jul 23, 2023
ef209f8
Fix reference counting mistake
da-woods Jul 23, 2023
c9cdf46
Merge branch 'limited-api-cyfunc-2' into limited-api-with-py-limited-…
da-woods Jul 23, 2023
5e2be08
Minimal change to get classes working
da-woods Jul 23, 2023
2ca354b
Fixed borrowed reference
da-woods Jul 23, 2023
6bd0f37
Merge remote-tracking branch 'real_origin/master' into limited-api-wi…
da-woods Jul 23, 2023
1a5368a
Remove duplication
da-woods Jul 23, 2023
f2e3af8
Comments from review; get code working
da-woods Jul 24, 2023
280bda5
Fix code on Py3.11 and Py3.12
da-woods Jul 24, 2023
7d746b7
Remove PyCodeObject on limited API
da-woods Jul 24, 2023
3cccfeb
Adapt to awkward arg naming change
da-woods Jul 24, 2023
6484bdb
Let's avoid owned reference at least for the very common and safe fas…
scoder Aug 11, 2023
38fb949
Reorder conditions to align them with surrounding code.
scoder Aug 11, 2023
e98acbe
Fix PyCode_New on 3.7
da-woods Aug 11, 2023
cf5facc
Replace replace on Python 3.7
da-woods Aug 11, 2023
9721840
Merge remote-tracking branch 'origin/limited-api-with-py-limited-api'…
da-woods Aug 11, 2023
60f8a75
Fix 3.11 typo
da-woods Aug 11, 2023
f932e43
Merge remote-tracking branch 'real_origin/master' into limited-api-wi…
da-woods Aug 11, 2023
08744ed
Let CPython compile with assume_safe_macros
da-woods Aug 11, 2023
c3ba32c
Fixed function argument reference counting code
da-woods Aug 13, 2023
aa0a4c5
Silence warning on newref
da-woods Aug 13, 2023
2c1c050
Make sure ownership is properly transford out of ParseOptionalArguments
da-woods Aug 14, 2023
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
83 changes: 62 additions & 21 deletions Cython/Compiler/Nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -3595,6 +3595,7 @@ class DefNodeWrapper(FuncDefNode):

defnode = None
target = None # Target DefNode
needs_values_cleanup = False

def __init__(self, *args, **kwargs):
FuncDefNode.__init__(self, *args, **kwargs)
Expand Down Expand Up @@ -3706,7 +3707,7 @@ def generate_function_definitions(self, env, code):
code.put_declare_refcount_context()
code.put_setup_refcount_context(EncodedString('%s (wrapper)' % self.name))

self.generate_argument_parsing_code(lenv, code)
self.generate_argument_parsing_code(lenv, code, tempvardecl_code)
self.generate_argument_type_tests(code)
self.generate_function_body(code)

Expand Down Expand Up @@ -3745,6 +3746,7 @@ def generate_function_definitions(self, env, code):
else:
code.put_var_decref(arg.entry)

self.generate_argument_values_cleanup_code(code)
code.put_finish_refcount_context()
if not self.return_type.is_void:
code.putln("return %s;" % Naming.retval_cname)
Expand Down Expand Up @@ -3847,10 +3849,10 @@ def generate_argument_declarations(self, env, code):
if entry.is_arg:
code.put_var_declaration(entry)

# Assign nargs variable as len(args), but avoid an "unused" warning in the few cases where we don't need it.
# Create nargs, but avoid an "unused" warning in the few cases where we don't need it.
if self.signature_has_generic_args():
nargs_code = "CYTHON_UNUSED const Py_ssize_t %s = PyTuple_GET_SIZE(%s);" % (
Naming.nargs_cname, Naming.args_cname)
# error handling for this is checked after the declarations
nargs_code = "CYTHON_UNUSED Py_ssize_t %s;" % Naming.nargs_cname
if self.signature.use_fastcall:
code.putln("#if !CYTHON_METH_FASTCALL")
code.putln(nargs_code)
Expand All @@ -3859,12 +3861,9 @@ def generate_argument_declarations(self, env, code):
code.putln(nargs_code)

# Array containing the values of keyword arguments when using METH_FASTCALL.
code.globalstate.use_utility_code(
UtilityCode.load_cached("fastcall", "FunctionArguments.c"))
code.putln('CYTHON_UNUSED PyObject *const *%s = __Pyx_KwValues_%s(%s, %s);' % (
Naming.kwvalues_cname, self.signature.fastvar, Naming.args_cname, Naming.nargs_cname))
code.putln('CYTHON_UNUSED PyObject *const *%s;' % Naming.kwvalues_cname)

def generate_argument_parsing_code(self, env, code):
def generate_argument_parsing_code(self, env, code, decl_code):
# Generate fast equivalent of PyArg_ParseTuple call for
# generic arguments, if any, including args/kwargs
old_error_label = code.new_error_label()
Expand All @@ -3880,6 +3879,25 @@ def generate_argument_parsing_code(self, env, code):
if not arg.type.create_from_py_utility_code(env):
pass # will fail later

# Assign nargs variable as len(args).
if self.signature_has_generic_args():
if self.signature.use_fastcall:
code.putln("#if !CYTHON_METH_FASTCALL")
code.putln("#if CYTHON_ASSUME_SAFE_MACROS")
code.putln("%s = PyTuple_GET_SIZE(%s);" % (
Naming.nargs_cname, Naming.args_cname))
code.putln("#else")
code.putln("%s = PyTuple_Size(%s);" % (
Naming.nargs_cname, Naming.args_cname))
code.putln(code.error_goto_if_neg(Naming.nargs_cname, self.pos))
code.putln("#endif")
if self.signature.use_fastcall:
code.putln("#endif")
code.globalstate.use_utility_code(
UtilityCode.load_cached("fastcall", "FunctionArguments.c"))
code.putln('%s = __Pyx_KwValues_%s(%s, %s);' % (
Naming.kwvalues_cname, self.signature.fastvar, Naming.args_cname, Naming.nargs_cname))

if not self.signature_has_generic_args():
if has_star_or_kw_args:
error(self.pos, "This method cannot have * or keyword arguments")
Expand All @@ -3892,13 +3910,20 @@ def generate_argument_parsing_code(self, env, code):
self.generate_stararg_copy_code(code)

else:
self.generate_tuple_and_keyword_parsing_code(self.args, end_label, code)
self.generate_tuple_and_keyword_parsing_code(self.args, end_label, code, decl_code)
self.needs_values_cleanup = True

code.error_label = old_error_label
if code.label_used(our_error_label):
if not code.label_used(end_label):
code.put_goto(end_label)
# This goto is just to surpress a warning that our_error_label is unused
# since it's quite likely that the only use is guarded by
# CYTHON_AVOID_BORROWED_REFERENCES
code.put_goto(our_error_label)
code.put_label(our_error_label)
self.generate_argument_values_cleanup_code(code)

if has_star_or_kw_args:
self.generate_arg_decref(self.star_arg, code)
if self.starstar_arg:
Expand Down Expand Up @@ -4006,7 +4031,7 @@ def generate_stararg_copy_code(self, code):
Naming.args_cname))
self.star_arg.entry.xdecref_cleanup = 0

def generate_tuple_and_keyword_parsing_code(self, args, success_label, code):
def generate_tuple_and_keyword_parsing_code(self, args, success_label, code, decl_code):
code.globalstate.use_utility_code(
UtilityCode.load_cached("fastcall", "FunctionArguments.c"))

Expand Down Expand Up @@ -4065,7 +4090,7 @@ def generate_tuple_and_keyword_parsing_code(self, args, success_label, code):
# C-typed default arguments are handled at conversion time,
# so their array value is NULL in the end if no argument
# was passed for them.
self.generate_argument_values_setup_code(all_args, code)
self.generate_argument_values_setup_code(all_args, code, decl_code)

# If all args are positional-only, we can raise an error
# straight away if we receive a non-empty kw-dict.
Expand Down Expand Up @@ -4251,24 +4276,35 @@ def generate_stararg_init_code(self, max_positional_args, code):
code.putln('}')
code.put_var_gotref(self.star_arg.entry)

def generate_argument_values_setup_code(self, args, code):
def generate_argument_values_setup_code(self, args, code, decl_code):
max_args = len(args)
# the 'values' array collects borrowed references to arguments
# before doing any type coercion etc.
code.putln("PyObject* values[%d] = {%s};" % (
# the 'values' array collects references to arguments
# before doing any type coercion etc.. Whether they are borrowed or not
# depends on the compilation options.
decl_code.putln("PyObject* values[%d] = {%s};" % (
max_args, ','.join('0'*max_args)))

if self.target.defaults_struct:
code.putln('%s *%s = __Pyx_CyFunction_Defaults(%s, %s);' % (
self.target.defaults_struct, Naming.dynamic_args_cname,
self.target.defaults_struct, Naming.self_cname))

# assign borrowed Python default values to the values array,
# assign (usually borrowed) Python default values to the values array,
# so that they can be overwritten by received arguments below
for i, arg in enumerate(args):
if arg.default and arg.type.is_pyobject:
default_value = arg.calculate_default_value_code(code)
code.putln('values[%d] = %s;' % (i, arg.type.as_pyobject(default_value)))
code.putln('values[%d] = __Pyx_Arg_NewRef_%s(%s);' % (
i, self.signature.fastvar, arg.type.as_pyobject(default_value)))

def generate_argument_values_cleanup_code(self, code):
if not self.needs_values_cleanup:
return
# The 'values' array may not be borrowed depending on the compilation options.
# This cleans it up in the case it isn't borrowed
code.putln("for (Py_ssize_t i=0; i<(Py_ssize_t)(sizeof(values)/sizeof(values[0])); ++i) {")
code.putln("__Pyx_Arg_XDECREF_%s(values[i]);" % self.signature.fastvar)
code.putln("}")

def generate_keyword_unpacking_code(self, min_positional_args, max_positional_args,
has_fixed_positional_count,
Expand Down Expand Up @@ -4352,12 +4388,16 @@ def generate_keyword_unpacking_code(self, min_positional_args, max_positional_ar
# don't overwrite default argument
code.putln('PyObject* value = __Pyx_GetKwValue_%s(%s, %s, %s);' % (
self.signature.fastvar, Naming.kwds_cname, Naming.kwvalues_cname, pystring_cname))
code.putln('if (value) { values[%d] = value; kw_args--; }' % i)
code.putln('if (value) { values[%d] = __Pyx_Arg_NewRef_%s(value); kw_args--; }' % (
i, self.signature.fastvar))
code.putln('else if (unlikely(PyErr_Occurred())) %s' % code.error_goto(self.pos))
code.putln('}')
else:
code.putln('if (likely((values[%d] = __Pyx_GetKwValue_%s(%s, %s, %s)) != 0)) kw_args--;' % (
code.putln('if (likely((values[%d] = __Pyx_GetKwValue_%s(%s, %s, %s)) != 0)) {' % (
i, self.signature.fastvar, Naming.kwds_cname, Naming.kwvalues_cname, pystring_cname))
code.putln('(void)__Pyx_Arg_NewRef_%s(values[%d]);' % (self.signature.fastvar, i))
code.putln('kw_args--;')
code.putln('}')
code.putln('else if (unlikely(PyErr_Occurred())) %s' % code.error_goto(self.pos))
if i < min_positional_args:
if i == 0:
Expand Down Expand Up @@ -4484,7 +4524,8 @@ def generate_optional_kwonly_args_unpacking_code(self, all_args, code):
Naming.kwvalues_cname,
Naming.pykwdlist_cname,
posonly_correction))
code.putln('if (value) { values[index] = value; kw_args--; }')
code.putln('if (value) { values[index] = __Pyx_Arg_NewRef_%s(value); kw_args--; }' %
self.signature.fastvar)
code.putln('else if (unlikely(PyErr_Occurred())) %s' % code.error_goto(self.pos))
if len(optional_args) > 1:
code.putln('}')
Expand Down
2 changes: 1 addition & 1 deletion Cython/Utility/CythonFunction.c
Original file line number Diff line number Diff line change
Expand Up @@ -1132,7 +1132,7 @@ static PyTypeObject __pyx_CyFunctionType_type = {
#ifdef Py_TPFLAGS_METHOD_DESCRIPTOR
Py_TPFLAGS_METHOD_DESCRIPTOR |
#endif
#ifdef _Py_TPFLAGS_HAVE_VECTORCALL
#if defined(_Py_TPFLAGS_HAVE_VECTORCALL) && CYTHON_METH_FASTCALL
_Py_TPFLAGS_HAVE_VECTORCALL |
#endif
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_BASETYPE, /*tp_flags*/
Expand Down
101 changes: 99 additions & 2 deletions Cython/Utility/Exceptions.c
Original file line number Diff line number Diff line change
Expand Up @@ -911,22 +911,119 @@ static void __Pyx_AddTraceback(const char *funcname, int c_line,
#include "compile.h"
#include "frameobject.h"
#include "traceback.h"
#if PY_VERSION_HEX >= 0x030b00a6
#if PY_VERSION_HEX >= 0x030b00a6 && !CYTHON_COMPILING_IN_LIMITED_API
#ifndef Py_BUILD_CORE
#define Py_BUILD_CORE 1
#endif
#include "internal/pycore_frame.h"
#endif

#if CYTHON_COMPILING_IN_LIMITED_API
static PyObject *__Pyx_PyCode_Replace_For_AddTraceback(PyObject *code, PyObject *scratch_dict,
PyObject *firstlineno, PyObject *name) {
PyObject *replace = NULL;
if (unlikely(PyDict_SetItemString(scratch_dict, "co_firstlineno", firstlineno))) return NULL;
if (unlikely(PyDict_SetItemString(scratch_dict, "co_name", name))) return NULL;

replace = PyObject_GetAttrString(code, "replace");
if (likely(replace)) {
PyObject *result;
result = PyObject_Call(replace, $empty_tuple, scratch_dict);
Py_DECREF(replace);
return result;
}

#if __PYX_LIMITED_VERSION_HEX < 0x030780000
// If we're here, we're probably on Python <=3.7 which doesn't have code.replace.
// In this we take a lazy interpretted route (without regard to performance
// since it's fairly old and this is mostly just to get something working)
PyErr_Clear();
{
PyObject *compiled = NULL, *result = NULL;
if (unlikely(PyDict_SetItemString(scratch_dict, "code", code))) return NULL;
if (unlikely(PyDict_SetItemString(scratch_dict, "type", (PyObject*)(&PyType_Type)))) return NULL;
compiled = Py_CompileString(
"out = type(code)(\n"
" code.co_argcount, code.co_kwonlyargcount, code.co_nlocals, code.co_stacksize,\n"
" code.co_flags, code.co_code, code.co_consts, code.co_names,\n"
" code.co_varnames, code.co_filename, co_name, co_firstlineno,\n"
" code.co_lnotab)\n", "<dummy>", Py_file_input);
if (!compiled) return NULL;
result = PyEval_EvalCode(compiled, scratch_dict, scratch_dict);
Py_DECREF(compiled);
if (!result) PyErr_Print();
Py_DECREF(result);
result = PyDict_GetItemString(scratch_dict, "out");
if (result) Py_INCREF(result);
return result;
}
#endif
}

static void __Pyx_AddTraceback(const char *funcname, int c_line,
int py_line, const char *filename) {
PyObject *code_object = NULL, *py_py_line = NULL, *py_funcname = NULL, *dict = NULL;
PyObject *replace = NULL, *getframe = NULL, *frame = NULL;
PyObject *exc_type, *exc_value, *exc_traceback;
int success = 0;
if (c_line) {
// Avoid "unused" warning as long as we don't use this.
(void) $cfilenm_cname;
(void) __Pyx_CLineForTraceback(__Pyx_PyThreadState_Current, c_line);
}
_PyTraceback_Add(funcname, filename, py_line);

// DW - this is a horrendous hack, but I'm quite proud of it. Essentially
Copy link
Contributor

Choose a reason for hiding this comment

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

I'll just ignore this part in my review and believe both properties as you state them here.

Copy link
Contributor

Choose a reason for hiding this comment

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

It does look like it would slow down exception propagation considerably, though. We can probably cache the code object (or its replace method) and just run that each time, rather than also creating a fresh one (i.e. two) for each traceback frame.

I'd be ok with keeping it as it is for now, and apply optimisations later. Let's just keep in mind that this is probably not final.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah - it'd noticed that it could be cached, but was just trying to keep things simple for the initial version.

We can definitely come back and optimize this later.

I'm also open to different approaches, but I honestly couldn't think of anything else that might work.

// we need to generate a frame with the right line number/filename/funcname.
// We do this by compiling a small bit of code that uses sys._getframe to get a
// frame, and then customizing the details of the code to match.
// We then run the code object and use the generated frame to set the traceback.

PyErr_Fetch(&exc_type, &exc_value, &exc_traceback);

code_object = Py_CompileString("_getframe()", filename, Py_eval_input);
if (unlikely(!code_object)) goto bad;
py_py_line = PyLong_FromLong(py_line);
if (unlikely(!py_py_line)) goto bad;
py_funcname = PyUnicode_FromString(funcname);
if (unlikely(!py_funcname)) goto bad;
dict = PyDict_New();
if (unlikely(!dict)) goto bad;
{
PyObject *old_code_object = code_object;
code_object = __Pyx_PyCode_Replace_For_AddTraceback(code_object, dict, py_py_line, py_funcname);
Py_DECREF(old_code_object);
}
if (unlikely(!code_object)) goto bad;

// Note that getframe is borrowed
getframe = PySys_GetObject("_getframe");
if (unlikely(!getframe)) goto bad;
// reuse dict as globals (nothing conflicts, and it saves an allocation)
if (unlikely(PyDict_SetItemString(dict, "_getframe", getframe))) goto bad;

frame = PyEval_EvalCode(code_object, dict, dict);
if (unlikely(!frame) || frame == Py_None) goto bad;
success = 1;

bad:
PyErr_Restore(exc_type, exc_value, exc_traceback);
Py_XDECREF(code_object);
Py_XDECREF(py_py_line);
Py_XDECREF(py_funcname);
Py_XDECREF(dict);
Py_XDECREF(replace);

if (success) {
// Unfortunately an easy way to check the type of frame isn't in the
// limited API. The check against None should cover the most
// likely wrong answer though.
PyTraceBack_Here(
// Python < 0x03090000 didn't expose PyFrameObject
// but they all expose struct _frame as an opaque type
(struct _frame*)frame);
}

Py_XDECREF(frame);
}
#else
static PyCodeObject* __Pyx_CreateCodeObjectForTraceback(
Expand Down