gh-117694: Improve tests for PyEval_EvalCodeEx() (GH-117695)

2 files changed, 114 insertions, 91 deletions

diff --git a/Lib/test/test_capi/test_eval_code_ex.py b/Lib/test/test_capi/test_eval_code_ex.py
index 2d28e52..b298e50 100644
--- a/Lib/test/test_capi/test_eval_code_ex.py
+++ b/Lib/test/test_capi/test_eval_code_ex.py
@@ -1,11 +1,16 @@
 import unittest
+import builtins
+from collections import UserDict
 
 from test.support import import_helper
+from test.support import swap_attr
 
 
 # Skip this test if the _testcapi module isn't available.
 _testcapi = import_helper.import_module('_testcapi')
 
+NULL = None
+
 
 class PyEval_EvalCodeExTests(unittest.TestCase):
 
@@ -13,43 +18,108 @@ class PyEval_EvalCodeExTests(unittest.TestCase):
         def f():
             return a
 
-        self.assertEqual(_testcapi.eval_code_ex(f.__code__, dict(a=1)), 1)
-
-    # Need to force the compiler to use LOAD_NAME
-    # def test_custom_locals(self):
-    #     def f():
-    #         return
+        eval_code_ex = _testcapi.eval_code_ex
+        code = f.__code__
+        self.assertEqual(eval_code_ex(code, dict(a=1)), 1)
+
+        self.assertRaises(NameError, eval_code_ex, code, {})
+        self.assertRaises(SystemError, eval_code_ex, code, UserDict(a=1))
+        self.assertRaises(SystemError, eval_code_ex, code, [])
+        self.assertRaises(SystemError, eval_code_ex, code, 1)
+        # CRASHES eval_code_ex(code, NULL)
+        # CRASHES eval_code_ex(1, {})
+        # CRASHES eval_code_ex(NULL, {})
+
+    def test_custom_locals(self):
+        # Monkey-patch __build_class__ to get a class code object.
+        code = None
+        def build_class(func, name, /, *bases, **kwds):
+            nonlocal code
+            code = func.__code__
+
+        with swap_attr(builtins, '__build_class__', build_class):
+            class A:
+                # Uses LOAD_NAME for a
+                r[:] = [a]
+
+        eval_code_ex = _testcapi.eval_code_ex
+        results = []
+        g = dict(a=1, r=results)
+        self.assertIsNone(eval_code_ex(code, g))
+        self.assertEqual(results, [1])
+        self.assertIsNone(eval_code_ex(code, g, dict(a=2)))
+        self.assertEqual(results, [2])
+        self.assertIsNone(eval_code_ex(code, g, UserDict(a=3)))
+        self.assertEqual(results, [3])
+        self.assertIsNone(eval_code_ex(code, g, {}))
+        self.assertEqual(results, [1])
+        self.assertIsNone(eval_code_ex(code, g, NULL))
+        self.assertEqual(results, [1])
+
+        self.assertRaises(TypeError, eval_code_ex, code, g, [])
+        self.assertRaises(TypeError, eval_code_ex, code, g, 1)
+        self.assertRaises(NameError, eval_code_ex, code, dict(r=results), {})
+        self.assertRaises(NameError, eval_code_ex, code, dict(r=results), NULL)
+        self.assertRaises(TypeError, eval_code_ex, code, dict(r=results), [])
+        self.assertRaises(TypeError, eval_code_ex, code, dict(r=results), 1)
 
     def test_with_args(self):
         def f(a, b, c):
             return a
 
-        self.assertEqual(_testcapi.eval_code_ex(f.__code__, {}, {}, (1, 2, 3)), 1)
+        eval_code_ex = _testcapi.eval_code_ex
+        code = f.__code__
+        self.assertEqual(eval_code_ex(code, {}, {}, (1, 2, 3)), 1)
+        self.assertRaises(TypeError, eval_code_ex, code, {}, {}, (1, 2))
+        self.assertRaises(TypeError, eval_code_ex, code, {}, {}, (1, 2, 3, 4))
 
     def test_with_kwargs(self):
         def f(a, b, c):
             return a
 
