From 51d79ea284eaf46a44a12b56a161525804fbc141 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Fri, 19 Jan 2024 21:27:26 +0100 Subject: [PATCH 1/2] gh-113317: Don't use global clinic instance in bad_argument() Make it possible for a converter to have multiple includes, make the 'includes' an instance attribute. This implies converter includes are added during template generation, so we have to add them to the clinic instance at the end of the template generation instead of in the beginning. --- Tools/clinic/clinic.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index c247bd075321cd..de66633a03373c 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -818,12 +818,6 @@ def output_templates( del parameters[0] converters = [p.converter for p in parameters] - # Copy includes from parameters to Clinic - for converter in converters: - include = converter.include - if include: - clinic.add_include(include.filename, include.reason, - condition=include.condition) if f.critical_section: clinic.add_include('pycore_critical_section.h', 'Py_BEGIN_CRITICAL_SECTION()') has_option_groups = parameters and (parameters[0].group or parameters[-1].group) @@ -1367,6 +1361,14 @@ def parser_body( declarations=declarations) + # Copy includes from parameters to Clinic after parse_arg() has been + # called above. + for converter in converters: + for include in converter.includes: + assert include is not None + clinic.add_include(include.filename, include.reason, + condition=include.condition) + if new_or_init: methoddef_define = '' @@ -2988,7 +2990,6 @@ class CConverter(metaclass=CConverterAutoRegister): # Only set by self_converter. signature_name: str | None = None - include: Include | None = None broken_limited_capi: bool = False # keep in sync with self_converter.__init__! @@ -3008,6 +3009,7 @@ def __init__(self, self.name = ensure_legal_c_identifier(name) self.py_name = py_name self.unused = unused + self.includes: list[Include] = [] if default is not unspecified: if (self.default_type @@ -3263,8 +3265,7 @@ def bad_argument(self, displayname: str, expected: str, *, limited_capi: bool, e else: if expected_literal: expected = f'"{expected}"' - if clinic is not None: - clinic.add_include('pycore_modsupport.h', '_PyArg_BadArgument()') + self.add_include('pycore_modsupport.h', '_PyArg_BadArgument()') return f'_PyArg_BadArgument("{{{{name}}}}", "{displayname}", {expected}, {{argname}});' def format_code(self, fmt: str, *, @@ -3336,9 +3337,8 @@ def parser_name(self) -> str: def add_include(self, name: str, reason: str, *, condition: str | None = None) -> None: - if self.include is not None: - raise ValueError("a converter only supports a single include") - self.include = Include(name, reason, condition) + include = Include(name, reason, condition) + self.includes.append(include) type_checks = { '&PyLong_Type': ('PyLong_Check', 'int'), From 11186378da9db522ce7fabe9da40cecc56b1fa8d Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 22 Jan 2024 15:20:17 +0100 Subject: [PATCH 2/2] Address review --- Tools/clinic/clinic.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index de66633a03373c..770878a3f8d2c7 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -1365,7 +1365,6 @@ def parser_body( # called above. for converter in converters: for include in converter.includes: - assert include is not None clinic.add_include(include.filename, include.reason, condition=include.condition)