From acfdab7933dd5c53775c62624709727a9f73a58a Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Thu, 4 May 2023 23:22:52 +0200 Subject: [PATCH 1/4] gh-64660: Don't hard-code '_return_value' in AC return converters --- Tools/clinic/clinic.py | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 2746b24c333e66..22d2879af8351c 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -3876,6 +3876,7 @@ class CReturnConverter(metaclass=CReturnConverterAutoRegister): # The Python default value for this parameter, as a Python value. # Or the magic value "unspecified" if there is no default. default = None + retvar = "_return_value" def __init__(self, *, py_default=None, **kwargs): self.py_default = py_default @@ -3888,15 +3889,15 @@ def __init__(self, *, py_default=None, **kwargs): def return_converter_init(self): pass - def declare(self, data, name="_return_value"): + def declare(self, data): line = [] add = line.append add(self.type) if not self.type.endswith('*'): add(' ') - add(name + ';') + add(self.retvar + ';') data.declarations.append(''.join(line)) - data.return_value = name + data.return_value = self.retvar def err_occurred_if(self, expr, data): data.return_conversion.append('if (({}) && PyErr_Occurred()) {{\n goto exit;\n}}\n'.format(expr)) @@ -3918,8 +3919,10 @@ class bool_return_converter(CReturnConverter): def render(self, function, data): self.declare(data) - self.err_occurred_if("_return_value == -1", data) - data.return_conversion.append('return_value = PyBool_FromLong((long)_return_value);\n') + self.err_occurred_if(f"{self.retvar} == -1", data) + data.return_conversion.append( + f'return_value = PyBool_FromLong((long){self.retvar});\n' + ) class long_return_converter(CReturnConverter): type = 'long' @@ -3929,9 +3932,10 @@ class long_return_converter(CReturnConverter): def render(self, function, data): self.declare(data) - self.err_occurred_if("_return_value == {}-1".format(self.unsigned_cast), data) + self.err_occurred_if(f"{self.retvar} == {self.unsigned_cast}-1", data) data.return_conversion.append( - ''.join(('return_value = ', self.conversion_fn, '(', self.cast, '_return_value);\n'))) + f'return_value = {self.conversion_fn}({self.cast}{self.retvar});\n' + ) class int_return_converter(long_return_converter): type = 'int' @@ -3973,9 +3977,10 @@ class double_return_converter(CReturnConverter): def render(self, function, data): self.declare(data) - self.err_occurred_if("_return_value == -1.0", data) + self.err_occurred_if(f"{self.retvar} == -1.0", data) data.return_conversion.append( - 'return_value = PyFloat_FromDouble(' + self.cast + '_return_value);\n') + f'return_value = PyFloat_FromDouble({self.cast}{self.retvar});\n' + ) class float_return_converter(double_return_converter): type = 'float' From de9dc35aa3557ee602727ae45c862723c7ec1ed4 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Fri, 5 May 2023 15:02:49 +0200 Subject: [PATCH 2/4] gh-64660: Don't hardcode Argument Clinic return converter result variable name --- Tools/clinic/clinic.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 22d2879af8351c..a0d320f2b09aca 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -355,6 +355,7 @@ def __init__(self): # value from the parse function. This is also where # you should check the _return_value for errors, and # "goto exit" if there are any. + self._return_value = "_return_value" self.return_conversion = [] # The C statements required to do some operations @@ -3876,7 +3877,6 @@ class CReturnConverter(metaclass=CReturnConverterAutoRegister): # The Python default value for this parameter, as a Python value. # Or the magic value "unspecified" if there is no default. default = None - retvar = "_return_value" def __init__(self, *, py_default=None, **kwargs): self.py_default = py_default @@ -3895,9 +3895,11 @@ def declare(self, data): add(self.type) if not self.type.endswith('*'): add(' ') - add(self.retvar + ';') + add(data._return_value + ';') data.declarations.append(''.join(line)) - data.return_value = self.retvar + # XXX Removing this hack breaks current generated code. + # We should aim to get rid of this workaround. + data.return_value = data._return_value def err_occurred_if(self, expr, data): data.return_conversion.append('if (({}) && PyErr_Occurred()) {{\n goto exit;\n}}\n'.format(expr)) @@ -3919,9 +3921,9 @@ class bool_return_converter(CReturnConverter): def render(self, function, data): self.declare(data) - self.err_occurred_if(f"{self.retvar} == -1", data) + self.err_occurred_if(f"{data._return_value} == -1", data) data.return_conversion.append( - f'return_value = PyBool_FromLong((long){self.retvar});\n' + f'return_value = PyBool_FromLong((long){data._return_value});\n' ) class long_return_converter(CReturnConverter): @@ -3932,9 +3934,9 @@ class long_return_converter(CReturnConverter): def render(self, function, data): self.declare(data) - self.err_occurred_if(f"{self.retvar} == {self.unsigned_cast}-1", data) + self.err_occurred_if(f"{data._return_value} == {self.unsigned_cast}-1", data) data.return_conversion.append( - f'return_value = {self.conversion_fn}({self.cast}{self.retvar});\n' + f'return_value = {self.conversion_fn}({self.cast}{data._return_value});\n' ) class int_return_converter(long_return_converter): @@ -3977,9 +3979,9 @@ class double_return_converter(CReturnConverter): def render(self, function, data): self.declare(data) - self.err_occurred_if(f"{self.retvar} == -1.0", data) + self.err_occurred_if(f"{data._return_value} == -1.0", data) data.return_conversion.append( - f'return_value = PyFloat_FromDouble({self.cast}{self.retvar});\n' + f'return_value = PyFloat_FromDouble({self.cast}{data._return_value});\n' ) class float_return_converter(double_return_converter): From 548732e801cd29a847b025c6c8f819a571b187ca Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Sun, 7 May 2023 23:28:24 +0200 Subject: [PATCH 3/4] Naming --- Tools/clinic/clinic.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 60ce5fc569dadd..16dfad4233e64b 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -355,8 +355,8 @@ def __init__(self): # value from the parse function. This is also where # you should check the _return_value for errors, and # "goto exit" if there are any. - self._return_value = "_return_value" self.return_conversion = [] + self.converter_retval = "_return_value" # The C statements required to do some operations # after the end of parsing but before cleaning up. @@ -3900,11 +3900,11 @@ def declare(self, data): add(self.type) if not self.type.endswith('*'): add(' ') - add(data._return_value + ';') + add(data.converter_retval + ';') data.declarations.append(''.join(line)) # XXX Removing this hack breaks current generated code. # We should aim to get rid of this workaround. - data.return_value = data._return_value + data.return_value = data.converter_retval def err_occurred_if(self, expr, data): data.return_conversion.append('if (({}) && PyErr_Occurred()) {{\n goto exit;\n}}\n'.format(expr)) @@ -3926,9 +3926,9 @@ class bool_return_converter(CReturnConverter): def render(self, function, data): self.declare(data) - self.err_occurred_if(f"{data._return_value} == -1", data) + self.err_occurred_if(f"{data.converter_retval} == -1", data) data.return_conversion.append( - f'return_value = PyBool_FromLong((long){data._return_value});\n' + f'return_value = PyBool_FromLong((long){data.converter_retval});\n' ) class long_return_converter(CReturnConverter): @@ -3939,9 +3939,9 @@ class long_return_converter(CReturnConverter): def render(self, function, data): self.declare(data) - self.err_occurred_if(f"{data._return_value} == {self.unsigned_cast}-1", data) + self.err_occurred_if(f"{data.converter_retval} == {self.unsigned_cast}-1", data) data.return_conversion.append( - f'return_value = {self.conversion_fn}({self.cast}{data._return_value});\n' + f'return_value = {self.conversion_fn}({self.cast}{data.converter_retval});\n' ) class int_return_converter(long_return_converter): @@ -3984,9 +3984,9 @@ class double_return_converter(CReturnConverter): def render(self, function, data): self.declare(data) - self.err_occurred_if(f"{data._return_value} == -1.0", data) + self.err_occurred_if(f"{data.converter_retval} == -1.0", data) data.return_conversion.append( - f'return_value = PyFloat_FromDouble({self.cast}{data._return_value});\n' + f'return_value = PyFloat_FromDouble({self.cast}{data.converter_retval});\n' ) class float_return_converter(double_return_converter): From fc2939f04e25b72a534c2c58c21100b148131383 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Sun, 7 May 2023 23:29:50 +0200 Subject: [PATCH 4/4] Remove XXX comment --- Tools/clinic/clinic.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 16dfad4233e64b..a6f330d1502dad 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -3902,8 +3902,6 @@ def declare(self, data): add(' ') add(data.converter_retval + ';') data.declarations.append(''.join(line)) - # XXX Removing this hack breaks current generated code. - # We should aim to get rid of this workaround. data.return_value = data.converter_retval def err_occurred_if(self, expr, data):