-        self.assertEqual(_testcapi.eval_code_ex(f.__code__, {}, {}, (), dict(a=1, b=2, c=3)), 1)
+        eval_code_ex = _testcapi.eval_code_ex
+        code = f.__code__
+        self.assertEqual(eval_code_ex(code, {}, {}, (), dict(a=1, b=2, c=3)), 1)
+        self.assertRaises(TypeError, eval_code_ex, code, {}, {}, (), dict(a=1, b=2))
+        self.assertRaises(TypeError, eval_code_ex, code, {}, {}, (), dict(a=1, b=2))
+        self.assertRaises(TypeError, eval_code_ex, code, {}, {}, (), dict(a=1, b=2, c=3, d=4))
 
     def test_with_default(self):
         def f(a):
             return a
 
-        self.assertEqual(_testcapi.eval_code_ex(f.__code__, {}, {}, (), {}, (1,)), 1)
+        eval_code_ex = _testcapi.eval_code_ex
+        code = f.__code__
+        self.assertEqual(eval_code_ex(code, {}, {}, (), {}, (1,)), 1)
+        self.assertRaises(TypeError, eval_code_ex, code, {}, {}, (), {}, ())
 
     def test_with_kwarg_default(self):
         def f(*, a):
             return a
 
-        self.assertEqual(_testcapi.eval_code_ex(f.__code__, {}, {}, (), {}, (), dict(a=1)), 1)
+        eval_code_ex = _testcapi.eval_code_ex
+        code = f.__code__
+        self.assertEqual(eval_code_ex(code, {}, {}, (), {}, (), dict(a=1)), 1)
+        self.assertRaises(TypeError, eval_code_ex, code, {}, {}, (), {}, (), {})
+        self.assertRaises(TypeError, eval_code_ex, code, {}, {}, (), {}, (), NULL)
+        self.assertRaises(SystemError, eval_code_ex, code, {}, {}, (), {}, (), UserDict(a=1))
+        self.assertRaises(SystemError, eval_code_ex, code, {}, {}, (), {}, (), [])
+        self.assertRaises(SystemError, eval_code_ex, code, {}, {}, (), {}, (), 1)
 
     def test_with_closure(self):
         a = 1
+        b = 2
         def f():
+            b
             return a
 
-        self.assertEqual(_testcapi.eval_code_ex(f.__code__, {}, {}, (), {}, (), {}, f.__closure__), 1)
+        eval_code_ex = _testcapi.eval_code_ex
+        code = f.__code__
+        self.assertEqual(eval_code_ex(code, {}, {}, (), {}, (), {}, f.__closure__), 1)
+        self.assertEqual(eval_code_ex(code, {}, {}, (), {}, (), {}, f.__closure__[::-1]), 2)
+
+        # CRASHES eval_code_ex(code, {}, {}, (), {}, (), {}, ()), 1)
+        # CRASHES eval_code_ex(code, {}, {}, (), {}, (), {}, NULL), 1)
 
 
 if __name__ == "__main__":
diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
index b2af47d..eff61dd 100644
--- a/Modules/_testcapimodule.c
+++ b/Modules/_testcapimodule.c
@@ -2645,107 +2645,60 @@ eval_eval_code_ex(PyObject *mod, PyObject *pos_args)
 
     PyObject **c_kwargs = NULL;
 
-    if (!PyArg_UnpackTuple(pos_args,
-                           "eval_code_ex",
-                           2,
-                           8,
-                           &code,
-                           &globals,
-                           &locals,
-                           &args,
-                           &kwargs,
-                           &defaults,
-                           &kw_defaults,
-                           &closure))
+    if (!PyArg_ParseTuple(pos_args,
+                          "OO|OO!O!O!OO:eval_code_ex",
+                          &code,
+                          &globals,
+                          &locals,
+                          &PyTuple_Type, &args,
+                          &PyDict_Type, &kwargs,
+                          &PyTuple_Type, &defaults,
+                          &kw_defaults,
+                          &closure))
     {
         goto exit;
     }
 
