From 72d9018be372ead490d36687187bff390dfaf82d Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 25 Nov 2023 20:13:18 +0900 Subject: [PATCH 01/18] gh-112205: Support @getter annotation from AC --- Modules/_io/bufferedio.c | 81 +++++++++++++------------------ Modules/_io/clinic/bufferedio.c.h | 56 ++++++++++++++++++++- Tools/clinic/clinic.py | 44 +++++++++++++++-- 3 files changed, 128 insertions(+), 53 deletions(-) diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c index 4f3786676d131f..a214a6dcec20a6 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -10,7 +10,6 @@ #include "Python.h" #include "pycore_bytesobject.h" // _PyBytes_Join() #include "pycore_call.h" // _PyObject_CallNoArgs() -#include "pycore_critical_section.h" // Py_BEGIN_CRITICAL_SECTION() #include "pycore_object.h" // _PyObject_GC_UNTRACK() #include "pycore_pyerrors.h" // _Py_FatalErrorFormat() #include "pycore_pylifecycle.h" // _Py_IsInterpreterFinalizing() @@ -518,25 +517,20 @@ buffered_closed(buffered *self) return closed; } +/*[clinic input] +@critical_section +@getter +_io._Buffered.closed +[clinic start generated code]*/ + static PyObject * -buffered_closed_get_impl(buffered *self, void *context) +_io__Buffered_closed_get_impl(buffered *self) +/*[clinic end generated code: output=f08ce57290703a1a input=18eddefdfe4a3d2f]*/ { CHECK_INITIALIZED(self) return PyObject_GetAttr(self->raw, &_Py_ID(closed)); } -static PyObject * -buffered_closed_get(buffered *self, void *context) -{ - PyObject *return_value = NULL; - - Py_BEGIN_CRITICAL_SECTION(self); - return_value = buffered_closed_get_impl(self, context); - Py_END_CRITICAL_SECTION(); - - return return_value; -} - /*[clinic input] @critical_section _io._Buffered.close @@ -662,44 +656,35 @@ _io__Buffered_writable_impl(buffered *self) return PyObject_CallMethodNoArgs(self->raw, &_Py_ID(writable)); } + +/*[clinic input] +@critical_section +@getter +_io._Buffered.name +[clinic start generated code]*/ + static PyObject * -buffered_name_get_impl(buffered *self, void *context) +_io__Buffered_name_get_impl(buffered *self) +/*[clinic end generated code: output=d2adf384051d3d10 input=6b84a0e6126f545e]*/ { CHECK_INITIALIZED(self) return PyObject_GetAttr(self->raw, &_Py_ID(name)); } -static PyObject * -buffered_name_get(buffered *self, void *context) -{ - PyObject *return_value = NULL; - - Py_BEGIN_CRITICAL_SECTION(self); - return_value = buffered_name_get_impl(self, context); - Py_END_CRITICAL_SECTION(); - - return return_value; -} +/*[clinic input] +@critical_section +@getter +_io._Buffered.mode +[clinic start generated code]*/ static PyObject * -buffered_mode_get_impl(buffered *self, void *context) +_io__Buffered_mode_get_impl(buffered *self) +/*[clinic end generated code: output=0feb205748892fa4 input=0762d5e28542fd8c]*/ { CHECK_INITIALIZED(self) return PyObject_GetAttr(self->raw, &_Py_ID(mode)); } -static PyObject * -buffered_mode_get(buffered *self, void *context) -{ - PyObject *return_value = NULL; - - Py_BEGIN_CRITICAL_SECTION(self); - return_value = buffered_mode_get_impl(self, context); - Py_END_CRITICAL_SECTION(); - - return return_value; -} - /* Lower-level APIs */ /*[clinic input] @@ -2541,9 +2526,9 @@ static PyMemberDef bufferedreader_members[] = { }; static PyGetSetDef bufferedreader_getset[] = { - {"closed", (getter)buffered_closed_get, NULL, NULL}, - {"name", (getter)buffered_name_get, NULL, NULL}, - {"mode", (getter)buffered_mode_get, NULL, NULL}, + _IO__BUFFERED_CLOSED_GET_GETTERDEF + _IO__BUFFERED_NAME_GET_GETTERDEF + _IO__BUFFERED_MODE_GET_GETTERDEF {NULL} }; @@ -2601,9 +2586,9 @@ static PyMemberDef bufferedwriter_members[] = { }; static PyGetSetDef bufferedwriter_getset[] = { - {"closed", (getter)buffered_closed_get, NULL, NULL}, - {"name", (getter)buffered_name_get, NULL, NULL}, - {"mode", (getter)buffered_mode_get, NULL, NULL}, + _IO__BUFFERED_CLOSED_GET_GETTERDEF + _IO__BUFFERED_NAME_GET_GETTERDEF + _IO__BUFFERED_MODE_GET_GETTERDEF {NULL} }; @@ -2719,9 +2704,9 @@ static PyMemberDef bufferedrandom_members[] = { }; static PyGetSetDef bufferedrandom_getset[] = { - {"closed", (getter)buffered_closed_get, NULL, NULL}, - {"name", (getter)buffered_name_get, NULL, NULL}, - {"mode", (getter)buffered_mode_get, NULL, NULL}, + _IO__BUFFERED_CLOSED_GET_GETTERDEF + _IO__BUFFERED_NAME_GET_GETTERDEF + _IO__BUFFERED_MODE_GET_GETTERDEF {NULL} }; diff --git a/Modules/_io/clinic/bufferedio.c.h b/Modules/_io/clinic/bufferedio.c.h index 20833a10139681..9fdf6157ad8465 100644 --- a/Modules/_io/clinic/bufferedio.c.h +++ b/Modules/_io/clinic/bufferedio.c.h @@ -327,6 +327,24 @@ _io__Buffered_simple_flush(buffered *self, PyObject *Py_UNUSED(ignored)) return return_value; } +#define _IO__BUFFERED_CLOSED_GET_GETTERDEF \ + {"closed", (getter)_io__Buffered_closed_get, NULL, NULL}, + +static PyObject * +_io__Buffered_closed_get_impl(buffered *self); + +static PyObject * +_io__Buffered_closed_get(buffered *self, void *context) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = _io__Buffered_closed_get_impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; +} + PyDoc_STRVAR(_io__Buffered_close__doc__, "close($self, /)\n" "--\n" @@ -442,6 +460,42 @@ _io__Buffered_writable(buffered *self, PyObject *Py_UNUSED(ignored)) return return_value; } +#define _IO__BUFFERED_NAME_GET_GETTERDEF \ + {"name", (getter)_io__Buffered_name_get, NULL, NULL}, + +static PyObject * +_io__Buffered_name_get_impl(buffered *self); + +static PyObject * +_io__Buffered_name_get(buffered *self, void *context) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = _io__Buffered_name_get_impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; +} + +#define _IO__BUFFERED_MODE_GET_GETTERDEF \ + {"mode", (getter)_io__Buffered_mode_get, NULL, NULL}, + +static PyObject * +_io__Buffered_mode_get_impl(buffered *self); + +static PyObject * +_io__Buffered_mode_get(buffered *self, void *context) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = _io__Buffered_mode_get_impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; +} + PyDoc_STRVAR(_io__Buffered_fileno__doc__, "fileno($self, /)\n" "--\n" @@ -1164,4 +1218,4 @@ _io_BufferedRandom___init__(PyObject *self, PyObject *args, PyObject *kwargs) exit: return return_value; } -/*[clinic end generated code: output=e8ad39a45531d7f2 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=b0d994daa11b05ca input=a9049054013a1b77]*/ diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index c0830864175adf..9e44f608b042a0 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -846,6 +846,10 @@ class CLanguage(Language): static PyObject * {c_basename}({self_type}{self_name}, PyObject *Py_UNUSED(ignored)) """) + PARSER_PROTOTYPE_GETTER: Final[str] = normalize_snippet(""" + static PyObject * + {c_basename}({self_type}{self_name}, void *context) + """) METH_O_PROTOTYPE: Final[str] = normalize_snippet(""" static PyObject * {c_basename}({impl_parameters}) @@ -865,6 +869,10 @@ class CLanguage(Language): #define {methoddef_name} \ {{"{name}", {methoddef_cast}{c_basename}{methoddef_cast_end}, {methoddef_flags}, {c_basename}__doc__}}, """) + GETTERDEF_PROTOTYPE_DEFINE: Final[str] = normalize_snippet(r""" + #define {methoddef_name} \ + {{"{name}", {methoddef_cast}{c_basename}{methoddef_cast_end}, NULL, NULL}}, + """) METHODDEF_PROTOTYPE_IFNDEF: Final[str] = normalize_snippet(""" #ifndef {methoddef_name} #define {methoddef_name} @@ -1161,6 +1169,9 @@ def output_templates( methoddef_define = self.METHODDEF_PROTOTYPE_DEFINE if new_or_init and not f.docstring: docstring_prototype = docstring_definition = '' + elif f.getter: + methoddef_define = self.GETTERDEF_PROTOTYPE_DEFINE + docstring_prototype = docstring_definition = '' else: docstring_prototype = self.DOCSTRING_PROTOTYPE_VAR docstring_definition = self.DOCSTRING_PROTOTYPE_STRVAR @@ -1217,7 +1228,11 @@ def parser_body( parsearg: str | None if not parameters: parser_code: list[str] | None - if not requires_defining_class: + if f.getter: + flags = "NULL" + parser_prototype = self.PARSER_PROTOTYPE_GETTER + parser_code = [] + elif not requires_defining_class: # no parameters, METH_NOARGS flags = "METH_NOARGS" parser_prototype = self.PARSER_PROTOTYPE_NOARGS @@ -1670,6 +1685,8 @@ def parser_body( methoddef_cast_end = "" if flags in ('METH_NOARGS', 'METH_O', 'METH_VARARGS'): methoddef_cast = "(PyCFunction)" + elif f.getter: + methoddef_cast = "(getter)" elif limited_capi: methoddef_cast = "(PyCFunction)(void(*)(void))" else: @@ -1928,7 +1945,10 @@ def render_function( template_dict = {'full_name': full_name} template_dict['name'] = f.displayname template_dict['c_basename'] = f.c_basename - template_dict['methoddef_name'] = f.c_basename.upper() + "_METHODDEF" + if f.getter: + template_dict['methoddef_name'] = f.c_basename.upper() + "_GETTERDEF" + else: + template_dict['methoddef_name'] = f.c_basename.upper() + "_METHODDEF" template_dict['docstring'] = self.docstring_for_c_string(f) @@ -2932,6 +2952,7 @@ class FunctionKind(enum.Enum): CLASS_METHOD = enum.auto() METHOD_INIT = enum.auto() METHOD_NEW = enum.auto() + GETTER = enum.auto() @functools.cached_property def new_or_init(self) -> bool: @@ -2947,6 +2968,7 @@ def __repr__(self) -> str: CLASS_METHOD: Final = FunctionKind.CLASS_METHOD METHOD_INIT: Final = FunctionKind.METHOD_INIT METHOD_NEW: Final = FunctionKind.METHOD_NEW +GETTER: Final = FunctionKind.GETTER ParamDict = dict[str, "Parameter"] ReturnConverterType = Callable[..., "CReturnConverter"] @@ -2983,6 +3005,7 @@ class Function: docstring_only: bool = False critical_section: bool = False target_critical_section: list[str] = dc.field(default_factory=list) + getter: bool = False def __post_init__(self) -> None: self.parent = self.cls or self.module @@ -3032,6 +3055,8 @@ def methoddef_flags(self) -> str | None: flags.append('METH_CLASS') case FunctionKind.STATIC_METHOD: flags.append('METH_STATIC') + case FunctionKind.GETTER: + pass case _ as kind: assert kind is FunctionKind.CALLABLE, f"unknown kind: {kind!r}" if self.coexist: @@ -4678,7 +4703,7 @@ def parse_arg(self, argname: str, displayname: str, *, limited_capi: bool) -> st def correct_name_for_self( f: Function ) -> tuple[str, str]: - if f.kind in (CALLABLE, METHOD_INIT): + if f.kind in (CALLABLE, METHOD_INIT, GETTER): if f.cls: return "PyObject *", "self" return "PyObject *", "module" @@ -5140,6 +5165,7 @@ class DSLParser: preserve_output: bool critical_section: bool target_critical_section: list[str] + getter: bool from_version_re = re.compile(r'([*/]) +\[from +(.+)\]') def __init__(self, clinic: Clinic) -> None: @@ -5176,6 +5202,7 @@ def reset(self) -> None: self.preserve_output = False self.critical_section = False self.target_critical_section = [] + self.getter = False def directive_version(self, required: str) -> None: global version @@ -5310,6 +5337,9 @@ def at_critical_section(self, *args: str) -> None: self.target_critical_section.extend(args) self.critical_section = True + def at_getter(self) -> None: + self.getter = True + def at_staticmethod(self) -> None: if self.kind is not CALLABLE: fail("Can't set @staticmethod, function is not a normal callable") @@ -5427,6 +5457,9 @@ def update_function_kind(self, fullname: str) -> None: if (self.kind is not CALLABLE) or (not cls): fail("'__init__' must be a normal method, not a class or static method!") self.kind = METHOD_INIT + elif self.getter and cls: + self.kind = GETTER + def state_modulename_name(self, line: str) -> None: # looking for declaration, which establishes the leftmost column @@ -5500,6 +5533,8 @@ def state_modulename_name(self, line: str) -> None: line, _, returns = line.partition('->') returns = returns.strip() full_name, c_basename = self.parse_function_names(line) + if self.getter: + c_basename += '_get' return_converter = None if returns: @@ -5534,7 +5569,8 @@ def state_modulename_name(self, line: str) -> None: self.function = Function(name=function_name, full_name=full_name, module=module, cls=cls, c_basename=c_basename, return_converter=return_converter, kind=self.kind, coexist=self.coexist, critical_section=self.critical_section, - target_critical_section=self.target_critical_section) + target_critical_section=self.target_critical_section, + getter=self.getter) self.block.signatures.append(self.function) # insert a self converter automatically From ea839738202add5e5b4841649eeb9b0cf61efaa4 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 25 Nov 2023 20:28:20 +0900 Subject: [PATCH 02/18] Update define macro --- Modules/_io/bufferedio.c | 18 +++++++++--------- Modules/_io/clinic/bufferedio.c.h | 8 ++++---- Tools/clinic/clinic.py | 5 ++--- 3 files changed, 15 insertions(+), 16 deletions(-) diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c index a214a6dcec20a6..679626863c385c 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -2526,9 +2526,9 @@ static PyMemberDef bufferedreader_members[] = { }; static PyGetSetDef bufferedreader_getset[] = { - _IO__BUFFERED_CLOSED_GET_GETTERDEF - _IO__BUFFERED_NAME_GET_GETTERDEF - _IO__BUFFERED_MODE_GET_GETTERDEF + _IO__BUFFERED_CLOSED_GETTERDEF + _IO__BUFFERED_NAME_GETTERDEF + _IO__BUFFERED_MODE_GETTERDEF {NULL} }; @@ -2586,9 +2586,9 @@ static PyMemberDef bufferedwriter_members[] = { }; static PyGetSetDef bufferedwriter_getset[] = { - _IO__BUFFERED_CLOSED_GET_GETTERDEF - _IO__BUFFERED_NAME_GET_GETTERDEF - _IO__BUFFERED_MODE_GET_GETTERDEF + _IO__BUFFERED_CLOSED_GETTERDEF + _IO__BUFFERED_NAME_GETTERDEF + _IO__BUFFERED_MODE_GETTERDEF {NULL} }; @@ -2704,9 +2704,9 @@ static PyMemberDef bufferedrandom_members[] = { }; static PyGetSetDef bufferedrandom_getset[] = { - _IO__BUFFERED_CLOSED_GET_GETTERDEF - _IO__BUFFERED_NAME_GET_GETTERDEF - _IO__BUFFERED_MODE_GET_GETTERDEF + _IO__BUFFERED_CLOSED_GETTERDEF + _IO__BUFFERED_NAME_GETTERDEF + _IO__BUFFERED_MODE_GETTERDEF {NULL} }; diff --git a/Modules/_io/clinic/bufferedio.c.h b/Modules/_io/clinic/bufferedio.c.h index 9fdf6157ad8465..70275fbd2fe3dc 100644 --- a/Modules/_io/clinic/bufferedio.c.h +++ b/Modules/_io/clinic/bufferedio.c.h @@ -327,7 +327,7 @@ _io__Buffered_simple_flush(buffered *self, PyObject *Py_UNUSED(ignored)) return return_value; } -#define _IO__BUFFERED_CLOSED_GET_GETTERDEF \ +#define _IO__BUFFERED_CLOSED_GETTERDEF \ {"closed", (getter)_io__Buffered_closed_get, NULL, NULL}, static PyObject * @@ -460,7 +460,7 @@ _io__Buffered_writable(buffered *self, PyObject *Py_UNUSED(ignored)) return return_value; } -#define _IO__BUFFERED_NAME_GET_GETTERDEF \ +#define _IO__BUFFERED_NAME_GETTERDEF \ {"name", (getter)_io__Buffered_name_get, NULL, NULL}, static PyObject * @@ -478,7 +478,7 @@ _io__Buffered_name_get(buffered *self, void *context) return return_value; } -#define _IO__BUFFERED_MODE_GET_GETTERDEF \ +#define _IO__BUFFERED_MODE_GETTERDEF \ {"mode", (getter)_io__Buffered_mode_get, NULL, NULL}, static PyObject * @@ -1218,4 +1218,4 @@ _io_BufferedRandom___init__(PyObject *self, PyObject *args, PyObject *kwargs) exit: return return_value; } -/*[clinic end generated code: output=b0d994daa11b05ca input=a9049054013a1b77]*/ +/*[clinic end generated code: output=03bb6139474e961a input=a9049054013a1b77]*/ diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 9e44f608b042a0..11f94c6487b6f7 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -1944,11 +1944,12 @@ def render_function( full_name = f.full_name template_dict = {'full_name': full_name} template_dict['name'] = f.displayname - template_dict['c_basename'] = f.c_basename if f.getter: template_dict['methoddef_name'] = f.c_basename.upper() + "_GETTERDEF" + template_dict['c_basename'] = f.c_basename + "_get" else: template_dict['methoddef_name'] = f.c_basename.upper() + "_METHODDEF" + template_dict['c_basename'] = f.c_basename template_dict['docstring'] = self.docstring_for_c_string(f) @@ -5533,8 +5534,6 @@ def state_modulename_name(self, line: str) -> None: line, _, returns = line.partition('->') returns = returns.strip() full_name, c_basename = self.parse_function_names(line) - if self.getter: - c_basename += '_get' return_converter = None if returns: From 969adf9a9d276d37f6b7e8c9580b0de2556342fc Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 25 Nov 2023 20:58:11 +0900 Subject: [PATCH 03/18] Apply suggestions from code review Co-authored-by: Alex Waygood --- Tools/clinic/clinic.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 11f94c6487b6f7..88ec9b79efc800 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -4704,7 +4704,7 @@ def parse_arg(self, argname: str, displayname: str, *, limited_capi: bool) -> st def correct_name_for_self( f: Function ) -> tuple[str, str]: - if f.kind in (CALLABLE, METHOD_INIT, GETTER): + if f.kind in {CALLABLE, METHOD_INIT, GETTER}: if f.cls: return "PyObject *", "self" return "PyObject *", "module" @@ -5461,7 +5461,6 @@ def update_function_kind(self, fullname: str) -> None: elif self.getter and cls: self.kind = GETTER - def state_modulename_name(self, line: str) -> None: # looking for declaration, which establishes the leftmost column # line should be From 716b8d9b8eb0a420e3dad10a52de14572a055b04 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 25 Nov 2023 20:59:37 +0900 Subject: [PATCH 04/18] nit --- Tools/clinic/clinic.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 88ec9b79efc800..b844970cc84c2f 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -3056,10 +3056,8 @@ def methoddef_flags(self) -> str | None: flags.append('METH_CLASS') case FunctionKind.STATIC_METHOD: flags.append('METH_STATIC') - case FunctionKind.GETTER: - pass case _ as kind: - assert kind is FunctionKind.CALLABLE, f"unknown kind: {kind!r}" + assert kind in {FunctionKind.CALLABLE, FunctionKind.GETTER}, f"unknown kind: {kind!r}" if self.coexist: flags.append('METH_COEXIST') return '|'.join(flags) From c5fe2b2c461107d21ba4cd2c13070f43b8661344 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 25 Nov 2023 22:00:21 +0900 Subject: [PATCH 05/18] Add test case --- Lib/test/clinic.test.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/Lib/test/clinic.test.c b/Lib/test/clinic.test.c index 81f88c4d1535ce..fd1cd99d10d61b 100644 --- a/Lib/test/clinic.test.c +++ b/Lib/test/clinic.test.c @@ -4951,6 +4951,27 @@ static PyObject * Test_meth_coexist_impl(TestObj *self) /*[clinic end generated code: output=808a293d0cd27439 input=2a1d75b5e6fec6dd]*/ +/*[clinic input] +@getter +Test.property +[clinic start generated code]*/ + +#define TEST_PROPERTY_GETTERDEF \ + {"property", (getter)Test_property_get, NULL, NULL}, + +static PyObject * +Test_property_get_impl(TestObj *self); + +static PyObject * +Test_property_get(TestObj *self, void *context) +{ + return Test_property_get_impl(self); +} + +static PyObject * +Test_property_get_impl(TestObj *self) +/*[clinic end generated code: output=0e6435be1a183de9 input=2d92b3449fbc7d2b]*/ + /*[clinic input] output push From 2d9800d4b578bb0a961846bb18c871ce2f02c774 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 25 Nov 2023 22:22:41 +0900 Subject: [PATCH 06/18] Add test case --- Lib/test/test_clinic.py | 17 ++++++++++------- Tools/clinic/clinic.py | 4 ++-- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index da957fcebaa296..2463ef55a701ea 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -2181,13 +2181,16 @@ class Foo "" "" def test_init_must_be_a_normal_method(self): err = "'__init__' must be a normal method, not a class or static method!" - block = """ - module foo - class Foo "" "" - @classmethod - Foo.__init__ - """ - self.expect_failure(block, err, lineno=3) + annotations = ["@classmethod", "@staticmethod", "@getter"] + for annotation in annotations: + with self.subTest(annotation=annotation): + block = f""" + module foo + class Foo "" "" + {annotation} + Foo.__init__ + """ + self.expect_failure(block, err, lineno=3) def test_duplicate_coexist(self): err = "Called @coexist twice" diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index b844970cc84c2f..72f0aa1122e396 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -5449,11 +5449,11 @@ def update_function_kind(self, fullname: str) -> None: if name in unsupported_special_methods: fail(f"{name!r} is a special method and cannot be converted to Argument Clinic!") if name == '__new__': - if (self.kind is not CLASS_METHOD) or (not cls): + if (self.kind is not CLASS_METHOD) or (not cls) or self.getter: fail("'__new__' must be a class method!") self.kind = METHOD_NEW elif name == '__init__': - if (self.kind is not CALLABLE) or (not cls): + if (self.kind is not CALLABLE) or (not cls) or self.getter: fail("'__init__' must be a normal method, not a class or static method!") self.kind = METHOD_INIT elif self.getter and cls: From 442c57583fbac0977f35136542f38aa139690c3e Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 25 Nov 2023 22:29:44 +0900 Subject: [PATCH 07/18] Clean up --- Tools/clinic/clinic.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 72f0aa1122e396..c46f0eece2037f 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -5448,14 +5448,17 @@ def update_function_kind(self, fullname: str) -> None: _, cls = self.clinic._module_and_class(fields) if name in unsupported_special_methods: fail(f"{name!r} is a special method and cannot be converted to Argument Clinic!") + if name == '__new__': - if (self.kind is not CLASS_METHOD) or (not cls) or self.getter: + if (self.kind is CLASS_METHOD) and cls and not self.getter: + self.kind = METHOD_NEW + else: fail("'__new__' must be a class method!") - self.kind = METHOD_NEW elif name == '__init__': - if (self.kind is not CALLABLE) or (not cls) or self.getter: + if (self.kind is CALLABLE) and cls and not self.getter: + self.kind = METHOD_INIT + else: fail("'__init__' must be a normal method, not a class or static method!") - self.kind = METHOD_INIT elif self.getter and cls: self.kind = GETTER From e0fb271e704c0a7a0b1f253e2af8689359c237f0 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 25 Nov 2023 22:31:12 +0900 Subject: [PATCH 08/18] Update Tools/clinic/clinic.py Co-authored-by: Alex Waygood --- Tools/clinic/clinic.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index c46f0eece2037f..b6801579ec77dc 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -5458,7 +5458,10 @@ def update_function_kind(self, fullname: str) -> None: if (self.kind is CALLABLE) and cls and not self.getter: self.kind = METHOD_INIT else: - fail("'__init__' must be a normal method, not a class or static method!") + fail( + "'__init__' must be a normal method, " + "not a classmethod, staticmethod or getter!" + ) elif self.getter and cls: self.kind = GETTER From 0778eb7b3f579543eddb94efb27c3faf8a0d0824 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 25 Nov 2023 22:41:37 +0900 Subject: [PATCH 09/18] Update logic --- Lib/test/test_clinic.py | 4 ++-- Tools/clinic/clinic.py | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index 2463ef55a701ea..594bc02105e9e9 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -638,7 +638,7 @@ class C "void *" "" C.__init__ = C.meth [clinic start generated code]*/ """ - err = "'__init__' must be a normal method, not a class or static method" + err = "'__init__' must be a normal method, not a classmethod, staticmethod or getter!" self.expect_failure(block, err, lineno=8) def test_validate_cloned_new(self): @@ -2180,7 +2180,7 @@ class Foo "" "" self.expect_failure(block, err, lineno=2) def test_init_must_be_a_normal_method(self): - err = "'__init__' must be a normal method, not a class or static method!" + err = "'__init__' must be a normal method, not a classmethod, staticmethod or getter!" annotations = ["@classmethod", "@staticmethod", "@getter"] for annotation in annotations: with self.subTest(annotation=annotation): diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index b6801579ec77dc..1b8bd37a379a76 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -5338,6 +5338,7 @@ def at_critical_section(self, *args: str) -> None: def at_getter(self) -> None: self.getter = True + self.kind = GETTER def at_staticmethod(self) -> None: if self.kind is not CALLABLE: @@ -5462,8 +5463,6 @@ def update_function_kind(self, fullname: str) -> None: "'__init__' must be a normal method, " "not a classmethod, staticmethod or getter!" ) - elif self.getter and cls: - self.kind = GETTER def state_modulename_name(self, line: str) -> None: # looking for declaration, which establishes the leftmost column From d5b17511d2545aac65bf6b6fecf8b96bd82edfea Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 25 Nov 2023 22:45:19 +0900 Subject: [PATCH 10/18] Update --- Tools/clinic/clinic.py | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 1b8bd37a379a76..56d3b9d3b82cf8 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -1169,7 +1169,7 @@ def output_templates( methoddef_define = self.METHODDEF_PROTOTYPE_DEFINE if new_or_init and not f.docstring: docstring_prototype = docstring_definition = '' - elif f.getter: + elif f.kind == GETTER: methoddef_define = self.GETTERDEF_PROTOTYPE_DEFINE docstring_prototype = docstring_definition = '' else: @@ -1228,7 +1228,7 @@ def parser_body( parsearg: str | None if not parameters: parser_code: list[str] | None - if f.getter: + if f.kind == GETTER: flags = "NULL" parser_prototype = self.PARSER_PROTOTYPE_GETTER parser_code = [] @@ -1685,7 +1685,7 @@ def parser_body( methoddef_cast_end = "" if flags in ('METH_NOARGS', 'METH_O', 'METH_VARARGS'): methoddef_cast = "(PyCFunction)" - elif f.getter: + elif f.kind == GETTER: methoddef_cast = "(getter)" elif limited_capi: methoddef_cast = "(PyCFunction)(void(*)(void))" @@ -1944,7 +1944,7 @@ def render_function( full_name = f.full_name template_dict = {'full_name': full_name} template_dict['name'] = f.displayname - if f.getter: + if f.kind == GETTER: template_dict['methoddef_name'] = f.c_basename.upper() + "_GETTERDEF" template_dict['c_basename'] = f.c_basename + "_get" else: @@ -5201,7 +5201,6 @@ def reset(self) -> None: self.preserve_output = False self.critical_section = False self.target_critical_section = [] - self.getter = False def directive_version(self, required: str) -> None: global version @@ -5337,7 +5336,6 @@ def at_critical_section(self, *args: str) -> None: self.critical_section = True def at_getter(self) -> None: - self.getter = True self.kind = GETTER def at_staticmethod(self) -> None: @@ -5451,12 +5449,12 @@ def update_function_kind(self, fullname: str) -> None: fail(f"{name!r} is a special method and cannot be converted to Argument Clinic!") if name == '__new__': - if (self.kind is CLASS_METHOD) and cls and not self.getter: + if (self.kind is CLASS_METHOD) and cls: self.kind = METHOD_NEW else: fail("'__new__' must be a class method!") elif name == '__init__': - if (self.kind is CALLABLE) and cls and not self.getter: + if (self.kind is CALLABLE) and cls: self.kind = METHOD_INIT else: fail( @@ -5570,8 +5568,7 @@ def state_modulename_name(self, line: str) -> None: self.function = Function(name=function_name, full_name=full_name, module=module, cls=cls, c_basename=c_basename, return_converter=return_converter, kind=self.kind, coexist=self.coexist, critical_section=self.critical_section, - target_critical_section=self.target_critical_section, - getter=self.getter) + target_critical_section=self.target_critical_section) self.block.signatures.append(self.function) # insert a self converter automatically From 7f4bfa8beb03a8bef1113f252a1e4868aab2ed96 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 25 Nov 2023 22:47:23 +0900 Subject: [PATCH 11/18] Clean up --- Tools/clinic/clinic.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 56d3b9d3b82cf8..7ca08b61d0e34e 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -1169,7 +1169,7 @@ def output_templates( methoddef_define = self.METHODDEF_PROTOTYPE_DEFINE if new_or_init and not f.docstring: docstring_prototype = docstring_definition = '' - elif f.kind == GETTER: + elif f.kind is GETTER: methoddef_define = self.GETTERDEF_PROTOTYPE_DEFINE docstring_prototype = docstring_definition = '' else: @@ -1228,7 +1228,7 @@ def parser_body( parsearg: str | None if not parameters: parser_code: list[str] | None - if f.kind == GETTER: + if f.kind is GETTER: flags = "NULL" parser_prototype = self.PARSER_PROTOTYPE_GETTER parser_code = [] @@ -1685,7 +1685,7 @@ def parser_body( methoddef_cast_end = "" if flags in ('METH_NOARGS', 'METH_O', 'METH_VARARGS'): methoddef_cast = "(PyCFunction)" - elif f.kind == GETTER: + elif f.kind is GETTER: methoddef_cast = "(getter)" elif limited_capi: methoddef_cast = "(PyCFunction)(void(*)(void))" @@ -1944,7 +1944,7 @@ def render_function( full_name = f.full_name template_dict = {'full_name': full_name} template_dict['name'] = f.displayname - if f.kind == GETTER: + if f.kind is GETTER: template_dict['methoddef_name'] = f.c_basename.upper() + "_GETTERDEF" template_dict['c_basename'] = f.c_basename + "_get" else: @@ -3006,7 +3006,6 @@ class Function: docstring_only: bool = False critical_section: bool = False target_critical_section: list[str] = dc.field(default_factory=list) - getter: bool = False def __post_init__(self) -> None: self.parent = self.cls or self.module @@ -5164,7 +5163,6 @@ class DSLParser: preserve_output: bool critical_section: bool target_critical_section: list[str] - getter: bool from_version_re = re.compile(r'([*/]) +\[from +(.+)\]') def __init__(self, clinic: Clinic) -> None: From fe46de03a7808dd0cc2aab869a1a1d727d7c9493 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Mon, 27 Nov 2023 20:24:52 +0900 Subject: [PATCH 12/18] Update Tools/clinic/clinic.py Co-authored-by: Alex Waygood --- Tools/clinic/clinic.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 7ca08b61d0e34e..9b928ac41ab15f 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -3056,7 +3056,8 @@ def methoddef_flags(self) -> str | None: case FunctionKind.STATIC_METHOD: flags.append('METH_STATIC') case _ as kind: - assert kind in {FunctionKind.CALLABLE, FunctionKind.GETTER}, f"unknown kind: {kind!r}" + acceptable_kinds = {FunctionKind.CALLABLE, FunctionKind.GETTER} + assert kind in acceptable_kinds, f"unknown kind: {kind!r}" if self.coexist: flags.append('METH_COEXIST') return '|'.join(flags) From 0892d0df11ea03a465b14570ab3e2d5d12f65444 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Mon, 27 Nov 2023 21:46:57 +0900 Subject: [PATCH 13/18] Address code review --- Lib/test/test_clinic.py | 17 +++++++++++------ Tools/clinic/clinic.py | 4 ++-- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index 594bc02105e9e9..f53e9481083106 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -638,7 +638,7 @@ class C "void *" "" C.__init__ = C.meth [clinic start generated code]*/ """ - err = "'__init__' must be a normal method, not a classmethod, staticmethod or getter!" + err = "'__init__' must be a normal method; got 'FunctionKind.CLASS_METHOD'!" self.expect_failure(block, err, lineno=8) def test_validate_cloned_new(self): @@ -2180,17 +2180,22 @@ class Foo "" "" self.expect_failure(block, err, lineno=2) def test_init_must_be_a_normal_method(self): - err = "'__init__' must be a normal method, not a classmethod, staticmethod or getter!" - annotations = ["@classmethod", "@staticmethod", "@getter"] - for annotation in annotations: - with self.subTest(annotation=annotation): + err_template = "'__init__' must be a normal method; got 'FunctionKind.{}'!" + annotations = { + "@classmethod": "CLASS_METHOD", + "@staticmethod": "STATIC_METHOD", + "@getter": "GETTER", + } + for annotation, invalid_kind in annotations.items(): + with self.subTest(annotation=annotation, invalid_kind=invalid_kind): block = f""" module foo class Foo "" "" {annotation} Foo.__init__ """ - self.expect_failure(block, err, lineno=3) + expected_error = err_template.format(invalid_kind) + self.expect_failure(block, expected_error, lineno=3) def test_duplicate_coexist(self): err = "Called @coexist twice" diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 9b928ac41ab15f..1f2e5e5adc9554 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -5457,8 +5457,8 @@ def update_function_kind(self, fullname: str) -> None: self.kind = METHOD_INIT else: fail( - "'__init__' must be a normal method, " - "not a classmethod, staticmethod or getter!" + "'__init__' must be a normal method; " + f"got '{self.kind}'!" ) def state_modulename_name(self, line: str) -> None: From 769f6cda5cf74d3cb5108bc637cd1e31e0dba89b Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Tue, 28 Nov 2023 20:44:11 +0900 Subject: [PATCH 14/18] Address code review --- Tools/clinic/clinic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 1f2e5e5adc9554..e66165b674e5d5 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -1229,7 +1229,7 @@ def parser_body( if not parameters: parser_code: list[str] | None if f.kind is GETTER: - flags = "NULL" + flags = "" # This should end up unused parser_prototype = self.PARSER_PROTOTYPE_GETTER parser_code = [] elif not requires_defining_class: From dde933e3e4d73ceb7ea00a876303c8908ca795c5 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Tue, 28 Nov 2023 23:07:53 +0900 Subject: [PATCH 15/18] Address code review --- Tools/clinic/clinic.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index e66165b674e5d5..4932b80807f9a7 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -871,7 +871,7 @@ class CLanguage(Language): """) GETTERDEF_PROTOTYPE_DEFINE: Final[str] = normalize_snippet(r""" #define {methoddef_name} \ - {{"{name}", {methoddef_cast}{c_basename}{methoddef_cast_end}, NULL, NULL}}, + {{"{name}", (getter){c_basename}, NULL, NULL}}, """) METHODDEF_PROTOTYPE_IFNDEF: Final[str] = normalize_snippet(""" #ifndef {methoddef_name} @@ -1686,7 +1686,7 @@ def parser_body( if flags in ('METH_NOARGS', 'METH_O', 'METH_VARARGS'): methoddef_cast = "(PyCFunction)" elif f.kind is GETTER: - methoddef_cast = "(getter)" + methoddef_cast = "" # This should end up unused elif limited_capi: methoddef_cast = "(PyCFunction)(void(*)(void))" else: From d381b5efb206b410b6020bd0ef9be77889c88f23 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Tue, 28 Nov 2023 23:10:36 +0900 Subject: [PATCH 16/18] Change template key --- Tools/clinic/clinic.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 4932b80807f9a7..514e1600e972d2 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -870,7 +870,7 @@ class CLanguage(Language): {{"{name}", {methoddef_cast}{c_basename}{methoddef_cast_end}, {methoddef_flags}, {c_basename}__doc__}}, """) GETTERDEF_PROTOTYPE_DEFINE: Final[str] = normalize_snippet(r""" - #define {methoddef_name} \ + #define {getter_name} \ {{"{name}", (getter){c_basename}, NULL, NULL}}, """) METHODDEF_PROTOTYPE_IFNDEF: Final[str] = normalize_snippet(""" @@ -1945,7 +1945,7 @@ def render_function( template_dict = {'full_name': full_name} template_dict['name'] = f.displayname if f.kind is GETTER: - template_dict['methoddef_name'] = f.c_basename.upper() + "_GETTERDEF" + template_dict['getter_name'] = f.c_basename.upper() + "_GETTERDEF" template_dict['c_basename'] = f.c_basename + "_get" else: template_dict['methoddef_name'] = f.c_basename.upper() + "_METHODDEF" From 351ba0847c1b03003abf5bfd1bcbee9094f4afe4 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Wed, 29 Nov 2023 06:51:21 +0900 Subject: [PATCH 17/18] Use Py_UNUSED for context --- Modules/_io/clinic/bufferedio.c.h | 8 ++++---- Tools/clinic/clinic.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Modules/_io/clinic/bufferedio.c.h b/Modules/_io/clinic/bufferedio.c.h index 70275fbd2fe3dc..69d28ad00c2ad5 100644 --- a/Modules/_io/clinic/bufferedio.c.h +++ b/Modules/_io/clinic/bufferedio.c.h @@ -334,7 +334,7 @@ static PyObject * _io__Buffered_closed_get_impl(buffered *self); static PyObject * -_io__Buffered_closed_get(buffered *self, void *context) +_io__Buffered_closed_get(buffered *self, void *Py_UNUSED(context)) { PyObject *return_value = NULL; @@ -467,7 +467,7 @@ static PyObject * _io__Buffered_name_get_impl(buffered *self); static PyObject * -_io__Buffered_name_get(buffered *self, void *context) +_io__Buffered_name_get(buffered *self, void *Py_UNUSED(context)) { PyObject *return_value = NULL; @@ -485,7 +485,7 @@ static PyObject * _io__Buffered_mode_get_impl(buffered *self); static PyObject * -_io__Buffered_mode_get(buffered *self, void *context) +_io__Buffered_mode_get(buffered *self, void *Py_UNUSED(context)) { PyObject *return_value = NULL; @@ -1218,4 +1218,4 @@ _io_BufferedRandom___init__(PyObject *self, PyObject *args, PyObject *kwargs) exit: return return_value; } -/*[clinic end generated code: output=03bb6139474e961a input=a9049054013a1b77]*/ +/*[clinic end generated code: output=f21ed03255032b43 input=a9049054013a1b77]*/ diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 514e1600e972d2..54962c9e1c92f9 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -848,7 +848,7 @@ class CLanguage(Language): """) PARSER_PROTOTYPE_GETTER: Final[str] = normalize_snippet(""" static PyObject * - {c_basename}({self_type}{self_name}, void *context) + {c_basename}({self_type}{self_name}, void *Py_UNUSED(context)) """) METH_O_PROTOTYPE: Final[str] = normalize_snippet(""" static PyObject * From f53a03597f245d5bf9d3e2eb1294fbff4561e972 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Wed, 29 Nov 2023 07:08:15 +0900 Subject: [PATCH 18/18] Update test --- Lib/test/clinic.test.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/clinic.test.c b/Lib/test/clinic.test.c index fd1cd99d10d61b..ee4a4228fd28be 100644 --- a/Lib/test/clinic.test.c +++ b/Lib/test/clinic.test.c @@ -4963,14 +4963,14 @@ static PyObject * Test_property_get_impl(TestObj *self); static PyObject * -Test_property_get(TestObj *self, void *context) +Test_property_get(TestObj *self, void *Py_UNUSED(context)) { return Test_property_get_impl(self); } static PyObject * Test_property_get_impl(TestObj *self) -/*[clinic end generated code: output=0e6435be1a183de9 input=2d92b3449fbc7d2b]*/ +/*[clinic end generated code: output=892b6fb351ff85fd input=2d92b3449fbc7d2b]*/ /*[clinic input]