From fe8f906a1fb23439b0a1628c41a2ec6f1c3ae879 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 15 Mar 2024 08:52:09 +0100 Subject: [PATCH 1/8] gh-113317: Change how Argument Clinic lists converters * Add a new create_python_parser_namespace() function for PythonParser to pass objects to executed code. * In run_clinic(), list converters using 'converters' and 'return_converters' dictionarties. * test_clinic: add 'object()' return converter. --- Lib/test/test_clinic.py | 1 + Tools/clinic/clinic.py | 65 ++++++++++++++++++++++++----------------- 2 files changed, 40 insertions(+), 26 deletions(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index a60f087ef2816e..52cb4d6e187855 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -2657,6 +2657,7 @@ def test_cli_converters(self): float() int() long() + object() Py_ssize_t() size_t() unsigned_int() diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index c9641cb9c82bf7..2b8ae8804a28c8 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -1988,13 +1988,30 @@ def parse_file( libclinic.write_file(output, cooked) +def create_python_parser_namespace(): + ns = dict( + CConverter=CConverter, + CReturnConverter=CReturnConverter, + buffer=buffer, + robuffer=robuffer, + rwbuffer=rwbuffer, + unspecified=unspecified, + ) + for name, converter in converters.items(): + ns[f'{name}_converter'] = converter + for name, converter in return_converters.items(): + ns[f'{name}_return_converter'] = converter + return ns + + class PythonParser: def __init__(self, clinic: Clinic) -> None: - pass + self.namespace = create_python_parser_namespace() def parse(self, block: Block) -> None: + ns = dict(self.namespace) with contextlib.redirect_stdout(io.StringIO()) as s: - exec(block.input) + exec(block.input, ns) block.output = s.getvalue() @@ -4984,25 +5001,22 @@ def run_clinic(parser: argparse.ArgumentParser, ns: argparse.Namespace) -> None: parser.error( "can't specify --converters and a filename at the same time" ) - converters: list[tuple[str, str]] = [] - return_converters: list[tuple[str, str]] = [] - ignored = set(""" - add_c_converter - add_c_return_converter - add_default_legacy_c_converter - add_legacy_c_converter - """.strip().split()) - module = globals() - for name in module: - for suffix, ids in ( - ("_return_converter", return_converters), - ("_converter", converters), - ): - if name in ignored: - continue - if name.endswith(suffix): - ids.append((name, name.removesuffix(suffix))) - break + converter_list: list[tuple[str, str]] = [] + return_converter_list: list[tuple[str, str]] = [] + + for name, converter in converters.items(): + converter_list.append(( + f'{name}_converter', + name, + converter, + )) + for name, converter in return_converters.items(): + return_converter_list.append(( + f'{name}_return_converter', + name, + converter + )) + print() print("Legacy converters:") @@ -5012,15 +5026,14 @@ def run_clinic(parser: argparse.ArgumentParser, ns: argparse.Namespace) -> None: print() for title, attribute, ids in ( - ("Converters", 'converter_init', converters), - ("Return converters", 'return_converter_init', return_converters), + ("Converters", 'converter_init', converter_list), + ("Return converters", 'return_converter_init', return_converter_list), ): print(title + ":") longest = -1 - for name, short_name in ids: + for name, short_name, converter in ids: longest = max(longest, len(short_name)) - for name, short_name in sorted(ids, key=lambda x: x[1].lower()): - cls = module[name] + for name, short_name, cls in sorted(ids, key=lambda x: x[1].lower()): callable = getattr(cls, attribute, None) if not callable: continue From 61fff9da6d5caac01f58aec7b37a2114b00a1b1b Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 15 Mar 2024 16:11:48 +0100 Subject: [PATCH 2/8] Fix mypy: add type annotations --- Tools/clinic/clinic.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 2b8ae8804a28c8..c2f254fbef1715 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -1988,7 +1988,7 @@ def parse_file( libclinic.write_file(output, cooked) -def create_python_parser_namespace(): +def create_python_parser_namespace() -> dict[str, Any]: ns = dict( CConverter=CConverter, CReturnConverter=CReturnConverter, @@ -1999,8 +1999,8 @@ def create_python_parser_namespace(): ) for name, converter in converters.items(): ns[f'{name}_converter'] = converter - for name, converter in return_converters.items(): - ns[f'{name}_return_converter'] = converter + for name, return_converter in return_converters.items(): + ns[f'{name}_return_converter'] = return_converter return ns @@ -5001,8 +5001,8 @@ def run_clinic(parser: argparse.ArgumentParser, ns: argparse.Namespace) -> None: parser.error( "can't specify --converters and a filename at the same time" ) - converter_list: list[tuple[str, str]] = [] - return_converter_list: list[tuple[str, str]] = [] + converter_list: list[tuple[str, str, Any]] = [] + return_converter_list: list[tuple[str, str, Any]] = [] for name, converter in converters.items(): converter_list.append(( @@ -5010,11 +5010,11 @@ def run_clinic(parser: argparse.ArgumentParser, ns: argparse.Namespace) -> None: name, converter, )) - for name, converter in return_converters.items(): + for name, return_converter in return_converters.items(): return_converter_list.append(( f'{name}_return_converter', name, - converter + return_converter )) print() From 784679203786a32a2c6af86b567e14fec155920c Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 19 Mar 2024 22:47:44 +0100 Subject: [PATCH 3/8] Apply suggestions from code review Co-authored-by: Erlend E. Aasland --- 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 c2f254fbef1715..ea7dc8df8b974b 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -1988,7 +1988,7 @@ def parse_file( libclinic.write_file(output, cooked) -def create_python_parser_namespace() -> dict[str, Any]: +def create_parser_namespace() -> dict[str, Any]: ns = dict( CConverter=CConverter, CReturnConverter=CReturnConverter, @@ -2006,7 +2006,7 @@ def create_python_parser_namespace() -> dict[str, Any]: class PythonParser: def __init__(self, clinic: Clinic) -> None: - self.namespace = create_python_parser_namespace() + self.namespace = create_parser_namespace() def parse(self, block: Block) -> None: ns = dict(self.namespace) From 52da03c2a6d65b22c996527b03e0cccdd01b75ee Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Fri, 22 Mar 2024 01:06:16 +0100 Subject: [PATCH 4/8] Use proper types for converters and return converters --- Tools/clinic/clinic.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index ea7dc8df8b974b..6d59cd70f729ba 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -56,7 +56,7 @@ from libclinic.block_parser import Block, BlockParser from libclinic.crenderdata import CRenderData, Include, TemplateDict from libclinic.converter import ( - CConverter, CConverterClassT, + CConverter, CConverterClassT, ConverterType, converters, legacy_converters) @@ -5001,8 +5001,8 @@ def run_clinic(parser: argparse.ArgumentParser, ns: argparse.Namespace) -> None: parser.error( "can't specify --converters and a filename at the same time" ) - converter_list: list[tuple[str, str, Any]] = [] - return_converter_list: list[tuple[str, str, Any]] = [] + converter_list: list[tuple[str, str, ConverterType]] = [] + return_converter_list: list[tuple[str, str, ReturnConverterType]] = [] for name, converter in converters.items(): converter_list.append(( From dfce0e293b8efa75f953f8a4aaf614d128feff9f Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 22 Mar 2024 08:53:21 +0100 Subject: [PATCH 5/8] Update * Use also create_parser_namespace() in eval_ast_expr(). * Only createa the parser namespace once: add _BASE_PARSER_NAMESPACE. * run_clinic(): remove the first tuple item in converter_list and return_converter_list. Rename the new first tuple item from 'short_name' to 'name'. --- Tools/clinic/clinic.py | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 6d59cd70f729ba..f3218a2ed7cfb3 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -1988,7 +1988,7 @@ def parse_file( libclinic.write_file(output, cooked) -def create_parser_namespace() -> dict[str, Any]: +def _create_parser_namespace() -> dict[str, Any]: ns = dict( CConverter=CConverter, CReturnConverter=CReturnConverter, @@ -2002,16 +2002,21 @@ def create_parser_namespace() -> dict[str, Any]: for name, return_converter in return_converters.items(): ns[f'{name}_return_converter'] = return_converter return ns +_BASE_PARSER_NAMESPACE = _create_parser_namespace() + + +def create_parser_namespace() -> dict[str, Any]: + return _BASE_PARSER_NAMESPACE.copy() class PythonParser: def __init__(self, clinic: Clinic) -> None: - self.namespace = create_parser_namespace() + pass def parse(self, block: Block) -> None: - ns = dict(self.namespace) + namespace = create_parser_namespace() with contextlib.redirect_stdout(io.StringIO()) as s: - exec(block.input, ns) + exec(block.input, namespace) block.output = s.getvalue() @@ -3477,8 +3482,9 @@ def eval_ast_expr( node = node.value expr = ast.Expression(node) + namespace = create_parser_namespace() co = compile(expr, filename, 'eval') - fn = FunctionType(co, globals) + fn = FunctionType(co, namespace) return fn() @@ -5001,18 +5007,17 @@ def run_clinic(parser: argparse.ArgumentParser, ns: argparse.Namespace) -> None: parser.error( "can't specify --converters and a filename at the same time" ) - converter_list: list[tuple[str, str, ConverterType]] = [] - return_converter_list: list[tuple[str, str, ReturnConverterType]] = [] + any_converter_type = ConverterType | ReturnConverterType + converter_list: list[tuple[str, any_converter_type]] = [] + return_converter_list: list[tuple[str, any_converter_type]] = [] for name, converter in converters.items(): converter_list.append(( - f'{name}_converter', name, converter, )) for name, return_converter in return_converters.items(): return_converter_list.append(( - f'{name}_return_converter', name, return_converter )) @@ -5030,10 +5035,13 @@ def run_clinic(parser: argparse.ArgumentParser, ns: argparse.Namespace) -> None: ("Return converters", 'return_converter_init', return_converter_list), ): print(title + ":") + + ids.sort(key=lambda item: item[0].lower()) longest = -1 - for name, short_name, converter in ids: - longest = max(longest, len(short_name)) - for name, short_name, cls in sorted(ids, key=lambda x: x[1].lower()): + for name, _ in ids: + longest = max(longest, len(name)) + + for name, cls in ids: callable = getattr(cls, attribute, None) if not callable: continue @@ -5046,7 +5054,7 @@ def run_clinic(parser: argparse.ArgumentParser, ns: argparse.Namespace) -> None: else: s = parameter_name parameters.append(s) - print(' {}({})'.format(short_name, ', '.join(parameters))) + print(' {}({})'.format(name, ', '.join(parameters))) print() print("All converters also accept (c_default=None, py_default=None, annotation=None).") print("All return converters also accept (py_default=None).") From 44d6b5ad0ecf4340273d23d12098df675584a1f1 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 22 Mar 2024 09:07:11 +0100 Subject: [PATCH 6/8] Fix create_parser_namespace() * Add NoneType. * Don't call create_parser_namespace() to startup, but only populate the cache at the first call. * Remove eval_ast_expr() 'globals' argument. --- Tools/clinic/clinic.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index f3218a2ed7cfb3..824d7d02c6e7cd 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -1988,7 +1988,11 @@ def parse_file( libclinic.write_file(output, cooked) -def _create_parser_namespace() -> dict[str, Any]: +def create_parser_namespace() -> dict[str, Any]: + global _BASE_PARSER_NAMESPACE + if _BASE_PARSER_NAMESPACE is not None: + return _BASE_PARSER_NAMESPACE.copy() + ns = dict( CConverter=CConverter, CReturnConverter=CReturnConverter, @@ -1996,17 +2000,16 @@ def _create_parser_namespace() -> dict[str, Any]: robuffer=robuffer, rwbuffer=rwbuffer, unspecified=unspecified, + NoneType=NoneType, ) for name, converter in converters.items(): ns[f'{name}_converter'] = converter for name, return_converter in return_converters.items(): ns[f'{name}_return_converter'] = return_converter - return ns -_BASE_PARSER_NAMESPACE = _create_parser_namespace() - -def create_parser_namespace() -> dict[str, Any]: + _BASE_PARSER_NAMESPACE = ns return _BASE_PARSER_NAMESPACE.copy() +_BASE_PARSER_NAMESPACE = None class PythonParser: @@ -3465,7 +3468,6 @@ class float_return_converter(double_return_converter): def eval_ast_expr( node: ast.expr, - globals: dict[str, Any], *, filename: str = '-' ) -> Any: @@ -4486,12 +4488,11 @@ def parse_converter( case ast.Name(name): return name, False, {} case ast.Call(func=ast.Name(name)): - symbols = globals() kwargs: ConverterArgs = {} for node in annotation.keywords: if not isinstance(node.arg, str): fail("Cannot use a kwarg splat in a function-call annotation") - kwargs[node.arg] = eval_ast_expr(node.value, symbols) + kwargs[node.arg] = eval_ast_expr(node.value) return name, False, kwargs case _: fail( From 9b98a8f5bc6a4caff0c2f6422fe169145ba18bd8 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 22 Mar 2024 09:32:37 +0100 Subject: [PATCH 7/8] AnyConverterType --- Tools/clinic/clinic.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 824d7d02c6e7cd..0b518c9f886a47 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -5008,9 +5008,9 @@ def run_clinic(parser: argparse.ArgumentParser, ns: argparse.Namespace) -> None: parser.error( "can't specify --converters and a filename at the same time" ) - any_converter_type = ConverterType | ReturnConverterType - converter_list: list[tuple[str, any_converter_type]] = [] - return_converter_list: list[tuple[str, any_converter_type]] = [] + AnyConverterType = ConverterType | ReturnConverterType + converter_list: list[tuple[str, AnyConverterType]] = [] + return_converter_list: list[tuple[str, AnyConverterType]] = [] for name, converter in converters.items(): converter_list.append(( From 150e1db7158281f8ca6e51dc56cfdfb5f37f8101 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 26 Mar 2024 08:39:28 +0100 Subject: [PATCH 8/8] Use @functools.cache --- Tools/clinic/clinic.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 0b518c9f886a47..ea480e61ba9a2b 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -1988,11 +1988,8 @@ def parse_file( libclinic.write_file(output, cooked) -def create_parser_namespace() -> dict[str, Any]: - global _BASE_PARSER_NAMESPACE - if _BASE_PARSER_NAMESPACE is not None: - return _BASE_PARSER_NAMESPACE.copy() - +@functools.cache +def _create_parser_base_namespace() -> dict[str, Any]: ns = dict( CConverter=CConverter, CReturnConverter=CReturnConverter, @@ -2006,10 +2003,13 @@ def create_parser_namespace() -> dict[str, Any]: ns[f'{name}_converter'] = converter for name, return_converter in return_converters.items(): ns[f'{name}_return_converter'] = return_converter + return ns + + +def create_parser_namespace() -> dict[str, Any]: + base_namespace = _create_parser_base_namespace() + return base_namespace.copy() - _BASE_PARSER_NAMESPACE = ns - return _BASE_PARSER_NAMESPACE.copy() -_BASE_PARSER_NAMESPACE = None class PythonParser: