From c268395de998ee77ef6555849e4e75e56145d09d Mon Sep 17 00:00:00 2001 From: Kaushik Kulkarni Date: Thu, 6 Oct 2022 11:29:46 -0500 Subject: [PATCH 1/4] [abc] better error message for undefined abstractmethod --- Include/internal/pycore_global_strings.h | 1 - Include/internal/pycore_runtime_init_generated.h | 5 ----- Lib/test/test_abc.py | 12 ++++++------ Lib/test/test_dataclasses.py | 2 +- Objects/typeobject.c | 10 ++++++---- 5 files changed, 13 insertions(+), 17 deletions(-) diff --git a/Include/internal/pycore_global_strings.h b/Include/internal/pycore_global_strings.h index f646979910c887..82db80a4c70430 100644 --- a/Include/internal/pycore_global_strings.h +++ b/Include/internal/pycore_global_strings.h @@ -37,7 +37,6 @@ struct _Py_global_strings { STRUCT_FOR_STR(anon_string, "") STRUCT_FOR_STR(anon_unknown, "") STRUCT_FOR_STR(close_br, "}") - STRUCT_FOR_STR(comma_sep, ", ") STRUCT_FOR_STR(dbl_close_br, "}}") STRUCT_FOR_STR(dbl_open_br, "{{") STRUCT_FOR_STR(dbl_percent, "%%") diff --git a/Include/internal/pycore_runtime_init_generated.h b/Include/internal/pycore_runtime_init_generated.h index bd1fedebd65cf5..75f2f8115e1f82 100644 --- a/Include/internal/pycore_runtime_init_generated.h +++ b/Include/internal/pycore_runtime_init_generated.h @@ -547,7 +547,6 @@ extern "C" { INIT_STR(anon_string, ""), \ INIT_STR(anon_unknown, ""), \ INIT_STR(close_br, "}"), \ - INIT_STR(comma_sep, ", "), \ INIT_STR(dbl_close_br, "}}"), \ INIT_STR(dbl_open_br, "{{"), \ INIT_STR(dbl_percent, "%%"), \ @@ -4750,10 +4749,6 @@ _PyStaticObjects_CheckRefcnt(void) { _PyObject_Dump((PyObject *)&_Py_STR(close_br)); Py_FatalError("immortal object has less refcnt than expected _PyObject_IMMORTAL_REFCNT"); }; - if (Py_REFCNT((PyObject *)&_Py_STR(comma_sep)) < _PyObject_IMMORTAL_REFCNT) { - _PyObject_Dump((PyObject *)&_Py_STR(comma_sep)); - Py_FatalError("immortal object has less refcnt than expected _PyObject_IMMORTAL_REFCNT"); - }; if (Py_REFCNT((PyObject *)&_Py_STR(dbl_close_br)) < _PyObject_IMMORTAL_REFCNT) { _PyObject_Dump((PyObject *)&_Py_STR(dbl_close_br)); Py_FatalError("immortal object has less refcnt than expected _PyObject_IMMORTAL_REFCNT"); diff --git a/Lib/test/test_abc.py b/Lib/test/test_abc.py index a083236fb0fc44..86f31a9acb4d55 100644 --- a/Lib/test/test_abc.py +++ b/Lib/test/test_abc.py @@ -154,7 +154,7 @@ class C(metaclass=abc_ABCMeta): @abc.abstractmethod def method_one(self): pass - msg = r"class C without an implementation for abstract method method_one" + msg = r"class C without an implementation for abstract method 'method_one'" self.assertRaisesRegex(TypeError, msg, C) def test_object_new_with_many_abstractmethods(self): @@ -165,7 +165,7 @@ def method_one(self): @abc.abstractmethod def method_two(self): pass - msg = r"class C without an implementation for abstract methods method_one, method_two" + msg = r"class C without an implementation for abstract methods 'method_one', 'method_two'" self.assertRaisesRegex(TypeError, msg, C) def test_abstractmethod_integration(self): @@ -535,7 +535,7 @@ def updated_foo(self): A.foo = updated_foo abc.update_abstractmethods(A) self.assertEqual(A.__abstractmethods__, {'foo', 'bar'}) - msg = "class A without an implementation for abstract methods bar, foo" + msg = "class A without an implementation for abstract methods 'bar', 'foo'" self.assertRaisesRegex(TypeError, msg, A) def test_update_implementation(self): @@ -547,7 +547,7 @@ def foo(self): class B(A): pass - msg = "class B without an implementation for abstract method foo" + msg = "class B without an implementation for abstract method 'foo'" self.assertRaisesRegex(TypeError, msg, B) self.assertEqual(B.__abstractmethods__, {'foo'}) @@ -605,7 +605,7 @@ def foo(self): abc.update_abstractmethods(B) - msg = "class B without an implementation for abstract method foo" + msg = "class B without an implementation for abstract method 'foo'" self.assertRaisesRegex(TypeError, msg, B) def test_update_layered_implementation(self): @@ -627,7 +627,7 @@ def foo(self): abc.update_abstractmethods(C) - msg = "class C without an implementation for abstract method foo" + msg = "class C without an implementation for abstract method 'foo'" self.assertRaisesRegex(TypeError, msg, C) def test_update_multi_inheritance(self): diff --git a/Lib/test/test_dataclasses.py b/Lib/test/test_dataclasses.py index 637c456dd49e7a..d3fb67d896164a 100644 --- a/Lib/test/test_dataclasses.py +++ b/Lib/test/test_dataclasses.py @@ -3962,7 +3962,7 @@ class Date(A): day: 'int' self.assertTrue(inspect.isabstract(Date)) - msg = 'class Date without an implementation for abstract method foo' + msg = "class Date without an implementation for abstract method 'foo'" self.assertRaisesRegex(TypeError, msg, Date) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 196a6aee4993b8..037ea6cc2231e1 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -4861,9 +4861,10 @@ object_new(PyTypeObject *type, PyObject *args, PyObject *kwds) PyObject *abstract_methods; PyObject *sorted_methods; PyObject *joined; + PyObject* comma_w_quotes_sep; Py_ssize_t method_count; - /* Compute ", ".join(sorted(type.__abstractmethods__)) + /* Compute "', '".join(sorted(type.__abstractmethods__)) into joined. */ abstract_methods = type_abstractmethods(type, NULL); if (abstract_methods == NULL) @@ -4876,8 +4877,8 @@ object_new(PyTypeObject *type, PyObject *args, PyObject *kwds) Py_DECREF(sorted_methods); return NULL; } - _Py_DECLARE_STR(comma_sep, ", "); - joined = PyUnicode_Join(&_Py_STR(comma_sep), sorted_methods); + comma_w_quotes_sep = PyUnicode_FromString("', '"); + joined = PyUnicode_Join(comma_w_quotes_sep, sorted_methods); method_count = PyObject_Length(sorted_methods); Py_DECREF(sorted_methods); if (joined == NULL) @@ -4887,11 +4888,12 @@ object_new(PyTypeObject *type, PyObject *args, PyObject *kwds) PyErr_Format(PyExc_TypeError, "Can't instantiate abstract class %s " - "without an implementation for abstract method%s %U", + "without an implementation for abstract method%s '%U'", type->tp_name, method_count > 1 ? "s" : "", joined); Py_DECREF(joined); + Py_DECREF(comma_w_quotes_sep); return NULL; } PyObject *obj = type->tp_alloc(type, 0); From 3603740464e724b61d70c4eddf12fdbbbdd4acca Mon Sep 17 00:00:00 2001 From: Kaushik Kulkarni Date: Sun, 9 Oct 2022 18:08:14 -0500 Subject: [PATCH 2/4] adds a NEWS entry --- .../next/Library/2022-10-09-18-08-08.gh-issue-97971.SaVHTd.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2022-10-09-18-08-08.gh-issue-97971.SaVHTd.rst diff --git a/Misc/NEWS.d/next/Library/2022-10-09-18-08-08.gh-issue-97971.SaVHTd.rst b/Misc/NEWS.d/next/Library/2022-10-09-18-08-08.gh-issue-97971.SaVHTd.rst new file mode 100644 index 00000000000000..ccabd421794380 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-10-09-18-08-08.gh-issue-97971.SaVHTd.rst @@ -0,0 +1,3 @@ +Improved :class:`TypeError` message for undefined abstract methods of a +:class:`abc.ABC` instance. The names of the missing methods are surrounded +by single-quotes to highlight them. From 4073b1fb4a0afe64c5c5742c75b12d80262bae31 Mon Sep 17 00:00:00 2001 From: Kaushik Kulkarni Date: Sat, 15 Oct 2022 10:44:18 -0500 Subject: [PATCH 3/4] corrects the NEWS entry --- ...1.SaVHTd.rst => 2022-10-15-10-43-45.gh-issue-98284.SaVHTd.rst} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename Misc/NEWS.d/next/Library/{2022-10-09-18-08-08.gh-issue-97971.SaVHTd.rst => 2022-10-15-10-43-45.gh-issue-98284.SaVHTd.rst} (100%) diff --git a/Misc/NEWS.d/next/Library/2022-10-09-18-08-08.gh-issue-97971.SaVHTd.rst b/Misc/NEWS.d/next/Library/2022-10-15-10-43-45.gh-issue-98284.SaVHTd.rst similarity index 100% rename from Misc/NEWS.d/next/Library/2022-10-09-18-08-08.gh-issue-97971.SaVHTd.rst rename to Misc/NEWS.d/next/Library/2022-10-15-10-43-45.gh-issue-98284.SaVHTd.rst From 6ae052ef771eb9e72f7cbb90b58a2bb1d10ae3f5 Mon Sep 17 00:00:00 2001 From: Kaushik Kulkarni Date: Sat, 5 Nov 2022 10:47:59 -0500 Subject: [PATCH 4/4] Avoid leaks by calling Py_DECREF in some code paths --- Objects/typeobject.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 037ea6cc2231e1..3975d932afeb08 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -4881,10 +4881,15 @@ object_new(PyTypeObject *type, PyObject *args, PyObject *kwds) joined = PyUnicode_Join(comma_w_quotes_sep, sorted_methods); method_count = PyObject_Length(sorted_methods); Py_DECREF(sorted_methods); - if (joined == NULL) + if (joined == NULL) { + Py_DECREF(comma_w_quotes_sep); return NULL; - if (method_count == -1) + } + if (method_count == -1) { + Py_DECREF(comma_w_quotes_sep); + Py_DECREF(joined); return NULL; + } PyErr_Format(PyExc_TypeError, "Can't instantiate abstract class %s "