From 07e2364d577b006ae3b93cf136cc1938177942ca Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Fri, 22 Dec 2023 21:28:15 +0100 Subject: [PATCH] gh-113317: Argument Clinic: tear out internal text accumulator APIs (#113402) Replace the internal accumulator APIs by using lists of strings and join(). Yak-shaving for separating out formatting code into a separate file. Co-authored-by: Alex Waygood --- Lib/test/test_clinic.py | 30 ----- Tools/clinic/clinic.py | 247 +++++++++++++++------------------------- 2 files changed, 89 insertions(+), 188 deletions(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index f3f73a174aeb132..199c9fec80205a7 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -3761,20 +3761,6 @@ def test_normalize_snippet(self): actual = clinic.normalize_snippet(snippet, indent=indent) self.assertEqual(actual, expected) - def test_accumulator(self): - acc = clinic.text_accumulator() - self.assertEqual(acc.output(), "") - acc.append("a") - self.assertEqual(acc.output(), "a") - self.assertEqual(acc.output(), "") - acc.append("b") - self.assertEqual(acc.output(), "b") - self.assertEqual(acc.output(), "") - acc.append("c") - acc.append("d") - self.assertEqual(acc.output(), "cd") - self.assertEqual(acc.output(), "") - def test_quoted_for_c_string(self): dataset = ( # input, expected @@ -3790,22 +3776,6 @@ def test_quoted_for_c_string(self): out = clinic.quoted_for_c_string(line) self.assertEqual(out, expected) - def test_rstrip_lines(self): - lines = ( - "a \n" - "b\n" - " c\n" - " d \n" - ) - expected = ( - "a\n" - "b\n" - " c\n" - " d\n" - ) - out = clinic.rstrip_lines(lines) - self.assertEqual(out, expected) - def test_format_escape(self): line = "{}, {a}" expected = "{{}}, {{a}}" diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index e8ba805bd53b458..4b00902052d5a52 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -105,42 +105,8 @@ def __repr__(self) -> str: sig_end_marker = '--' -Appender = Callable[[str], None] -Outputter = Callable[[], str] TemplateDict = dict[str, str] -class _TextAccumulator(NamedTuple): - text: list[str] - append: Appender - output: Outputter - -def _text_accumulator() -> _TextAccumulator: - text: list[str] = [] - def output() -> str: - s = ''.join(text) - text.clear() - return s - return _TextAccumulator(text, text.append, output) - - -class TextAccumulator(NamedTuple): - append: Appender - output: Outputter - -def text_accumulator() -> TextAccumulator: - """ - Creates a simple text accumulator / joiner. - - Returns a pair of callables: - append, output - "append" appends a string to the accumulator. - "output" returns the contents of the accumulator - joined together (''.join(accumulator)) and - empties the accumulator. - """ - text, append, output = _text_accumulator() - return TextAccumulator(append, output) - @dc.dataclass class ClinicError(Exception): @@ -264,14 +230,6 @@ def ensure_legal_c_identifier(s: str) -> str: return s + "_value" return s -def rstrip_lines(s: str) -> str: - text, add, output = _text_accumulator() - for line in s.split('\n'): - add(line.rstrip()) - add('\n') - text.pop() - return output() - def format_escape(s: str) -> str: # double up curly-braces, this string will be used # as part of a format_map() template later @@ -294,18 +252,16 @@ def linear_format(s: str, **kwargs: str) -> str: * A newline will be added to the end. """ - add, output = text_accumulator() + lines = [] for line in s.split('\n'): indent, curly, trailing = line.partition('{') if not curly: - add(line) - add('\n') + lines.extend([line, "\n"]) continue name, curly, trailing = trailing.partition('}') if not curly or name not in kwargs: - add(line) - add('\n') + lines.extend([line, "\n"]) continue if trailing: @@ -319,51 +275,32 @@ def linear_format(s: str, **kwargs: str) -> str: if not value: continue - value = textwrap.indent(rstrip_lines(value), indent) - add(value) - add('\n') + stripped = [line.rstrip() for line in value.split("\n")] + value = textwrap.indent("\n".join(stripped), indent) + lines.extend([value, "\n"]) - return output()[:-1] + return "".join(lines[:-1]) -def indent_all_lines(s: str, prefix: str) -> str: +def _add_prefix_and_suffix(s: str, prefix: str = "", suffix: str = "") -> str: """ - Returns 's', with 'prefix' prepended to all lines. + Return 's', with 'prefix' prepended and 'suffix' appended to all lines. - If the last line is empty, prefix is not prepended - to it. (If s is blank, returns s unchanged.) + If the last line is empty, it remains unchanged. + If s is blank, returns s unchanged. (textwrap.indent only adds to non-blank lines.) """ - split = s.split('\n') - last = split.pop() - final = [] - for line in split: - final.append(prefix) - final.append(line) - final.append('\n') + *split, last = s.split("\n") + lines = [prefix + line + suffix + "\n" for line in split] if last: - final.append(prefix) - final.append(last) - return ''.join(final) + lines.append(prefix + last + suffix) + return "".join(lines) -def suffix_all_lines(s: str, suffix: str) -> str: - """ - Returns 's', with 'suffix' appended to all lines. +def indent_all_lines(s: str, prefix: str) -> str: + return _add_prefix_and_suffix(s, prefix=prefix) - If the last line is empty, suffix is not appended - to it. (If s is blank, returns s unchanged.) - """ - split = s.split('\n') - last = split.pop() - final = [] - for line in split: - final.append(line) - final.append(suffix) - final.append('\n') - if last: - final.append(last) - final.append(suffix) - return ''.join(final) +def suffix_all_lines(s: str, suffix: str) -> str: + return _add_prefix_and_suffix(s, suffix=suffix) def pprint_words(items: list[str]) -> str: @@ -1068,21 +1005,21 @@ def docstring_for_c_string( self, f: Function ) -> str: - text, add, output = _text_accumulator() + lines = [] # turn docstring into a properly quoted C string for line in f.docstring.split('\n'): - add('"') - add(quoted_for_c_string(line)) - add('\\n"\n') + lines.append('"') + lines.append(quoted_for_c_string(line)) + lines.append('\\n"\n') - if text[-2] == sig_end_marker: + if lines[-2] == sig_end_marker: # If we only have a signature, add the blank line that the # __text_signature__ getter expects to be there. - add('"\\n"') + lines.append('"\\n"') else: - text.pop() - add('"') - return ''.join(text) + lines.pop() + lines.append('"') + return ''.join(lines) def output_templates( self, @@ -1183,8 +1120,8 @@ def parser_body( declarations: str = '' ) -> str: nonlocal parser_body_fields - add, output = text_accumulator() - add(prototype) + lines = [] + lines.append(prototype) parser_body_fields = fields preamble = normalize_snippet(""" @@ -1208,9 +1145,8 @@ def parser_body( }} """) for field in preamble, *fields, finale: - add('\n') - add(field) - return linear_format(output(), parser_declarations=declarations) + lines.append(field) + return linear_format("\n".join(lines), parser_declarations=declarations) fastcall = not new_or_init limited_capi = clinic.limited_capi @@ -1785,7 +1721,7 @@ def render_option_group_parsing( # Clinic prefers groups on the left. So in the above example, # five arguments would map to B+C, not C+D. - add, output = text_accumulator() + out = [] parameters = list(f.parameters.values()) if isinstance(parameters[0].converter, self_converter): del parameters[0] @@ -1817,14 +1753,14 @@ def render_option_group_parsing( nargs = 'PyTuple_Size(args)' else: nargs = 'PyTuple_GET_SIZE(args)' - add(f"switch ({nargs}) {{\n") + out.append(f"switch ({nargs}) {{\n") for subset in permute_optional_groups(left, required, right): count = len(subset) count_min = min(count_min, count) count_max = max(count_max, count) if count == 0: - add(""" case 0: + out.append(""" case 0: break; """) continue @@ -1856,14 +1792,15 @@ def render_option_group_parsing( """ s = linear_format(s, group_booleans=lines) s = s.format_map(d) - add(s) + out.append(s) - add(" default:\n") + out.append(" default:\n") s = ' PyErr_SetString(PyExc_TypeError, "{} requires {} to {} arguments");\n' - add(s.format(f.full_name, count_min, count_max)) - add(' goto exit;\n') - add("}") - template_dict['option_group_parsing'] = format_escape(output()) + out.append(s.format(f.full_name, count_min, count_max)) + out.append(' goto exit;\n') + out.append("}") + + template_dict['option_group_parsing'] = format_escape("".join(out)) def render_function( self, @@ -1873,7 +1810,6 @@ def render_function( if f is None or clinic is None: return "" - add, output = text_accumulator() data = CRenderData() assert f.parameters, "We should always have a 'self' at this point!" @@ -2202,7 +2138,7 @@ def _line(self, lookahead: bool = False) -> str: return line def parse_verbatim_block(self) -> Block: - add, output = text_accumulator() + lines = [] self.block_start_line_number = self.line_number while self.input: @@ -2211,12 +2147,12 @@ def parse_verbatim_block(self) -> Block: if dsl_name: self.dsl_name = dsl_name break - add(line) + lines.append(line) - return Block(output()) + return Block("".join(lines)) def parse_clinic_block(self, dsl_name: str) -> Block: - input_add, input_output = text_accumulator() + in_lines = [] self.block_start_line_number = self.line_number + 1 stop_line = self.language.stop_line.format(dsl_name=dsl_name) body_prefix = self.language.body_prefix.format(dsl_name=dsl_name) @@ -2244,7 +2180,7 @@ def is_stop_line(line: str) -> bool: line = line.lstrip() assert line.startswith(body_prefix) line = line.removeprefix(body_prefix) - input_add(line) + in_lines.append(line) # consume output and checksum line, if present. if self.last_dsl_name == dsl_name: @@ -2258,7 +2194,7 @@ def is_stop_line(line: str) -> bool: assert checksum_re is not None # scan forward for checksum line - output_add, output_output = text_accumulator() + out_lines = [] arguments = None while self.input: line = self._line(lookahead=True) @@ -2266,12 +2202,12 @@ def is_stop_line(line: str) -> bool: arguments = match.group(1) if match else None if arguments: break - output_add(line) + out_lines.append(line) if self.is_start_line(line): break output: str | None - output = output_output() + output = "".join(out_lines) if arguments: d = {} for field in shlex.split(arguments): @@ -2299,7 +2235,7 @@ def is_stop_line(line: str) -> bool: self.input.extend(reversed(output_lines)) output = None - return Block(input_output(), dsl_name, output=output) + return Block("".join(in_lines), dsl_name, output=output) @dc.dataclass(slots=True, frozen=True) @@ -2406,6 +2342,9 @@ def write(self, text: str) -> None: self.f.write(text) +TextAccumulator = list[str] + + class BufferSeries: """ Behaves like a "defaultlist". @@ -2419,26 +2358,26 @@ class BufferSeries: def __init__(self) -> None: self._start = 0 - self._array: list[_TextAccumulator] = [] - self._constructor = _text_accumulator + self._array: list[TextAccumulator] = [] - def __getitem__(self, i: int) -> _TextAccumulator: + def __getitem__(self, i: int) -> TextAccumulator: i -= self._start if i < 0: self._start += i - prefix = [self._constructor() for x in range(-i)] + prefix: list[TextAccumulator] = [[] for x in range(-i)] self._array = prefix + self._array i = 0 while i >= len(self._array): - self._array.append(self._constructor()) + self._array.append([]) return self._array[i] def clear(self) -> None: for ta in self._array: - ta.text.clear() + ta.clear() def dump(self) -> str: - texts = [ta.output() for ta in self._array] + texts = ["".join(ta) for ta in self._array] + self.clear() return "".join(texts) @@ -2624,7 +2563,7 @@ def __init__( 'impl_definition': d('block'), } - DestBufferType = dict[str, _TextAccumulator] + DestBufferType = dict[str, TextAccumulator] DestBufferList = list[DestBufferType] self.destination_buffers_stack: DestBufferList = [] @@ -2696,7 +2635,7 @@ def get_destination_buffer( self, name: str, item: int = 0 - ) -> _TextAccumulator: + ) -> TextAccumulator: d = self.get_destination(name) return d.buffers[item] @@ -3150,11 +3089,9 @@ def get_displayname(self, i: int) -> str: return f'argument {i}' def render_docstring(self) -> str: - add, out = text_accumulator() - add(f" {self.name}\n") - for line in self.docstring.split("\n"): - add(f" {line}\n") - return out().rstrip() + lines = [f" {self.name}"] + lines.extend(f" {line}" for line in self.docstring.split("\n")) + return "\n".join(lines).rstrip() CConverterClassT = TypeVar("CConverterClassT", bound=type["CConverter"]) @@ -6220,15 +6157,15 @@ def state_function_docstring(self, line: str) -> None: def format_docstring_signature( self, f: Function, parameters: list[Parameter] ) -> str: - text, add, output = _text_accumulator() - add(f.displayname) + lines = [] + lines.append(f.displayname) if self.forced_text_signature: - add(self.forced_text_signature) + lines.append(self.forced_text_signature) elif f.kind in {GETTER, SETTER}: # @getter and @setter do not need signatures like a method or a function. return '' else: - add('(') + lines.append('(') # populate "right_bracket_count" field for every parameter assert parameters, "We should always have a self parameter. " + repr(f) @@ -6282,7 +6219,7 @@ def fix_right_bracket_count(desired: int) -> str: first_parameter = True last_p = parameters[-1] - line_length = len(''.join(text)) + line_length = len(''.join(lines)) indent = " " * line_length def add_parameter(text: str) -> None: nonlocal line_length @@ -6293,12 +6230,11 @@ def add_parameter(text: str) -> None: else: s = ' ' + text if line_length + len(s) >= 72: - add('\n') - add(indent) + lines.extend(["\n", indent]) line_length = len(indent) s = text line_length += len(s) - add(s) + lines.append(s) for p in parameters: if not p.converter.show_in_signature: @@ -6321,8 +6257,7 @@ def add_parameter(text: str) -> None: added_star = True add_parameter('*,') - p_add, p_output = text_accumulator() - p_add(fix_right_bracket_count(p.right_bracket_count)) + p_lines = [fix_right_bracket_count(p.right_bracket_count)] if isinstance(p.converter, self_converter): # annotate first parameter as being a "self". @@ -6340,30 +6275,31 @@ def add_parameter(text: str) -> None: # have a docstring.) if this is an __init__ # (or __new__), then this signature is for # calling the class to construct a new instance. - p_add('$') + p_lines.append('$') if p.is_vararg(): - p_add("*") + p_lines.append("*") name = p.converter.signature_name or p.name - p_add(name) + p_lines.append(name) if not p.is_vararg() and p.converter.is_optional(): - p_add('=') + p_lines.append('=') value = p.converter.py_default if not value: value = repr(p.converter.default) - p_add(value) + p_lines.append(value) if (p != last_p) or need_a_trailing_slash: - p_add(',') + p_lines.append(',') - add_parameter(p_output()) + p_output = "".join(p_lines) + add_parameter(p_output) - add(fix_right_bracket_count(0)) + lines.append(fix_right_bracket_count(0)) if need_a_trailing_slash: add_parameter('/') - add(')') + lines.append(')') # PEP 8 says: # @@ -6375,13 +6311,13 @@ def add_parameter(text: str) -> None: # therefore this is commented out: # # if f.return_converter.py_default: - # add(' -> ') - # add(f.return_converter.py_default) + # lines.append(' -> ') + # lines.append(f.return_converter.py_default) if not f.docstring_only: - add("\n" + sig_end_marker + "\n") + lines.append("\n" + sig_end_marker + "\n") - signature_line = output() + signature_line = "".join(lines) # now fix up the places where the brackets look wrong return signature_line.replace(', ]', ',] ') @@ -6389,12 +6325,7 @@ def add_parameter(text: str) -> None: @staticmethod def format_docstring_parameters(params: list[Parameter]) -> str: """Create substitution text for {parameters}""" - add, output = text_accumulator() - for p in params: - if p.docstring: - add(p.render_docstring()) - add('\n') - return output() + return "".join(p.render_docstring() + "\n" for p in params if p.docstring) def format_docstring(self) -> str: assert self.function is not None