From 6af2ec3666905f8e17a8741526b4d87e797fb0c8 Mon Sep 17 00:00:00 2001 From: Eddy Xu Date: Tue, 7 Apr 2026 18:43:38 -0400 Subject: [PATCH 1/7] add warning for scope issue of exec --- Lib/test/test_py3kwarn.py | 46 +++++++ Python/ceval.c | 278 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 324 insertions(+) diff --git a/Lib/test/test_py3kwarn.py b/Lib/test/test_py3kwarn.py index afd3c1b420bce3..bc0cf15c997cdb 100644 --- a/Lib/test/test_py3kwarn.py +++ b/Lib/test/test_py3kwarn.py @@ -430,6 +430,52 @@ def test_b16encode_warns(self): base64.b16encode(b'test') check_py3k_warnings(expected, UserWarning) + def assertExecLocalWritebackWarning(self, recorder, local_name): + self.assertEqual(len(recorder.warnings), 1) + msg = str(recorder.warnings[0].message) + self.assertTrue(msg.startswith( + "exec() modified local '%s'" % local_name)) + recorder.reset() + + def test_exec_local_writeback_warning(self): + def f_exec(code): + b = 42 + exec code + return b + + with check_py3k_warnings() as w: + f_exec("b = 99") + self.assertExecLocalWritebackWarning(w, "b") + + def f_exec_no_write(code): + b = 42 + exec code + return b + + with check_py3k_warnings() as w: + f_exec_no_write("print b") + self.assertEqual(len(w.warnings), 0) + + def f_exec_overwrite(code): + b = 42 + exec code + b = 7 + return b + + with check_py3k_warnings() as w: + f_exec_overwrite("b = 99") + self.assertEqual(len(w.warnings), 0) + + def f_exec_explicit(code): + b = 42 + ns = {'b': b} + exec code in globals(), ns + return b + + with check_py3k_warnings() as w: + f_exec_explicit("b = 99") + self.assertEqual(len(w.warnings), 0) + class TestStdlibRemovals(unittest.TestCase): diff --git a/Python/ceval.c b/Python/ceval.c index 2af1ef7a42fed6..582acc5f7a425a 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -66,6 +66,184 @@ _Py3kWarn_NextOpcode(void) return -1; } +static PyObject *exec_local_writeback_map = NULL; + +static PyObject * +_get_exec_local_writeback_map(void) +{ + if (exec_local_writeback_map == NULL) { + exec_local_writeback_map = PyDict_New(); + } + return exec_local_writeback_map; +} + +static void +_clear_exec_local_writeback_for_frame(PyFrameObject *f) +{ + PyObject *frame_key; + + if (exec_local_writeback_map == NULL) { + return; + } + frame_key = PyLong_FromVoidPtr(f); + if (frame_key == NULL) { + PyErr_Clear(); + return; + } + if (PyDict_DelItem(exec_local_writeback_map, frame_key) < 0) { + PyErr_Clear(); + } + Py_DECREF(frame_key); +} + +static int +_warn_exec_local_writeback(PyFrameObject *f, int oparg, int opcode) +{ + PyObject *frame_key = NULL; + PyObject *frame_dict = NULL; + PyObject *entry = NULL; + PyObject *idx = NULL; + PyObject *msg = NULL; + PyObject *name_obj = NULL; + const char *local_name = NULL; + const char *func_name = NULL; + int exec_lineno = 0; + int exec_offset = 0; + int read_lineno = 0; + int read_offset = f->f_lasti; + int warn_result; + + if (exec_local_writeback_map == NULL) { + return 0; + } + frame_key = PyLong_FromVoidPtr(f); + if (frame_key == NULL) { + PyErr_Clear(); + return 0; + } + frame_dict = PyDict_GetItem(exec_local_writeback_map, frame_key); + if (frame_dict == NULL) { + Py_DECREF(frame_key); + return 0; + } + idx = PyInt_FromLong(oparg); + if (idx == NULL) { + Py_DECREF(frame_key); + PyErr_Clear(); + return 0; + } + entry = PyDict_GetItem(frame_dict, idx); + if (entry == NULL) { + Py_DECREF(idx); + Py_DECREF(frame_key); + return 0; + } + if (PyTuple_Check(entry) && PyTuple_GET_SIZE(entry) == 2) { + exec_lineno = (int)PyInt_AsLong(PyTuple_GET_ITEM(entry, 0)); + exec_offset = (int)PyInt_AsLong(PyTuple_GET_ITEM(entry, 1)); + } + if (exec_lineno < 0) { + exec_lineno = 0; + } + read_lineno = PyCode_Addr2Line(f->f_code, f->f_lasti); + name_obj = PyTuple_GetItem(f->f_code->co_varnames, oparg); + if (name_obj != NULL) { + local_name = PyString_AsString(name_obj); + } + if (local_name == NULL) { + local_name = ""; + } + if (f->f_code->co_name != NULL) { + func_name = PyString_AsString(f->f_code->co_name); + } + if (func_name == NULL) { + func_name = ""; + } + msg = PyString_FromFormat( + "exec() modified local '%s' which is read later in the enclosing function; " + "in 3.x exec() does not reliably write back to function locals without an explicit locals mapping" + "\nPYGRATE_META: {\"warning_type\":\"EXEC_LOCAL_WRITEBACK_WARNING\"," + "\"scope_kind\":\"function\",\"has_explicit_globals\":false," + "\"has_explicit_locals\":false,\"local_name\":\"%s\"," + "\"exec_lineno\":%d,\"read_lineno\":%d,\"read_opcode\":%d," + "\"exec_offset\":%d,\"read_offset\":%d,\"function_name\":\"%s\"}", + local_name, local_name, exec_lineno, read_lineno, opcode, + exec_offset, read_offset, func_name); + if (msg == NULL) { + Py_DECREF(idx); + Py_DECREF(frame_key); + PyErr_Clear(); + return 0; + } + + warn_result = PyErr_WarnExplicit_WithFix( + PyExc_Py3xWarning, + PyString_AsString(msg), + "use exec(code, globals, locals) with an explicit locals mapping", + PyString_AsString(f->f_code->co_filename), + read_lineno, + NULL, + NULL); + + Py_DECREF(msg); + if (warn_result < 0) { + Py_DECREF(idx); + Py_DECREF(frame_key); + return -1; + } + + if (PyDict_DelItem(frame_dict, idx) < 0) { + PyErr_Clear(); + } + Py_DECREF(idx); + + if (PyDict_Size(frame_dict) <= 0) { + if (PyDict_DelItem(exec_local_writeback_map, frame_key) < 0) { + PyErr_Clear(); + } + } + Py_DECREF(frame_key); + return 0; +} + +static void +_clear_exec_local_writeback_for_local(PyFrameObject *f, int oparg) +{ + PyObject *frame_key = NULL; + PyObject *frame_dict = NULL; + PyObject *idx = NULL; + + if (exec_local_writeback_map == NULL) { + return; + } + frame_key = PyLong_FromVoidPtr(f); + if (frame_key == NULL) { + PyErr_Clear(); + return; + } + frame_dict = PyDict_GetItem(exec_local_writeback_map, frame_key); + if (frame_dict == NULL) { + Py_DECREF(frame_key); + return; + } + idx = PyInt_FromLong(oparg); + if (idx == NULL) { + Py_DECREF(frame_key); + PyErr_Clear(); + return; + } + if (PyDict_DelItem(frame_dict, idx) < 0) { + PyErr_Clear(); + } + Py_DECREF(idx); + if (PyDict_Size(frame_dict) <= 0) { + if (PyDict_DelItem(exec_local_writeback_map, frame_key) < 0) { + PyErr_Clear(); + } + } + Py_DECREF(frame_key); +} + #ifndef WITH_TSC #define READ_TIMESTAMP(var) @@ -1273,6 +1451,12 @@ PyEval_EvalFrameEx(PyFrameObject *f, int throwflag) { x = GETLOCAL(oparg); if (x != NULL) { + if (Py_Py3kWarningFlag) { + if (_warn_exec_local_writeback(f, oparg, opcode) < 0) { + err = -1; + break; + } + } Py_INCREF(x); PUSH(x); FAST_DISPATCH(); @@ -1296,6 +1480,9 @@ PyEval_EvalFrameEx(PyFrameObject *f, int throwflag) { v = POP(); SETLOCAL(oparg, v); + if (Py_Py3kWarningFlag) { + _clear_exec_local_writeback_for_local(f, oparg); + } FAST_DISPATCH(); } @@ -2454,6 +2641,9 @@ PyEval_EvalFrameEx(PyFrameObject *f, int throwflag) x = GETLOCAL(oparg); if (x != NULL) { SETLOCAL(oparg, NULL); + if (Py_Py3kWarningFlag) { + _clear_exec_local_writeback_for_local(f, oparg); + } DISPATCH(); } format_exc_check_arg( @@ -3404,6 +3594,7 @@ PyEval_EvalFrameEx(PyFrameObject *f, int throwflag) /* pop frame */ exit_eval_frame: + _clear_exec_local_writeback_for_frame(f); Py_LeaveRecursiveCall(); tstate->frame = f->f_back; @@ -5086,6 +5277,12 @@ exec_statement(PyFrameObject *f, PyObject *prog, PyObject *globals, int n; PyObject *v; int plain = 0; + int track_locals = 0; + int exec_lineno = 0; + int exec_offset = 0; + int nlocals = 0; + PyObject **before = NULL; + int i; if (PyTuple_Check(prog) && globals == Py_None && locals == Py_None && ((n = PyTuple_Size(prog)) == 2 || n == 3)) { @@ -5131,6 +5328,23 @@ exec_statement(PyFrameObject *f, PyObject *prog, PyObject *globals, } if (PyDict_GetItemString(globals, "__builtins__") == NULL) PyDict_SetItemString(globals, "__builtins__", f->f_builtins); + + if (plain && Py_Py3kWarningFlag && + (f->f_code->co_flags & CO_NEWLOCALS) && + f->f_code->co_nlocals > 0 && + f->f_localsplus != NULL) { + nlocals = f->f_code->co_nlocals; + before = PyMem_New(PyObject *, nlocals); + if (before != NULL) { + for (i = 0; i < nlocals; i++) { + before[i] = f->f_localsplus[i]; + Py_XINCREF(before[i]); + } + track_locals = 1; + exec_offset = f->f_lasti; + exec_lineno = PyCode_Addr2Line(f->f_code, exec_offset); + } + } if (PyCode_Check(prog)) { if (PyCode_GetNumFree((PyCodeObject *)prog) > 0) { PyErr_SetString(PyExc_TypeError, @@ -5178,6 +5392,70 @@ exec_statement(PyFrameObject *f, PyObject *prog, PyObject *globals, } if (plain) PyFrame_LocalsToFast(f, 0); + if (track_locals && v != NULL) { + PyObject *frame_key = NULL; + PyObject *frame_dict = NULL; + PyObject *map = NULL; + for (i = 0; i < nlocals; i++) { + PyObject *before_obj = before[i]; + PyObject *after_obj = f->f_localsplus[i]; + if (before_obj == after_obj) { + continue; + } + if (frame_key == NULL) { + frame_key = PyLong_FromVoidPtr(f); + if (frame_key == NULL) { + PyErr_Clear(); + break; + } + } + if (map == NULL) { + map = _get_exec_local_writeback_map(); + if (map == NULL) { + PyErr_Clear(); + break; + } + } + if (frame_dict == NULL) { + frame_dict = PyDict_GetItem(map, frame_key); + if (frame_dict == NULL) { + frame_dict = PyDict_New(); + if (frame_dict == NULL) { + PyErr_Clear(); + break; + } + if (PyDict_SetItem(map, frame_key, frame_dict) < 0) { + PyErr_Clear(); + Py_DECREF(frame_dict); + frame_dict = NULL; + break; + } + Py_DECREF(frame_dict); + frame_dict = PyDict_GetItem(map, frame_key); + } + } + if (frame_dict != NULL) { + PyObject *idx = PyInt_FromLong(i); + PyObject *val = Py_BuildValue("ii", exec_lineno, exec_offset); + if (idx != NULL && val != NULL) { + if (PyDict_SetItem(frame_dict, idx, val) < 0) { + PyErr_Clear(); + } + } else { + PyErr_Clear(); + } + Py_XDECREF(idx); + Py_XDECREF(val); + } + } + Py_XDECREF(frame_key); + } + if (before != NULL) { + for (i = 0; i < nlocals; i++) { + Py_XDECREF(before[i]); + } + PyMem_Free(before); + } if (v == NULL) return -1; Py_DECREF(v); From 79e9686b880c023541e942b236a523edcd89e9fa Mon Sep 17 00:00:00 2001 From: Eddy Xu Date: Wed, 8 Apr 2026 00:35:23 -0400 Subject: [PATCH 2/7] mro --- Lib/test/test_py3kwarn.py | 159 +++++++++- Objects/classobject.c | 592 +++++++++++++++++++++++++++++++++++++- test_mro.py | 44 +++ 3 files changed, 790 insertions(+), 5 deletions(-) create mode 100644 test_mro.py diff --git a/Lib/test/test_py3kwarn.py b/Lib/test/test_py3kwarn.py index bc0cf15c997cdb..d6e89e287bb0e7 100644 --- a/Lib/test/test_py3kwarn.py +++ b/Lib/test/test_py3kwarn.py @@ -1,5 +1,6 @@ import unittest import sys +import contextlib from test.test_support import check_py3k_warnings, CleanImport, run_unittest import warnings import base64 @@ -35,6 +36,23 @@ def reset_module_registry(module): class TestPy3KWarnings(unittest.TestCase): + @contextlib.contextmanager + def check_py3k_warnings_with_fix(self): + frame = sys._getframe(2) + registry = frame.f_globals.get('__warningregistry__') + if registry: + registry.clear() + with warnings.catch_warnings(record=True) as w: + showwarningwithfix = warnings.showwarningwithfix + def record_warning_with_fix(*args, **kwargs): + w.append(warnings.WarningMessageWithFix(*args, **kwargs)) + warnings.showwarningwithfix = record_warning_with_fix + warnings.simplefilter("always") + try: + yield test_support.WarningsRecorder(w) + finally: + warnings.showwarningwithfix = showwarningwithfix + def assertWarning(self, _, warning, expected_message): self.assertEqual(str(warning.message), expected_message) @@ -437,13 +455,29 @@ def assertExecLocalWritebackWarning(self, recorder, local_name): "exec() modified local '%s'" % local_name)) recorder.reset() + def assertMROWarning(self, recorder, risk_kind, class_name, + attr_name=None, classic_provider=None, + c3_provider=None): + self.assertEqual(len(recorder.warnings), 1) + msg = str(recorder.warnings[0].message) + self.assertIn('"warning_type":"MRO_RISK_WARNING"', msg) + self.assertIn('"risk_kind":"%s"' % risk_kind, msg) + self.assertIn('"class_name":"%s"' % class_name, msg) + if attr_name is not None: + self.assertIn('"attr_name":"%s"' % attr_name, msg) + if classic_provider is not None: + self.assertIn('"classic_provider":"%s"' % classic_provider, msg) + if c3_provider is not None: + self.assertIn('"c3_provider":"%s"' % c3_provider, msg) + recorder.reset() + def test_exec_local_writeback_warning(self): def f_exec(code): b = 42 exec code return b - with check_py3k_warnings() as w: + with self.check_py3k_warnings_with_fix() as w: f_exec("b = 99") self.assertExecLocalWritebackWarning(w, "b") @@ -452,7 +486,7 @@ def f_exec_no_write(code): exec code return b - with check_py3k_warnings() as w: + with self.check_py3k_warnings_with_fix() as w: f_exec_no_write("print b") self.assertEqual(len(w.warnings), 0) @@ -462,7 +496,7 @@ def f_exec_overwrite(code): b = 7 return b - with check_py3k_warnings() as w: + with self.check_py3k_warnings_with_fix() as w: f_exec_overwrite("b = 99") self.assertEqual(len(w.warnings), 0) @@ -472,10 +506,127 @@ def f_exec_explicit(code): exec code in globals(), ns return b - with check_py3k_warnings() as w: + with self.check_py3k_warnings_with_fix() as w: f_exec_explicit("b = 99") self.assertEqual(len(w.warnings), 0) + def test_classic_mro_resolution_change_warning(self): + with self.check_py3k_warnings_with_fix() as w: + class A: + def do_this(self): + return "A" + + class B(A): + pass + + class C(A): + def do_this(self): + return "C" + + class D(B, C): + pass + + self.assertEqual(D().do_this(), "A") + self.assertMROWarning( + w, "resolution_change", "D", + attr_name="do_this", + classic_provider="A", + c3_provider="C") + + def test_classic_mro_single_inheritance_no_warning(self): + with self.check_py3k_warnings_with_fix() as w: + class A: + def only_here(self): + return "A" + + class B(A): + pass + + self.assertEqual(B().only_here(), "A") + self.assertEqual(len(w.warnings), 0) + + def test_classic_mro_no_conflicting_name_no_warning(self): + with self.check_py3k_warnings_with_fix() as w: + class A: + pass + + class B(A): + def left(self): + return "left" + + class C(A): + def right(self): + return "right" + + class D(B, C): + pass + + self.assertEqual(D().left(), "left") + self.assertEqual(D().right(), "right") + self.assertEqual(len(w.warnings), 0) + + def test_classic_mro_provider_unchanged_no_warning(self): + with self.check_py3k_warnings_with_fix() as w: + class A: + def only_here(self): + return "A" + + class B(A): + pass + + class C(A): + pass + + class D(B, C): + pass + + self.assertEqual(D().only_here(), "A") + self.assertEqual(len(w.warnings), 0) + + def test_classic_mro_c3_conflict_warning(self): + with self.check_py3k_warnings_with_fix() as w: + class A: + pass + + class B: + pass + + class X(A, B): + pass + + class Y(B, A): + pass + + class Z(X, Y): + pass + + self.assertMROWarning(w, "c3_conflict", "Z") + + def test_classic_mro_bases_update_warning(self): + with self.check_py3k_warnings_with_fix() as w: + class A: + def do_this(self): + return "A" + + class B(A): + pass + + class C(A): + def do_this(self): + return "C" + + class D(B): + pass + + self.assertEqual(len(w.warnings), 0) + D.__bases__ = (B, C) + self.assertEqual(D().do_this(), "A") + self.assertMROWarning( + w, "resolution_change", "D", + attr_name="do_this", + classic_provider="A", + c3_provider="C") + class TestStdlibRemovals(unittest.TestCase): diff --git a/Objects/classobject.c b/Objects/classobject.c index 02d7cfd019b910..693de47b7e3129 100644 --- a/Objects/classobject.c +++ b/Objects/classobject.c @@ -2,6 +2,7 @@ /* Class object implementation */ #include "Python.h" +#include "frameobject.h" #include "structmember.h" /* Free list for method objects to save malloc/free overhead @@ -24,6 +25,583 @@ static PyObject *instance_getattr2(PyInstanceObject *, PyObject *); static PyObject *getattrstr, *setattrstr, *delattrstr; +/* MRO warning helpers */ +static int +warn_classic_mro_risk(PyClassObject *klass); + +static PyObject * +classic_mro_list(PyClassObject *klass); + +static PyObject * +hypothetical_c3_mro(PyClassObject *klass, PyObject **error_text); + + +/* ---------- classic MRO / C3 risk analysis ---------- */ + +static int +mro_noise_name(PyObject *name) +{ + char *s; + if (name == NULL || !PyString_Check(name)) + return 1; + s = PyString_AS_STRING(name); + return ( + strcmp(s, "__module__") == 0 || + strcmp(s, "__doc__") == 0 || + strcmp(s, "__name__") == 0 || + strcmp(s, "__dict__") == 0 || + strcmp(s, "__weakref__") == 0 + ); +} + +static char * +class_name_cstr(PyClassObject *klass) +{ + if (klass != NULL && klass->cl_name != NULL && + PyString_Check(klass->cl_name)) + return PyString_AS_STRING(klass->cl_name); + return "?"; +} + +static int +list_contains_ptr(PyObject *list, PyObject *obj) +{ + Py_ssize_t i, n; + n = PyList_GET_SIZE(list); + for (i = 0; i < n; i++) { + if (PyList_GET_ITEM(list, i) == obj) + return 1; + } + return 0; +} + +static int +list_append_unique_ptr(PyObject *list, PyObject *obj) +{ + if (list_contains_ptr(list, obj)) + return 0; + return PyList_Append(list, obj); +} + +static int +classic_mro_dfs(PyClassObject *klass, PyObject *out) +{ + Py_ssize_t i, n; + if (list_append_unique_ptr(out, (PyObject *)klass) < 0) + return -1; + + n = PyTuple_GET_SIZE(klass->cl_bases); + for (i = 0; i < n; i++) { + PyObject *base = PyTuple_GET_ITEM(klass->cl_bases, i); + if (!PyClass_Check(base)) + continue; + if (classic_mro_dfs((PyClassObject *)base, out) < 0) + return -1; + } + return 0; +} + +static PyObject * +classic_mro_list(PyClassObject *klass) +{ + PyObject *out = PyList_New(0); + if (out == NULL) + return NULL; + if (classic_mro_dfs(klass, out) < 0) { + Py_DECREF(out); + return NULL; + } + return out; +} + +static int +c3_head_in_tails(PyObject *seqs, PyObject *candidate) +{ + Py_ssize_t i, j, nseqs; + nseqs = PyList_GET_SIZE(seqs); + for (i = 0; i < nseqs; i++) { + PyObject *seq = PyList_GET_ITEM(seqs, i); + Py_ssize_t n = PyList_GET_SIZE(seq); + for (j = 1; j < n; j++) { + if (PyList_GET_ITEM(seq, j) == candidate) + return 1; + } + } + return 0; +} + +static PyObject * +c3_merge(PyObject *seqs, PyObject **error_text) +{ + PyObject *result = NULL; + int progress = 1; + + result = PyList_New(0); + if (result == NULL) + return NULL; + + while (progress) { + Py_ssize_t i, nseqs; + int nonempty = 0; + + progress = 0; + nseqs = PyList_GET_SIZE(seqs); + + for (i = 0; i < nseqs; i++) { + PyObject *seq = PyList_GET_ITEM(seqs, i); + Py_ssize_t n = PyList_GET_SIZE(seq); + PyObject *cand; + + if (n == 0) + continue; + + nonempty = 1; + cand = PyList_GET_ITEM(seq, 0); + + if (!c3_head_in_tails(seqs, cand)) { + Py_ssize_t j; + if (PyList_Append(result, cand) < 0) { + Py_DECREF(result); + return NULL; + } + for (j = 0; j < nseqs; j++) { + PyObject *s = PyList_GET_ITEM(seqs, j); + if (PyList_GET_SIZE(s) > 0 && + PyList_GET_ITEM(s, 0) == cand) { + if (PySequence_DelItem(s, 0) < 0) { + Py_DECREF(result); + return NULL; + } + } + } + progress = 1; + break; + } + } + + if (!nonempty) + return result; + + if (!progress) { + if (error_text != NULL && *error_text == NULL) { + *error_text = PyString_FromString( + "no consistent C3 MRO for classic hierarchy"); + } + Py_DECREF(result); + return NULL; + } + } + + return result; +} + +static PyObject * +hypothetical_c3_mro(PyClassObject *klass, PyObject **error_text) +{ + PyObject *result = NULL; + PyObject *seqs = NULL; + PyObject *bases_list = NULL; + PyObject *merged = NULL; + Py_ssize_t i, n; + + result = PyList_New(0); + if (result == NULL) + goto error; + + if (PyList_Append(result, (PyObject *)klass) < 0) + goto error; + + seqs = PyList_New(0); + if (seqs == NULL) + goto error; + + bases_list = PyList_New(0); + if (bases_list == NULL) + goto error; + + n = PyTuple_GET_SIZE(klass->cl_bases); + for (i = 0; i < n; i++) { + PyObject *base = PyTuple_GET_ITEM(klass->cl_bases, i); + PyObject *base_mro; + + if (!PyClass_Check(base)) + continue; + + if (PyList_Append(bases_list, base) < 0) + goto error; + + base_mro = hypothetical_c3_mro((PyClassObject *)base, error_text); + if (base_mro == NULL) + goto error; + + if (PyList_Append(seqs, base_mro) < 0) { + Py_DECREF(base_mro); + goto error; + } + Py_DECREF(base_mro); + } + + if (PyList_Append(seqs, bases_list) < 0) + goto error; + + merged = c3_merge(seqs, error_text); + if (merged == NULL) + goto error; + + n = PyList_GET_SIZE(merged); + for (i = 0; i < n; i++) { + PyObject *item = PyList_GET_ITEM(merged, i); + if (PyList_Append(result, item) < 0) + goto error; + } + + Py_DECREF(merged); + Py_DECREF(bases_list); + Py_DECREF(seqs); + return result; + +error: + Py_XDECREF(merged); + Py_XDECREF(bases_list); + Py_XDECREF(seqs); + Py_XDECREF(result); + return NULL; +} + +static PyObject * +join_class_names_from_seq(PyObject *seq) +{ + PyObject *parts = NULL, *sep = NULL, *res = NULL; + Py_ssize_t i, n; + + n = PySequence_Size(seq); + if (n < 0) + return NULL; + + parts = PyList_New(0); + if (parts == NULL) + return NULL; + + for (i = 0; i < n; i++) { + PyObject *item = PySequence_GetItem(seq, i); + PyObject *name = NULL; + + if (item == NULL) + goto error; + + if (PyClass_Check(item) && + ((PyClassObject *)item)->cl_name != NULL && + PyString_Check(((PyClassObject *)item)->cl_name)) { + name = ((PyClassObject *)item)->cl_name; + Py_INCREF(name); + } + else { + name = PyString_FromString("?"); + } + + Py_DECREF(item); + + if (name == NULL) + goto error; + if (PyList_Append(parts, name) < 0) { + Py_DECREF(name); + goto error; + } + Py_DECREF(name); + } + + sep = PyString_FromString("|"); + if (sep == NULL) + goto error; + + res = PyObject_CallMethod(sep, "join", "O", parts); + + Py_DECREF(sep); + Py_DECREF(parts); + return res; + +error: + Py_XDECREF(sep); + Py_XDECREF(parts); + return NULL; +} + +static PyClassObject * +c3_provider_for_name(PyObject *c3_mro, PyObject *name) +{ + Py_ssize_t i, n; + n = PyList_GET_SIZE(c3_mro); + for (i = 0; i < n; i++) { + PyClassObject *cp = (PyClassObject *)PyList_GET_ITEM(c3_mro, i); + if (PyDict_GetItem(cp->cl_dict, name) != NULL) + return cp; + } + return NULL; +} + +static PyObject * +duplicate_candidate_names(PyObject *classic_mro) +{ + PyObject *seen = NULL, *dups = NULL, *keys = NULL; + Py_ssize_t i, n; + + seen = PyDict_New(); + dups = PyDict_New(); + if (seen == NULL || dups == NULL) + goto error; + + n = PyList_GET_SIZE(classic_mro); + for (i = 0; i < n; i++) { + PyClassObject *cp = (PyClassObject *)PyList_GET_ITEM(classic_mro, i); + Py_ssize_t pos = 0; + PyObject *k, *v; + + while (PyDict_Next(cp->cl_dict, &pos, &k, &v)) { + int contains; + + if (mro_noise_name(k)) + continue; + + contains = PyDict_Contains(seen, k); + if (contains < 0) + goto error; + + if (contains) { + if (PyDict_SetItem(dups, k, Py_True) < 0) + goto error; + } + else { + if (PyDict_SetItem(seen, k, Py_True) < 0) + goto error; + } + } + } + + keys = PyDict_Keys(dups); + if (keys == NULL) + goto error; + if (PyList_Sort(keys) < 0) + goto error; + + Py_DECREF(seen); + Py_DECREF(dups); + return keys; + +error: + Py_XDECREF(keys); + Py_XDECREF(seen); + Py_XDECREF(dups); + return NULL; +} + +static int +warn_mro_resolution_change(PyClassObject *klass, + PyObject *name, + PyClassObject *classic_provider, + PyClassObject *c3_provider, + PyObject *classic_mro, + PyObject *c3_mro) +{ + PyFrameObject *f = PyEval_GetFrame(); + const char *filename = ""; + int lineno = 0; + PyObject *base_names = NULL, *classic_names = NULL, *c3_names = NULL; + PyObject *msg = NULL; + int rc; + + if (f != NULL && f->f_code != NULL && + f->f_code->co_filename != NULL && + PyString_Check(f->f_code->co_filename)) { + filename = PyString_AS_STRING(f->f_code->co_filename); + lineno = PyCode_Addr2Line(f->f_code, f->f_lasti); + } + + base_names = join_class_names_from_seq(klass->cl_bases); + classic_names = join_class_names_from_seq(classic_mro); + c3_names = join_class_names_from_seq(c3_mro); + if (base_names == NULL || classic_names == NULL || c3_names == NULL) + goto error; + + msg = PyString_FromFormat( + "classic multiple inheritance for class '%s' may resolve attribute '%s' " + "from '%s' in 2.x but from '%s' in 3.x due to C3 MRO" + "\nPYGRATE_META: {\"warning_type\":\"MRO_RISK_WARNING\"," + "\"risk_kind\":\"resolution_change\"," + "\"class_name\":\"%s\"," + "\"base_names\":\"%s\"," + "\"attr_name\":\"%s\"," + "\"classic_provider\":\"%s\"," + "\"c3_provider\":\"%s\"," + "\"classic_mro\":\"%s\"," + "\"c3_mro\":\"%s\"}", + class_name_cstr(klass), + PyString_AsString(name), + class_name_cstr(classic_provider), + class_name_cstr(c3_provider), + class_name_cstr(klass), + PyString_AsString(base_names), + PyString_AsString(name), + class_name_cstr(classic_provider), + class_name_cstr(c3_provider), + PyString_AsString(classic_names), + PyString_AsString(c3_names)); + if (msg == NULL) + goto error; + + rc = PyErr_WarnExplicit_WithFix( + PyExc_Py3xWarning, + PyString_AsString(msg), + "make the hierarchy new-style and review the conflicting attribute resolution", + filename, + lineno, + NULL, + NULL); + + Py_DECREF(base_names); + Py_DECREF(classic_names); + Py_DECREF(c3_names); + Py_DECREF(msg); + return rc; + +error: + Py_XDECREF(base_names); + Py_XDECREF(classic_names); + Py_XDECREF(c3_names); + Py_XDECREF(msg); + return -1; +} + +static int +warn_mro_c3_conflict(PyClassObject *klass, PyObject *error_text) +{ + PyFrameObject *f = PyEval_GetFrame(); + const char *filename = ""; + int lineno = 0; + PyObject *base_names = NULL, *msg = NULL; + int rc; + + if (f != NULL && f->f_code != NULL && + f->f_code->co_filename != NULL && + PyString_Check(f->f_code->co_filename)) { + filename = PyString_AS_STRING(f->f_code->co_filename); + lineno = PyCode_Addr2Line(f->f_code, f->f_lasti); + } + + base_names = join_class_names_from_seq(klass->cl_bases); + if (base_names == NULL) + goto error; + + msg = PyString_FromFormat( + "classic multiple inheritance hierarchy for class '%s' has no " + "consistent C3 MRO and will fail in 3.x" + "\nPYGRATE_META: {\"warning_type\":\"MRO_RISK_WARNING\"," + "\"risk_kind\":\"c3_conflict\"," + "\"class_name\":\"%s\"," + "\"base_names\":\"%s\"," + "\"c3_error\":\"%s\"}", + class_name_cstr(klass), + class_name_cstr(klass), + PyString_AsString(base_names), + (error_text != NULL && PyString_Check(error_text)) + ? PyString_AsString(error_text) + : "no consistent C3 MRO"); + if (msg == NULL) + goto error; + + rc = PyErr_WarnExplicit_WithFix( + PyExc_Py3xWarning, + PyString_AsString(msg), + "make the hierarchy new-style and resolve the inconsistent base ordering", + filename, + lineno, + NULL, + NULL); + + Py_DECREF(base_names); + Py_DECREF(msg); + return rc; + +error: + Py_XDECREF(base_names); + Py_XDECREF(msg); + return -1; +} + +static int +warn_classic_mro_risk(PyClassObject *klass) +{ + PyObject *classic_mro = NULL; + PyObject *c3_mro = NULL; + PyObject *c3_error = NULL; + PyObject *candidates = NULL; + Py_ssize_t i, n; + + if (!Py_Py3kWarningFlag) + return 0; + + if (klass == NULL) + return 0; + + if (PyTuple_GET_SIZE(klass->cl_bases) < 2) + return 0; + + classic_mro = classic_mro_list(klass); + if (classic_mro == NULL) + goto error; + + c3_mro = hypothetical_c3_mro(klass, &c3_error); + if (c3_mro == NULL) { + if (c3_error != NULL) { + int rc = warn_mro_c3_conflict(klass, c3_error); + Py_DECREF(classic_mro); + Py_DECREF(c3_error); + return rc; + } + goto error; + } + + candidates = duplicate_candidate_names(classic_mro); + if (candidates == NULL) + goto error; + + n = PyList_GET_SIZE(candidates); + for (i = 0; i < n; i++) { + PyObject *name = PyList_GET_ITEM(candidates, i); + PyClassObject *classic_provider = NULL; + PyClassObject *c3_provider = NULL; + + if (class_lookup(klass, name, &classic_provider) == NULL) + continue; + c3_provider = c3_provider_for_name(c3_mro, name); + + if (classic_provider != NULL && + c3_provider != NULL && + classic_provider != c3_provider) { + int rc = warn_mro_resolution_change( + klass, name, classic_provider, c3_provider, + classic_mro, c3_mro); + Py_DECREF(classic_mro); + Py_DECREF(c3_mro); + Py_DECREF(candidates); + return rc; + } + } + + Py_DECREF(classic_mro); + Py_DECREF(c3_mro); + Py_DECREF(candidates); + return 0; + +error: + Py_XDECREF(classic_mro); + Py_XDECREF(c3_mro); + Py_XDECREF(c3_error); + Py_XDECREF(candidates); + return -1; +} + +/* ---------- end classic MRO / C3 risk analysis ---------- */ + PyObject * PyClass_New(PyObject *bases, PyObject *dict, PyObject *name) @@ -132,6 +710,12 @@ PyClass_New(PyObject *bases, PyObject *dict, PyObject *name) Py_XINCREF(op->cl_setattr); Py_XINCREF(op->cl_delattr); _PyObject_GC_TRACK(op); + + if (warn_classic_mro_risk(op) < 0) { + Py_DECREF(op); + return NULL; + } + return (PyObject *) op; } @@ -353,8 +937,14 @@ class_setattr(PyClassObject *op, PyObject *name, PyObject *v) char *err = NULL; if (strcmp(sname, "__dict__") == 0) err = set_dict(op, v); - else if (strcmp(sname, "__bases__") == 0) + else if (strcmp(sname, "__bases__") == 0) { err = set_bases(op, v); + if (err != NULL && *err == '\0') { + if (warn_classic_mro_risk(op) < 0) + return -1; + return 0; + } + } else if (strcmp(sname, "__name__") == 0) err = set_name(op, v); else if (strcmp(sname, "__getattr__") == 0) diff --git a/test_mro.py b/test_mro.py new file mode 100644 index 00000000000000..de8598a9d45458 --- /dev/null +++ b/test_mro.py @@ -0,0 +1,44 @@ +import sys + + +# Warns: classic lookup would find A.do_this before C.do_this, +# but hypothetical C3 would find C.do_this first. +class A: + def do_this(self): + return "A" + + +class B(A): + pass + + +class C(A): + def do_this(self): + return "C" + + +class D(B, C): + pass + +sys.stdout.write("warns: %s\n" % D().do_this()) + + +# Does not warn: classic order and C3 order differ, but both still resolve +# do_this to the same defining class because only SafeA provides it. +class SafeA: + def do_this(self): + return "SafeA" + + +class SafeB(SafeA): + pass + + +class SafeC(SafeA): + pass + + +class SafeD(SafeB, SafeC): + pass + +sys.stdout.write("safe: %s\n" % SafeD().do_this()) From 0ef690a1c071f08cb8cfa048c8aa197eaf05def1 Mon Sep 17 00:00:00 2001 From: Eddy Xu Date: Tue, 21 Apr 2026 18:44:41 -0400 Subject: [PATCH 3/7] add warning for class inheritance problem --- Lib/test/test_py3kwarn.py | 35 ++++----- Objects/classobject.c | 148 +++++--------------------------------- test_mro.py | 44 ------------ 3 files changed, 33 insertions(+), 194 deletions(-) delete mode 100644 test_mro.py diff --git a/Lib/test/test_py3kwarn.py b/Lib/test/test_py3kwarn.py index d6e89e287bb0e7..e4e9fbf04365f3 100644 --- a/Lib/test/test_py3kwarn.py +++ b/Lib/test/test_py3kwarn.py @@ -455,20 +455,10 @@ def assertExecLocalWritebackWarning(self, recorder, local_name): "exec() modified local '%s'" % local_name)) recorder.reset() - def assertMROWarning(self, recorder, risk_kind, class_name, - attr_name=None, classic_provider=None, - c3_provider=None): + def assertMROWarning(self, recorder, expected_message): self.assertEqual(len(recorder.warnings), 1) msg = str(recorder.warnings[0].message) - self.assertIn('"warning_type":"MRO_RISK_WARNING"', msg) - self.assertIn('"risk_kind":"%s"' % risk_kind, msg) - self.assertIn('"class_name":"%s"' % class_name, msg) - if attr_name is not None: - self.assertIn('"attr_name":"%s"' % attr_name, msg) - if classic_provider is not None: - self.assertIn('"classic_provider":"%s"' % classic_provider, msg) - if c3_provider is not None: - self.assertIn('"c3_provider":"%s"' % c3_provider, msg) + self.assertEqual(msg, expected_message) recorder.reset() def test_exec_local_writeback_warning(self): @@ -528,10 +518,10 @@ class D(B, C): self.assertEqual(D().do_this(), "A") self.assertMROWarning( - w, "resolution_change", "D", - attr_name="do_this", - classic_provider="A", - c3_provider="C") + w, + "classic multiple inheritance for class 'D' may resolve " + "attribute 'do_this' from 'A' in 2.x but from 'C' in 3.x " + "due to C3 MRO") def test_classic_mro_single_inheritance_no_warning(self): with self.check_py3k_warnings_with_fix() as w: @@ -600,7 +590,10 @@ class Y(B, A): class Z(X, Y): pass - self.assertMROWarning(w, "c3_conflict", "Z") + self.assertMROWarning( + w, + "classic multiple inheritance hierarchy for class 'Z' has no " + "consistent C3 MRO and will fail in 3.x") def test_classic_mro_bases_update_warning(self): with self.check_py3k_warnings_with_fix() as w: @@ -622,10 +615,10 @@ class D(B): D.__bases__ = (B, C) self.assertEqual(D().do_this(), "A") self.assertMROWarning( - w, "resolution_change", "D", - attr_name="do_this", - classic_provider="A", - c3_provider="C") + w, + "classic multiple inheritance for class 'D' may resolve " + "attribute 'do_this' from 'A' in 2.x but from 'C' in 3.x " + "due to C3 MRO") class TestStdlibRemovals(unittest.TestCase): diff --git a/Objects/classobject.c b/Objects/classobject.c index 693de47b7e3129..809066d2a7e3f1 100644 --- a/Objects/classobject.c +++ b/Objects/classobject.c @@ -33,7 +33,7 @@ static PyObject * classic_mro_list(PyClassObject *klass); static PyObject * -hypothetical_c3_mro(PyClassObject *klass, PyObject **error_text); +hypothetical_c3_mro(PyClassObject *klass, int *conflict); /* ---------- classic MRO / C3 risk analysis ---------- */ @@ -131,7 +131,7 @@ c3_head_in_tails(PyObject *seqs, PyObject *candidate) } static PyObject * -c3_merge(PyObject *seqs, PyObject **error_text) +c3_merge(PyObject *seqs, int *conflict) { PyObject *result = NULL; int progress = 1; @@ -183,10 +183,8 @@ c3_merge(PyObject *seqs, PyObject **error_text) return result; if (!progress) { - if (error_text != NULL && *error_text == NULL) { - *error_text = PyString_FromString( - "no consistent C3 MRO for classic hierarchy"); - } + if (conflict != NULL) + *conflict = 1; Py_DECREF(result); return NULL; } @@ -196,7 +194,7 @@ c3_merge(PyObject *seqs, PyObject **error_text) } static PyObject * -hypothetical_c3_mro(PyClassObject *klass, PyObject **error_text) +hypothetical_c3_mro(PyClassObject *klass, int *conflict) { PyObject *result = NULL; PyObject *seqs = NULL; @@ -230,7 +228,7 @@ hypothetical_c3_mro(PyClassObject *klass, PyObject **error_text) if (PyList_Append(bases_list, base) < 0) goto error; - base_mro = hypothetical_c3_mro((PyClassObject *)base, error_text); + base_mro = hypothetical_c3_mro((PyClassObject *)base, conflict); if (base_mro == NULL) goto error; @@ -244,7 +242,7 @@ hypothetical_c3_mro(PyClassObject *klass, PyObject **error_text) if (PyList_Append(seqs, bases_list) < 0) goto error; - merged = c3_merge(seqs, error_text); + merged = c3_merge(seqs, conflict); if (merged == NULL) goto error; @@ -268,64 +266,6 @@ hypothetical_c3_mro(PyClassObject *klass, PyObject **error_text) return NULL; } -static PyObject * -join_class_names_from_seq(PyObject *seq) -{ - PyObject *parts = NULL, *sep = NULL, *res = NULL; - Py_ssize_t i, n; - - n = PySequence_Size(seq); - if (n < 0) - return NULL; - - parts = PyList_New(0); - if (parts == NULL) - return NULL; - - for (i = 0; i < n; i++) { - PyObject *item = PySequence_GetItem(seq, i); - PyObject *name = NULL; - - if (item == NULL) - goto error; - - if (PyClass_Check(item) && - ((PyClassObject *)item)->cl_name != NULL && - PyString_Check(((PyClassObject *)item)->cl_name)) { - name = ((PyClassObject *)item)->cl_name; - Py_INCREF(name); - } - else { - name = PyString_FromString("?"); - } - - Py_DECREF(item); - - if (name == NULL) - goto error; - if (PyList_Append(parts, name) < 0) { - Py_DECREF(name); - goto error; - } - Py_DECREF(name); - } - - sep = PyString_FromString("|"); - if (sep == NULL) - goto error; - - res = PyObject_CallMethod(sep, "join", "O", parts); - - Py_DECREF(sep); - Py_DECREF(parts); - return res; - -error: - Py_XDECREF(sep); - Py_XDECREF(parts); - return NULL; -} - static PyClassObject * c3_provider_for_name(PyObject *c3_mro, PyObject *name) { @@ -398,14 +338,11 @@ static int warn_mro_resolution_change(PyClassObject *klass, PyObject *name, PyClassObject *classic_provider, - PyClassObject *c3_provider, - PyObject *classic_mro, - PyObject *c3_mro) + PyClassObject *c3_provider) { PyFrameObject *f = PyEval_GetFrame(); const char *filename = ""; int lineno = 0; - PyObject *base_names = NULL, *classic_names = NULL, *c3_names = NULL; PyObject *msg = NULL; int rc; @@ -416,35 +353,13 @@ warn_mro_resolution_change(PyClassObject *klass, lineno = PyCode_Addr2Line(f->f_code, f->f_lasti); } - base_names = join_class_names_from_seq(klass->cl_bases); - classic_names = join_class_names_from_seq(classic_mro); - c3_names = join_class_names_from_seq(c3_mro); - if (base_names == NULL || classic_names == NULL || c3_names == NULL) - goto error; - msg = PyString_FromFormat( "classic multiple inheritance for class '%s' may resolve attribute '%s' " - "from '%s' in 2.x but from '%s' in 3.x due to C3 MRO" - "\nPYGRATE_META: {\"warning_type\":\"MRO_RISK_WARNING\"," - "\"risk_kind\":\"resolution_change\"," - "\"class_name\":\"%s\"," - "\"base_names\":\"%s\"," - "\"attr_name\":\"%s\"," - "\"classic_provider\":\"%s\"," - "\"c3_provider\":\"%s\"," - "\"classic_mro\":\"%s\"," - "\"c3_mro\":\"%s\"}", - class_name_cstr(klass), - PyString_AsString(name), - class_name_cstr(classic_provider), - class_name_cstr(c3_provider), + "from '%s' in 2.x but from '%s' in 3.x due to C3 MRO", class_name_cstr(klass), - PyString_AsString(base_names), PyString_AsString(name), class_name_cstr(classic_provider), - class_name_cstr(c3_provider), - PyString_AsString(classic_names), - PyString_AsString(c3_names)); + class_name_cstr(c3_provider)); if (msg == NULL) goto error; @@ -457,27 +372,21 @@ warn_mro_resolution_change(PyClassObject *klass, NULL, NULL); - Py_DECREF(base_names); - Py_DECREF(classic_names); - Py_DECREF(c3_names); Py_DECREF(msg); return rc; error: - Py_XDECREF(base_names); - Py_XDECREF(classic_names); - Py_XDECREF(c3_names); Py_XDECREF(msg); return -1; } static int -warn_mro_c3_conflict(PyClassObject *klass, PyObject *error_text) +warn_mro_c3_conflict(PyClassObject *klass) { PyFrameObject *f = PyEval_GetFrame(); const char *filename = ""; int lineno = 0; - PyObject *base_names = NULL, *msg = NULL; + PyObject *msg = NULL; int rc; if (f != NULL && f->f_code != NULL && @@ -487,24 +396,10 @@ warn_mro_c3_conflict(PyClassObject *klass, PyObject *error_text) lineno = PyCode_Addr2Line(f->f_code, f->f_lasti); } - base_names = join_class_names_from_seq(klass->cl_bases); - if (base_names == NULL) - goto error; - msg = PyString_FromFormat( "classic multiple inheritance hierarchy for class '%s' has no " - "consistent C3 MRO and will fail in 3.x" - "\nPYGRATE_META: {\"warning_type\":\"MRO_RISK_WARNING\"," - "\"risk_kind\":\"c3_conflict\"," - "\"class_name\":\"%s\"," - "\"base_names\":\"%s\"," - "\"c3_error\":\"%s\"}", - class_name_cstr(klass), - class_name_cstr(klass), - PyString_AsString(base_names), - (error_text != NULL && PyString_Check(error_text)) - ? PyString_AsString(error_text) - : "no consistent C3 MRO"); + "consistent C3 MRO and will fail in 3.x", + class_name_cstr(klass)); if (msg == NULL) goto error; @@ -517,12 +412,10 @@ warn_mro_c3_conflict(PyClassObject *klass, PyObject *error_text) NULL, NULL); - Py_DECREF(base_names); Py_DECREF(msg); return rc; error: - Py_XDECREF(base_names); Py_XDECREF(msg); return -1; } @@ -532,9 +425,9 @@ warn_classic_mro_risk(PyClassObject *klass) { PyObject *classic_mro = NULL; PyObject *c3_mro = NULL; - PyObject *c3_error = NULL; PyObject *candidates = NULL; Py_ssize_t i, n; + int c3_conflict = 0; if (!Py_Py3kWarningFlag) return 0; @@ -549,12 +442,11 @@ warn_classic_mro_risk(PyClassObject *klass) if (classic_mro == NULL) goto error; - c3_mro = hypothetical_c3_mro(klass, &c3_error); + c3_mro = hypothetical_c3_mro(klass, &c3_conflict); if (c3_mro == NULL) { - if (c3_error != NULL) { - int rc = warn_mro_c3_conflict(klass, c3_error); + if (c3_conflict) { + int rc = warn_mro_c3_conflict(klass); Py_DECREF(classic_mro); - Py_DECREF(c3_error); return rc; } goto error; @@ -578,8 +470,7 @@ warn_classic_mro_risk(PyClassObject *klass) c3_provider != NULL && classic_provider != c3_provider) { int rc = warn_mro_resolution_change( - klass, name, classic_provider, c3_provider, - classic_mro, c3_mro); + klass, name, classic_provider, c3_provider); Py_DECREF(classic_mro); Py_DECREF(c3_mro); Py_DECREF(candidates); @@ -595,7 +486,6 @@ warn_classic_mro_risk(PyClassObject *klass) error: Py_XDECREF(classic_mro); Py_XDECREF(c3_mro); - Py_XDECREF(c3_error); Py_XDECREF(candidates); return -1; } diff --git a/test_mro.py b/test_mro.py deleted file mode 100644 index de8598a9d45458..00000000000000 --- a/test_mro.py +++ /dev/null @@ -1,44 +0,0 @@ -import sys - - -# Warns: classic lookup would find A.do_this before C.do_this, -# but hypothetical C3 would find C.do_this first. -class A: - def do_this(self): - return "A" - - -class B(A): - pass - - -class C(A): - def do_this(self): - return "C" - - -class D(B, C): - pass - -sys.stdout.write("warns: %s\n" % D().do_this()) - - -# Does not warn: classic order and C3 order differ, but both still resolve -# do_this to the same defining class because only SafeA provides it. -class SafeA: - def do_this(self): - return "SafeA" - - -class SafeB(SafeA): - pass - - -class SafeC(SafeA): - pass - - -class SafeD(SafeB, SafeC): - pass - -sys.stdout.write("safe: %s\n" % SafeD().do_this()) From aa16a0827a54ccdd01d86982d159642a6a7a6fb0 Mon Sep 17 00:00:00 2001 From: Eddy Xu Date: Tue, 21 Apr 2026 22:55:25 -0400 Subject: [PATCH 4/7] fix format issues --- Lib/test/test_py3kwarn.py | 1 + Objects/classobject.c | 86 +++++++++++++++++++-------------------- 2 files changed, 44 insertions(+), 43 deletions(-) diff --git a/Lib/test/test_py3kwarn.py b/Lib/test/test_py3kwarn.py index e4e9fbf04365f3..b59316893533a9 100644 --- a/Lib/test/test_py3kwarn.py +++ b/Lib/test/test_py3kwarn.py @@ -43,6 +43,7 @@ def check_py3k_warnings_with_fix(self): if registry: registry.clear() with warnings.catch_warnings(record=True) as w: + # PyErr_WarnExplicit_WithFix uses this runtime hook directly. showwarningwithfix = warnings.showwarningwithfix def record_warning_with_fix(*args, **kwargs): w.append(warnings.WarningMessageWithFix(*args, **kwargs)) diff --git a/Objects/classobject.c b/Objects/classobject.c index 809066d2a7e3f1..3282043898d181 100644 --- a/Objects/classobject.c +++ b/Objects/classobject.c @@ -33,7 +33,7 @@ static PyObject * classic_mro_list(PyClassObject *klass); static PyObject * -hypothetical_c3_mro(PyClassObject *klass, int *conflict); +hypothetical_c3_mro(PyClassObject *klass, int *merge_conflicted); /* ---------- classic MRO / C3 risk analysis ---------- */ @@ -131,20 +131,21 @@ c3_head_in_tails(PyObject *seqs, PyObject *candidate) } static PyObject * -c3_merge(PyObject *seqs, int *conflict) +c3_merge(PyObject *seqs, int *merge_conflicted) { PyObject *result = NULL; - int progress = 1; + int selected_head = 1; result = PyList_New(0); if (result == NULL) return NULL; - while (progress) { + while (selected_head) { Py_ssize_t i, nseqs; - int nonempty = 0; + int has_pending_sequences = 0; - progress = 0; + /* No selected head in a pass means the C3 constraints conflict. */ + selected_head = 0; nseqs = PyList_GET_SIZE(seqs); for (i = 0; i < nseqs; i++) { @@ -155,7 +156,7 @@ c3_merge(PyObject *seqs, int *conflict) if (n == 0) continue; - nonempty = 1; + has_pending_sequences = 1; cand = PyList_GET_ITEM(seq, 0); if (!c3_head_in_tails(seqs, cand)) { @@ -167,24 +168,23 @@ c3_merge(PyObject *seqs, int *conflict) for (j = 0; j < nseqs; j++) { PyObject *s = PyList_GET_ITEM(seqs, j); if (PyList_GET_SIZE(s) > 0 && - PyList_GET_ITEM(s, 0) == cand) { - if (PySequence_DelItem(s, 0) < 0) { - Py_DECREF(result); - return NULL; - } + PyList_GET_ITEM(s, 0) == cand && + PySequence_DelItem(s, 0) < 0) { + Py_DECREF(result); + return NULL; } } - progress = 1; + selected_head = 1; break; } } - if (!nonempty) + if (!has_pending_sequences) return result; - if (!progress) { - if (conflict != NULL) - *conflict = 1; + if (!selected_head) { + if (merge_conflicted != NULL) + *merge_conflicted = 1; Py_DECREF(result); return NULL; } @@ -194,7 +194,7 @@ c3_merge(PyObject *seqs, int *conflict) } static PyObject * -hypothetical_c3_mro(PyClassObject *klass, int *conflict) +hypothetical_c3_mro(PyClassObject *klass, int *merge_conflicted) { PyObject *result = NULL; PyObject *seqs = NULL; @@ -228,7 +228,8 @@ hypothetical_c3_mro(PyClassObject *klass, int *conflict) if (PyList_Append(bases_list, base) < 0) goto error; - base_mro = hypothetical_c3_mro((PyClassObject *)base, conflict); + base_mro = hypothetical_c3_mro((PyClassObject *)base, + merge_conflicted); if (base_mro == NULL) goto error; @@ -242,7 +243,7 @@ hypothetical_c3_mro(PyClassObject *klass, int *conflict) if (PyList_Append(seqs, bases_list) < 0) goto error; - merged = c3_merge(seqs, conflict); + merged = c3_merge(seqs, merge_conflicted); if (merged == NULL) goto error; @@ -282,12 +283,12 @@ c3_provider_for_name(PyObject *c3_mro, PyObject *name) static PyObject * duplicate_candidate_names(PyObject *classic_mro) { - PyObject *seen = NULL, *dups = NULL, *keys = NULL; + PyObject *seen_names = NULL, *duplicate_names = NULL, *keys = NULL; Py_ssize_t i, n; - seen = PyDict_New(); - dups = PyDict_New(); - if (seen == NULL || dups == NULL) + seen_names = PyDict_New(); + duplicate_names = PyDict_New(); + if (seen_names == NULL || duplicate_names == NULL) goto error; n = PyList_GET_SIZE(classic_mro); @@ -297,40 +298,38 @@ duplicate_candidate_names(PyObject *classic_mro) PyObject *k, *v; while (PyDict_Next(cp->cl_dict, &pos, &k, &v)) { - int contains; + int name_already_seen; if (mro_noise_name(k)) continue; - contains = PyDict_Contains(seen, k); - if (contains < 0) + name_already_seen = PyDict_Contains(seen_names, k); + if (name_already_seen < 0) goto error; - if (contains) { - if (PyDict_SetItem(dups, k, Py_True) < 0) - goto error; - } - else { - if (PyDict_SetItem(seen, k, Py_True) < 0) - goto error; - } + if (name_already_seen && + PyDict_SetItem(duplicate_names, k, Py_True) < 0) + goto error; + else if (!name_already_seen && + PyDict_SetItem(seen_names, k, Py_True) < 0) + goto error; } } - keys = PyDict_Keys(dups); + keys = PyDict_Keys(duplicate_names); if (keys == NULL) goto error; if (PyList_Sort(keys) < 0) goto error; - Py_DECREF(seen); - Py_DECREF(dups); + Py_DECREF(seen_names); + Py_DECREF(duplicate_names); return keys; error: Py_XDECREF(keys); - Py_XDECREF(seen); - Py_XDECREF(dups); + Py_XDECREF(seen_names); + Py_XDECREF(duplicate_names); return NULL; } @@ -427,7 +426,8 @@ warn_classic_mro_risk(PyClassObject *klass) PyObject *c3_mro = NULL; PyObject *candidates = NULL; Py_ssize_t i, n; - int c3_conflict = 0; + /* Keep C3 conflicts separate from ordinary allocation/lookup errors. */ + int c3_merge_conflicted = 0; if (!Py_Py3kWarningFlag) return 0; @@ -442,9 +442,9 @@ warn_classic_mro_risk(PyClassObject *klass) if (classic_mro == NULL) goto error; - c3_mro = hypothetical_c3_mro(klass, &c3_conflict); + c3_mro = hypothetical_c3_mro(klass, &c3_merge_conflicted); if (c3_mro == NULL) { - if (c3_conflict) { + if (c3_merge_conflicted) { int rc = warn_mro_c3_conflict(klass); Py_DECREF(classic_mro); return rc; From 18eb66d3d3490e26e65a7721427c8e7a950aa597 Mon Sep 17 00:00:00 2001 From: Eddy Xu Date: Tue, 21 Apr 2026 23:01:26 -0400 Subject: [PATCH 5/7] undone cevel changes that came from another PR due to some problem..... --- Python/ceval.c | 325 ------------------------------------------------- 1 file changed, 325 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 582acc5f7a425a..e1140a8e401243 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -19,231 +19,6 @@ #include -int -_Py3kWarn_NextOpcode(void) -{ - PyFrameObject *frame; - char *code; - Py_ssize_t n; - int offset; - int op; - int steps; - - frame = PyEval_GetFrame(); - if (frame == NULL || frame->f_code == NULL) - return -1; - if (PyString_AsStringAndSize(frame->f_code->co_code, &code, &n) < 0) { - PyErr_Clear(); - return -1; - } - - offset = frame->f_lasti; - if (offset < 0 || offset >= n) - return -1; - - op = (unsigned char)code[offset]; - offset += 1; - if (HAS_ARG(op)) - offset += 2; - - /* These warnings only care about the immediate consumer of the - just-evaluated call result. Looking ahead a handful of opcodes is - enough to skip trivial stack setup before we either find a relevant - consumer or hit a stop opcode showing the value has already been - stored, returned, or discarded. */ - for (steps = 0; steps < 8 && offset >= 0 && offset < n; steps++) { - op = (unsigned char)code[offset]; - if (op == BINARY_ADD || op == INPLACE_ADD || op == GET_ITER || - op == BINARY_SUBSCR || op == STORE_SUBSCR) - return op; - if (op == RETURN_VALUE || op == STORE_NAME || op == STORE_FAST || - op == STORE_GLOBAL || op == STORE_ATTR || op == POP_TOP) - return -1; - offset += 1; - if (HAS_ARG(op)) - offset += 2; - } - return -1; -} - -static PyObject *exec_local_writeback_map = NULL; - -static PyObject * -_get_exec_local_writeback_map(void) -{ - if (exec_local_writeback_map == NULL) { - exec_local_writeback_map = PyDict_New(); - } - return exec_local_writeback_map; -} - -static void -_clear_exec_local_writeback_for_frame(PyFrameObject *f) -{ - PyObject *frame_key; - - if (exec_local_writeback_map == NULL) { - return; - } - frame_key = PyLong_FromVoidPtr(f); - if (frame_key == NULL) { - PyErr_Clear(); - return; - } - if (PyDict_DelItem(exec_local_writeback_map, frame_key) < 0) { - PyErr_Clear(); - } - Py_DECREF(frame_key); -} - -static int -_warn_exec_local_writeback(PyFrameObject *f, int oparg, int opcode) -{ - PyObject *frame_key = NULL; - PyObject *frame_dict = NULL; - PyObject *entry = NULL; - PyObject *idx = NULL; - PyObject *msg = NULL; - PyObject *name_obj = NULL; - const char *local_name = NULL; - const char *func_name = NULL; - int exec_lineno = 0; - int exec_offset = 0; - int read_lineno = 0; - int read_offset = f->f_lasti; - int warn_result; - - if (exec_local_writeback_map == NULL) { - return 0; - } - frame_key = PyLong_FromVoidPtr(f); - if (frame_key == NULL) { - PyErr_Clear(); - return 0; - } - frame_dict = PyDict_GetItem(exec_local_writeback_map, frame_key); - if (frame_dict == NULL) { - Py_DECREF(frame_key); - return 0; - } - idx = PyInt_FromLong(oparg); - if (idx == NULL) { - Py_DECREF(frame_key); - PyErr_Clear(); - return 0; - } - entry = PyDict_GetItem(frame_dict, idx); - if (entry == NULL) { - Py_DECREF(idx); - Py_DECREF(frame_key); - return 0; - } - if (PyTuple_Check(entry) && PyTuple_GET_SIZE(entry) == 2) { - exec_lineno = (int)PyInt_AsLong(PyTuple_GET_ITEM(entry, 0)); - exec_offset = (int)PyInt_AsLong(PyTuple_GET_ITEM(entry, 1)); - } - if (exec_lineno < 0) { - exec_lineno = 0; - } - read_lineno = PyCode_Addr2Line(f->f_code, f->f_lasti); - name_obj = PyTuple_GetItem(f->f_code->co_varnames, oparg); - if (name_obj != NULL) { - local_name = PyString_AsString(name_obj); - } - if (local_name == NULL) { - local_name = ""; - } - if (f->f_code->co_name != NULL) { - func_name = PyString_AsString(f->f_code->co_name); - } - if (func_name == NULL) { - func_name = ""; - } - msg = PyString_FromFormat( - "exec() modified local '%s' which is read later in the enclosing function; " - "in 3.x exec() does not reliably write back to function locals without an explicit locals mapping" - "\nPYGRATE_META: {\"warning_type\":\"EXEC_LOCAL_WRITEBACK_WARNING\"," - "\"scope_kind\":\"function\",\"has_explicit_globals\":false," - "\"has_explicit_locals\":false,\"local_name\":\"%s\"," - "\"exec_lineno\":%d,\"read_lineno\":%d,\"read_opcode\":%d," - "\"exec_offset\":%d,\"read_offset\":%d,\"function_name\":\"%s\"}", - local_name, local_name, exec_lineno, read_lineno, opcode, - exec_offset, read_offset, func_name); - if (msg == NULL) { - Py_DECREF(idx); - Py_DECREF(frame_key); - PyErr_Clear(); - return 0; - } - - warn_result = PyErr_WarnExplicit_WithFix( - PyExc_Py3xWarning, - PyString_AsString(msg), - "use exec(code, globals, locals) with an explicit locals mapping", - PyString_AsString(f->f_code->co_filename), - read_lineno, - NULL, - NULL); - - Py_DECREF(msg); - if (warn_result < 0) { - Py_DECREF(idx); - Py_DECREF(frame_key); - return -1; - } - - if (PyDict_DelItem(frame_dict, idx) < 0) { - PyErr_Clear(); - } - Py_DECREF(idx); - - if (PyDict_Size(frame_dict) <= 0) { - if (PyDict_DelItem(exec_local_writeback_map, frame_key) < 0) { - PyErr_Clear(); - } - } - Py_DECREF(frame_key); - return 0; -} - -static void -_clear_exec_local_writeback_for_local(PyFrameObject *f, int oparg) -{ - PyObject *frame_key = NULL; - PyObject *frame_dict = NULL; - PyObject *idx = NULL; - - if (exec_local_writeback_map == NULL) { - return; - } - frame_key = PyLong_FromVoidPtr(f); - if (frame_key == NULL) { - PyErr_Clear(); - return; - } - frame_dict = PyDict_GetItem(exec_local_writeback_map, frame_key); - if (frame_dict == NULL) { - Py_DECREF(frame_key); - return; - } - idx = PyInt_FromLong(oparg); - if (idx == NULL) { - Py_DECREF(frame_key); - PyErr_Clear(); - return; - } - if (PyDict_DelItem(frame_dict, idx) < 0) { - PyErr_Clear(); - } - Py_DECREF(idx); - if (PyDict_Size(frame_dict) <= 0) { - if (PyDict_DelItem(exec_local_writeback_map, frame_key) < 0) { - PyErr_Clear(); - } - } - Py_DECREF(frame_key); -} - #ifndef WITH_TSC #define READ_TIMESTAMP(var) @@ -1451,12 +1226,6 @@ PyEval_EvalFrameEx(PyFrameObject *f, int throwflag) { x = GETLOCAL(oparg); if (x != NULL) { - if (Py_Py3kWarningFlag) { - if (_warn_exec_local_writeback(f, oparg, opcode) < 0) { - err = -1; - break; - } - } Py_INCREF(x); PUSH(x); FAST_DISPATCH(); @@ -1480,9 +1249,6 @@ PyEval_EvalFrameEx(PyFrameObject *f, int throwflag) { v = POP(); SETLOCAL(oparg, v); - if (Py_Py3kWarningFlag) { - _clear_exec_local_writeback_for_local(f, oparg); - } FAST_DISPATCH(); } @@ -2641,9 +2407,6 @@ PyEval_EvalFrameEx(PyFrameObject *f, int throwflag) x = GETLOCAL(oparg); if (x != NULL) { SETLOCAL(oparg, NULL); - if (Py_Py3kWarningFlag) { - _clear_exec_local_writeback_for_local(f, oparg); - } DISPATCH(); } format_exc_check_arg( @@ -3594,7 +3357,6 @@ PyEval_EvalFrameEx(PyFrameObject *f, int throwflag) /* pop frame */ exit_eval_frame: - _clear_exec_local_writeback_for_frame(f); Py_LeaveRecursiveCall(); tstate->frame = f->f_back; @@ -5277,12 +5039,6 @@ exec_statement(PyFrameObject *f, PyObject *prog, PyObject *globals, int n; PyObject *v; int plain = 0; - int track_locals = 0; - int exec_lineno = 0; - int exec_offset = 0; - int nlocals = 0; - PyObject **before = NULL; - int i; if (PyTuple_Check(prog) && globals == Py_None && locals == Py_None && ((n = PyTuple_Size(prog)) == 2 || n == 3)) { @@ -5328,23 +5084,6 @@ exec_statement(PyFrameObject *f, PyObject *prog, PyObject *globals, } if (PyDict_GetItemString(globals, "__builtins__") == NULL) PyDict_SetItemString(globals, "__builtins__", f->f_builtins); - - if (plain && Py_Py3kWarningFlag && - (f->f_code->co_flags & CO_NEWLOCALS) && - f->f_code->co_nlocals > 0 && - f->f_localsplus != NULL) { - nlocals = f->f_code->co_nlocals; - before = PyMem_New(PyObject *, nlocals); - if (before != NULL) { - for (i = 0; i < nlocals; i++) { - before[i] = f->f_localsplus[i]; - Py_XINCREF(before[i]); - } - track_locals = 1; - exec_offset = f->f_lasti; - exec_lineno = PyCode_Addr2Line(f->f_code, exec_offset); - } - } if (PyCode_Check(prog)) { if (PyCode_GetNumFree((PyCodeObject *)prog) > 0) { PyErr_SetString(PyExc_TypeError, @@ -5392,70 +5131,6 @@ exec_statement(PyFrameObject *f, PyObject *prog, PyObject *globals, } if (plain) PyFrame_LocalsToFast(f, 0); - if (track_locals && v != NULL) { - PyObject *frame_key = NULL; - PyObject *frame_dict = NULL; - PyObject *map = NULL; - for (i = 0; i < nlocals; i++) { - PyObject *before_obj = before[i]; - PyObject *after_obj = f->f_localsplus[i]; - if (before_obj == after_obj) { - continue; - } - if (frame_key == NULL) { - frame_key = PyLong_FromVoidPtr(f); - if (frame_key == NULL) { - PyErr_Clear(); - break; - } - } - if (map == NULL) { - map = _get_exec_local_writeback_map(); - if (map == NULL) { - PyErr_Clear(); - break; - } - } - if (frame_dict == NULL) { - frame_dict = PyDict_GetItem(map, frame_key); - if (frame_dict == NULL) { - frame_dict = PyDict_New(); - if (frame_dict == NULL) { - PyErr_Clear(); - break; - } - if (PyDict_SetItem(map, frame_key, frame_dict) < 0) { - PyErr_Clear(); - Py_DECREF(frame_dict); - frame_dict = NULL; - break; - } - Py_DECREF(frame_dict); - frame_dict = PyDict_GetItem(map, frame_key); - } - } - if (frame_dict != NULL) { - PyObject *idx = PyInt_FromLong(i); - PyObject *val = Py_BuildValue("ii", exec_lineno, exec_offset); - if (idx != NULL && val != NULL) { - if (PyDict_SetItem(frame_dict, idx, val) < 0) { - PyErr_Clear(); - } - } else { - PyErr_Clear(); - } - Py_XDECREF(idx); - Py_XDECREF(val); - } - } - Py_XDECREF(frame_key); - } - if (before != NULL) { - for (i = 0; i < nlocals; i++) { - Py_XDECREF(before[i]); - } - PyMem_Free(before); - } if (v == NULL) return -1; Py_DECREF(v); From 9fc208a1c8b8e5c4b5958bac34d41f3b355375c7 Mon Sep 17 00:00:00 2001 From: Eddy Xu Date: Tue, 21 Apr 2026 23:05:14 -0400 Subject: [PATCH 6/7] still fix cevel issues. --- Python/ceval.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/Python/ceval.c b/Python/ceval.c index e1140a8e401243..2af1ef7a42fed6 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -19,6 +19,53 @@ #include +int +_Py3kWarn_NextOpcode(void) +{ + PyFrameObject *frame; + char *code; + Py_ssize_t n; + int offset; + int op; + int steps; + + frame = PyEval_GetFrame(); + if (frame == NULL || frame->f_code == NULL) + return -1; + if (PyString_AsStringAndSize(frame->f_code->co_code, &code, &n) < 0) { + PyErr_Clear(); + return -1; + } + + offset = frame->f_lasti; + if (offset < 0 || offset >= n) + return -1; + + op = (unsigned char)code[offset]; + offset += 1; + if (HAS_ARG(op)) + offset += 2; + + /* These warnings only care about the immediate consumer of the + just-evaluated call result. Looking ahead a handful of opcodes is + enough to skip trivial stack setup before we either find a relevant + consumer or hit a stop opcode showing the value has already been + stored, returned, or discarded. */ + for (steps = 0; steps < 8 && offset >= 0 && offset < n; steps++) { + op = (unsigned char)code[offset]; + if (op == BINARY_ADD || op == INPLACE_ADD || op == GET_ITER || + op == BINARY_SUBSCR || op == STORE_SUBSCR) + return op; + if (op == RETURN_VALUE || op == STORE_NAME || op == STORE_FAST || + op == STORE_GLOBAL || op == STORE_ATTR || op == POP_TOP) + return -1; + offset += 1; + if (HAS_ARG(op)) + offset += 2; + } + return -1; +} + #ifndef WITH_TSC #define READ_TIMESTAMP(var) From 2f0ac8d1a9794231c63a1558ef43bb7c842f1053 Mon Sep 17 00:00:00 2001 From: Eddy Xu Date: Tue, 21 Apr 2026 23:07:18 -0400 Subject: [PATCH 7/7] I seems star this repo from exec improve so it contain not relevant chanegs --- Lib/test/test_py3kwarn.py | 50 ++------------------------------------- 1 file changed, 2 insertions(+), 48 deletions(-) diff --git a/Lib/test/test_py3kwarn.py b/Lib/test/test_py3kwarn.py index b59316893533a9..0b2ce773740c89 100644 --- a/Lib/test/test_py3kwarn.py +++ b/Lib/test/test_py3kwarn.py @@ -443,64 +443,18 @@ def test_b32encode_warns(self): expected = "base64.b32encode returns str in Python 2 (bytes in 3.x)" base64.b32encode(b'test') check_py3k_warnings(expected, UserWarning) - + def test_b16encode_warns(self): expected = "base64.b16encode returns str in Python 2 (bytes in 3.x)" base64.b16encode(b'test') check_py3k_warnings(expected, UserWarning) - def assertExecLocalWritebackWarning(self, recorder, local_name): - self.assertEqual(len(recorder.warnings), 1) - msg = str(recorder.warnings[0].message) - self.assertTrue(msg.startswith( - "exec() modified local '%s'" % local_name)) - recorder.reset() - def assertMROWarning(self, recorder, expected_message): self.assertEqual(len(recorder.warnings), 1) msg = str(recorder.warnings[0].message) self.assertEqual(msg, expected_message) recorder.reset() - - def test_exec_local_writeback_warning(self): - def f_exec(code): - b = 42 - exec code - return b - - with self.check_py3k_warnings_with_fix() as w: - f_exec("b = 99") - self.assertExecLocalWritebackWarning(w, "b") - - def f_exec_no_write(code): - b = 42 - exec code - return b - - with self.check_py3k_warnings_with_fix() as w: - f_exec_no_write("print b") - self.assertEqual(len(w.warnings), 0) - - def f_exec_overwrite(code): - b = 42 - exec code - b = 7 - return b - - with self.check_py3k_warnings_with_fix() as w: - f_exec_overwrite("b = 99") - self.assertEqual(len(w.warnings), 0) - - def f_exec_explicit(code): - b = 42 - ns = {'b': b} - exec code in globals(), ns - return b - - with self.check_py3k_warnings_with_fix() as w: - f_exec_explicit("b = 99") - self.assertEqual(len(w.warnings), 0) - + def test_classic_mro_resolution_change_warning(self): with self.check_py3k_warnings_with_fix() as w: class A: