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

gh-94808: add tests covering PyFunction_{Get,Set}Closure #99429

Merged
merged 4 commits into from
Mar 20, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 118 additions & 0 deletions Lib/test/test_capi/test_misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -1171,6 +1171,124 @@ class dictsub(dict): ... # dict subclasses must work
self.assertEqual(_testcapi.function_get_kw_defaults(some), None)
self.assertEqual(some.__kwdefaults__, None)

def test_function_get_closure(self):
from types import CellType

def regular_function(): ...
def unused_one_level(arg1):
def inner(arg2, arg3): ...
return inner
def unused_two_levels(arg1, arg2):
def decorator(arg3, arg4):
def inner(arg5, arg6): ...
return inner
return decorator
def with_one_level(arg1):
def inner(arg2, arg3):
return arg1 + arg2 + arg3
return inner
def with_two_levels(arg1, arg2):
def decorator(arg3, arg4):
def inner(arg5, arg6):
return arg1 + arg2 + arg3 + arg4 + arg5 + arg6
return inner
return decorator

# Functions without closures:
self.assertIsNone(_testcapi.function_get_closure(regular_function))
self.assertIsNone(regular_function.__closure__)

func = unused_one_level(1)
closure = _testcapi.function_get_closure(func)
self.assertIsNone(closure)
self.assertIsNone(func.__closure__)

func = unused_two_levels(1, 2)(3, 4)
closure = _testcapi.function_get_closure(func)
self.assertIsNone(closure)
self.assertIsNone(func.__closure__)

# Functions with closures:
func = with_one_level(5)
closure = _testcapi.function_get_closure(func)
self.assertEqual(closure, func.__closure__)
self.assertIsInstance(closure, tuple)
self.assertEqual(len(closure), 1)
self.assertEqual(len(closure), len(func.__code__.co_freevars))
self.assertTrue(all(isinstance(cell, CellType) for cell in closure))
self.assertTrue(closure[0].cell_contents, 5)

func = with_two_levels(1, 2)(3, 4)
closure = _testcapi.function_get_closure(func)
self.assertEqual(closure, func.__closure__)
self.assertIsInstance(closure, tuple)
self.assertEqual(len(closure), 4)
self.assertEqual(len(closure), len(func.__code__.co_freevars))
self.assertTrue(all(isinstance(cell, CellType) for cell in closure))
self.assertEqual([cell.cell_contents for cell in closure],
[1, 2, 3, 4])

def test_function_get_closure_error(self):
with self.assertRaises(SystemError):
_testcapi.function_get_closure(1)
with self.assertRaises(SystemError):
_testcapi.function_get_closure(None)

def test_function_set_closure(self):
from types import CellType

def function_without_closure(): ...
def function_with_closure(arg):
def inner():
return arg
return inner

func = function_without_closure
_testcapi.function_set_closure(func, (CellType(1), CellType(1)))
closure = _testcapi.function_get_closure(func)
self.assertEqual([c.cell_contents for c in closure], [1, 1])
self.assertEqual([c.cell_contents for c in func.__closure__], [1, 1])

func = function_with_closure(1)
_testcapi.function_set_closure(func,
(CellType(1), CellType(2), CellType(3)))
closure = _testcapi.function_get_closure(func)
self.assertEqual([c.cell_contents for c in closure], [1, 2, 3])
self.assertEqual([c.cell_contents for c in func.__closure__], [1, 2, 3])

def test_function_set_closure_none(self):
def function_without_closure(): ...
def function_with_closure(arg):
def inner():
return arg
return inner

_testcapi.function_set_closure(function_without_closure, None)
self.assertIsNone(
_testcapi.function_get_closure(function_without_closure))
self.assertIsNone(function_without_closure.__closure__)

_testcapi.function_set_closure(function_with_closure, None)
self.assertIsNone(
_testcapi.function_get_closure(function_with_closure))
self.assertIsNone(function_with_closure.__closure__)

def test_function_set_closure_errors(self):
def function_without_closure(): ...

with self.assertRaises(SystemError):
_testcapi.function_set_closure(None, ()) # not a function

with self.assertRaises(SystemError):
_testcapi.function_set_closure(function_without_closure, 1)
self.assertIsNone(function_without_closure.__closure__) # no change

# NOTE: this works, but goes against the docs:
_testcapi.function_set_closure(function_without_closure, (1, 2))
self.assertEqual(
_testcapi.function_get_closure(function_without_closure), (1, 2))
self.assertEqual(function_without_closure.__closure__, (1, 2))


class TestPendingCalls(unittest.TestCase):

Expand Down
29 changes: 29 additions & 0 deletions Modules/_testcapimodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -6009,6 +6009,33 @@ function_set_kw_defaults(PyObject *self, PyObject *args)
Py_RETURN_NONE;
}

static PyObject *
function_get_closure(PyObject *self, PyObject *func)
{
PyObject *closure = PyFunction_GetClosure(func);
if (closure != NULL) {
Py_INCREF(closure);
return closure;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Py_INCREF(closure);
return closure;
return Py_NewRef(closure);

} else if (PyErr_Occurred()) {
return NULL;
} else {
Py_RETURN_NONE; // This can happen when `closure` is set to `None`
}
}

static PyObject *
function_set_closure(PyObject *self, PyObject *args)
{
PyObject *func = NULL, *closure = NULL;
if (!PyArg_ParseTuple(args, "OO", &func, &closure)) {
return NULL;
}
int result = PyFunction_SetClosure(func, closure);
if (result == -1)
return NULL;
Py_RETURN_NONE;
}


// type watchers

Expand Down Expand Up @@ -6434,6 +6461,8 @@ static PyMethodDef TestMethods[] = {
{"function_set_defaults", function_set_defaults, METH_VARARGS, NULL},
{"function_get_kw_defaults", function_get_kw_defaults, METH_O, NULL},
{"function_set_kw_defaults", function_set_kw_defaults, METH_VARARGS, NULL},
{"function_get_closure", function_get_closure, METH_O, NULL},
{"function_set_closure", function_set_closure, METH_VARARGS, NULL},
{"add_type_watcher", add_type_watcher, METH_O, NULL},
{"clear_type_watcher", clear_type_watcher, METH_O, NULL},
{"watch_type", watch_type, METH_VARARGS, NULL},
Expand Down