-    if (!PyCode_Check(code)) {
-        PyErr_SetString(PyExc_TypeError,
-                        "code must be a Python code object");
-        goto exit;
-    }
-
-    if (!PyDict_Check(globals)) {
-        PyErr_SetString(PyExc_TypeError, "globals must be a dict");
-        goto exit;
-    }
-
-    if (locals && !PyMapping_Check(locals)) {
-        PyErr_SetString(PyExc_TypeError, "locals must be a mapping");
-        goto exit;
-    }
-    if (locals == Py_None) {
-        locals = NULL;
-    }
+    NULLABLE(code);
+    NULLABLE(globals);
+    NULLABLE(locals);
+    NULLABLE(kw_defaults);
+    NULLABLE(closure);
 
     PyObject **c_args = NULL;
     Py_ssize_t c_args_len = 0;
-
-    if (args)
-    {
-        if (!PyTuple_Check(args)) {
-            PyErr_SetString(PyExc_TypeError, "args must be a tuple");
-            goto exit;
-        } else {
-            c_args = &PyTuple_GET_ITEM(args, 0);
-            c_args_len = PyTuple_Size(args);
-        }
+    if (args) {
+        c_args = &PyTuple_GET_ITEM(args, 0);
+        c_args_len = PyTuple_Size(args);
     }
 
     Py_ssize_t c_kwargs_len = 0;
+    if (kwargs) {
+        c_kwargs_len = PyDict_Size(kwargs);
+        if (c_kwargs_len > 0) {
+            c_kwargs = PyMem_NEW(PyObject*, 2 * c_kwargs_len);
+            if (!c_kwargs) {
+                PyErr_NoMemory();
+                goto exit;
+            }
 
-    if (kwargs)
-    {
-        if (!PyDict_Check(kwargs)) {
-            PyErr_SetString(PyExc_TypeError, "keywords must be a dict");
-            goto exit;
-        } else {
-            c_kwargs_len = PyDict_Size(kwargs);
-            if (c_kwargs_len > 0) {
-                c_kwargs = PyMem_NEW(PyObject*, 2 * c_kwargs_len);
-                if (!c_kwargs) {
-                    PyErr_NoMemory();
-                    goto exit;
-                }
-
-                Py_ssize_t i = 0;
-                Py_ssize_t pos = 0;
-
-                while (PyDict_Next(kwargs,
-                                   &pos,
-                                   &c_kwargs[i],
-                                   &c_kwargs[i + 1]))
-                {
-                    i += 2;
-                }
-                c_kwargs_len = i / 2;
-                /* XXX This is broken if the caller deletes dict items! */
+            Py_ssize_t i = 0;
+            Py_ssize_t pos = 0;
+            while (PyDict_Next(kwargs, &pos, &c_kwargs[i], &c_kwargs[i + 1])) {
+                i += 2;
             }
+            c_kwargs_len = i / 2;
+            /* XXX This is broken if the caller deletes dict items! */
         }
     }
 
-
     PyObject **c_defaults = NULL;
     Py_ssize_t c_defaults_len = 0;
-
-    if (defaults && PyTuple_Check(defaults)) {
+    if (defaults) {
         c_defaults = &PyTuple_GET_ITEM(defaults, 0);
         c_defaults_len = PyTuple_Size(defaults);
     }
 
-    if (kw_defaults && !PyDict_Check(kw_defaults)) {
-        PyErr_SetString(PyExc_TypeError, "kw_defaults must be a dict");
-        goto exit;
-    }
-
-    if (closure && !PyTuple_Check(closure)) {
-        PyErr_SetString(PyExc_TypeError, "closure must be a tuple of cells");
-        goto exit;
-    }
-
-
     result = PyEval_EvalCodeEx(
         code,
         globals,