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 <Alex.Waygood@Gmail.com>

1 files changed, 89 insertions, 158 deletions

diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py
index e8ba805..4b00902 100755
--- a/Tools/clinic/clinic.py
+++ b/Tools/clinic/clinic.py
@@ -105,42 +105,8 @@ NULL = Null()
 
 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 @@ class CLanguage(Language):
             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 @@ class CLanguage(Language):
                 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 @@ class CLanguage(Language):
                 }}
             """)
             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 @@ class CLanguage(Language):
         # 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 @@ class CLanguage(Language):
             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 @@ class CLanguage(Language):
 """
             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 @@ class CLanguage(Language):
         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 @@ class BlockParser:
         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 @@ class BlockParser:
             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 @@ class BlockParser:
                 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 @@ class BlockParser:
         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 @@ class BlockParser:
             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 @@ class BlockParser:
             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 @@ class BlockPrinter:
         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 @@ impl_definition block
             'impl_definition': d('block'),
         }
 
-        DestBufferType = dict[str, _TextAccumulator]
+        DestBufferType = dict[str, TextAccumulator]
         DestBufferList = list[DestBufferType]
 
         self.destination_buffers_stack: DestBufferList = []
@@ -2696,7 +2635,7 @@ impl_definition block
             self,
             name: str,
             item: int = 0
-    ) -> _TextAccumulator:
+    ) -> TextAccumulator:
         d = self.get_destination(name)
         return d.buffers[item]
 
@@ -3150,11 +3089,9 @@ class Parameter:
             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 @@ class DSLParser:
     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 @@ class DSLParser:
 
             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 @@ class DSLParser:
                 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 @@ class DSLParser:
                     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 @@ class DSLParser:
                     # 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 @@ class DSLParser:
         # 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 @@ class DSLParser:
     @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