Escape string arguments as Python string literals

.gitignore

@@ -6,5 +6,5 @@ __pycache__/
 /docs/_build/
 /src/vunit_python_bridge/bin/
 vunit_out/
-/docs/release_notes.rst
+/docs/release_notes.md
 /docs/api/

docs/conf.py

@@ -26,7 +26,7 @@
 with (ROOT / "pyproject.toml").open("rb") as fptr:
     release = tomllib.load(fptr)["project"]["version"]
 
-extensions = ["myst_parser"]
+extensions = ["myst_parser", "sphinxcontrib.towncrier.ext"]
 # The release notes start at ## since they are included under a version heading
 suppress_warnings = ["myst.header"]
 exclude_patterns = ["_build"]
@@ -46,13 +46,21 @@
 }
 
 
-# The release notes page, generated from release_notes/*.md: unreleased.md, then the releases, newest first
-_notes = ["Release notes", "=============", ""]
+# The release notes page, generated from release_notes: the draft of the news fragments in
+# news.d, rendered by towncrier, and unreleased.md, then the releases, newest first. It is
+# Markdown, since towncrier writes Markdown and the draft is parsed like the page.
+towncrier_draft_autoversion_mode = "draft"
+towncrier_draft_include_empty = False
+towncrier_draft_working_directory = ROOT
+_notes = ["# Release notes", ""]
+_unreleased = sorted((ROOT / "release_notes").glob("unreleased.md"))
+if _unreleased or any((ROOT / "release_notes" / "news.d").glob("*.md")):
+    _notes += ["## Unreleased", "", "```{towncrier-draft-entries}", "```", ""]
 _releases = sorted((ROOT / "release_notes").glob("[0-9]*.md"), key=lambda path: Version(path.stem), reverse=True)
-for _path in sorted((ROOT / "release_notes").glob("unreleased.md")) + _releases:
-    _title = _path.stem.capitalize()
-    _notes += [_title, "-" * len(_title), "", f".. include:: ../release_notes/{_path.name}"]
-    _notes += ["   :parser: myst_parser.sphinx_", ""]
-(ROOT / "docs" / "release_notes.rst").write_text("\n".join(_notes))
+for _path in _unreleased + _releases:
+    if _path.stem != "unreleased":
+        _notes += [f"## {_path.stem}", ""]
+    _notes += [f"```{{include}} ../release_notes/{_path.name}", ":heading-offset: 1", "```", ""]
+(ROOT / "docs" / "release_notes.md").write_text("\n".join(_notes))
 
 vhdl_api.write(ROOT / "docs" / "api")

docs/errors.rst

@@ -69,6 +69,8 @@ Semantics to be aware of
      Python ``float`` (also double precision) is in range. A value that is not
      finite, such as ``float("inf")`` or ``float("nan")``, has no VHDL ``real``
      representation and is reported as a failure.
-   * ``string`` arguments are passed to Python double-quoted verbatim: a quote
-     or backslash inside the string is not escaped, so it must be avoided or
-     already be valid inside a Python double-quoted string.
+   * ``string`` arguments are passed to Python as string literals: quotes,
+     backslashes and control characters (such as line feed and tab) are
+     escaped, so the Python function receives the text exactly as given.
+     Pass plain text, not text already escaped for Python. Bytes >= 128 are
+     kept as they are, so UTF-8 text arrives as the same Python characters.

docs/requirements.txt

@@ -2,3 +2,5 @@
 sphinx==9.1.0
 sphinx-rtd-theme==3.1.0
 myst-parser==5.1.0
+sphinxcontrib-towncrier==0.5.0a0
+towncrier==24.8.0

pyproject.toml

@@ -56,6 +56,39 @@ vunit_python_bridge = [
   "bin/*.dll",
 ]
 
+# The release notes of the next release are news fragments, release_notes/news.d/<PR>.<type>.md,
+# collected into release_notes/unreleased.md with `towncrier build --version <version>` when releasing.
+[tool.towncrier]
+directory = "release_notes/news.d"
+filename = "release_notes/unreleased.md"
+title_format = false
+issue_format = "#{issue}"
+
+[[tool.towncrier.type]]
+directory = "breaking"
+name = "Breaking changes"
+showcontent = true
+
+[[tool.towncrier.type]]
+directory = "feature"
+name = "Features"
+showcontent = true
+
+[[tool.towncrier.type]]
+directory = "bugfix"
+name = "Bug fixes"
+showcontent = true
+
+[[tool.towncrier.type]]
+directory = "doc"
+name = "Documentation"
+showcontent = true
+
+[[tool.towncrier.type]]
+directory = "misc"
+name = "Miscellaneous"
+showcontent = true
+
 [tool.black]
 line-length = 120
 

release_notes/news.d/16.breaking.md

@@ -0,0 +1 @@
+`arg(value : string)` and `kwarg(kw, value : string)` now produce a valid Python string literal: quotes, backslashes and control characters in the text are escaped. Callers that escaped the text themselves (for example passing `\n` to get a newline) now get that text escaped again, and must pass the plain text instead.

src/vunit_python_bridge/hdl/src/python_pkg.vhd

@@ -424,6 +424,31 @@ package body python_pkg is
     return result;
   end;
 
+  -- The Python source text of a message, escaped for a double quoted Python
+  -- string. Quote, backslash and control characters are escaped; bytes >= 128
+  -- are kept so that Python decodes UTF-8 text.
+  function p_quoted(message : string) return string is
+    variable result : line;
+  begin
+    swrite(result, """");
+    for idx in message'range loop
+      case message(idx) is
+        when '"' | '\' => swrite(result, "\" & message(idx));
+        when LF => swrite(result, "\n");
+        when CR => swrite(result, "\r");
+        when others =>
+          if character'pos(message(idx)) < 32 or message(idx) = DEL then
+            swrite(result, "\x" & to_hstring(to_unsigned(character'pos(message(idx)), 8)));
+          else
+            swrite(result, string'(1 => message(idx)));
+          end if;
+      end case;
+    end loop;
+    swrite(result, """");
+
+    return result.all;
+  end;
+
   function arg(value : integer) return arg_t is
   begin
     return (p_positional_arg, to_string(value));
@@ -436,12 +461,12 @@ package body python_pkg is
 
   function arg(value : string) return arg_t is
   begin
-    return (p_positional_arg, '"' & value & '"');
+    return (p_positional_arg, p_quoted(value));
   end;
 
   function kwarg(kw : string; value : string) return arg_t is
   begin
-    return (kw, '"' & value & '"');
+    return (kw, p_quoted(value));
   end;
 
   function arg(value : integer_vector) return arg_t is
@@ -482,26 +507,6 @@ package body python_pkg is
     end if;
   end;
 
-  -- The Python source text of a message, escaped for a double quoted Python
-  -- string. A quote, a backslash and a line break are the characters that
-  -- cannot be written as they are.
-  function p_quoted(message : string) return string is
-    variable result : line;
-  begin
-    swrite(result, """");
-    for idx in message'range loop
-      case message(idx) is
-        when '"' | '\' => swrite(result, "\" & message(idx));
-        when LF => swrite(result, "\n");
-        when CR => swrite(result, "\r");
-        when others => swrite(result, string'(1 => message(idx)));
-      end case;
-    end loop;
-    swrite(result, """");
-
-    return result.all;
-  end;
-
   -- Report a value that could not be converted and give it a Python source
   -- text raising the same message, so that the call the argument is used in
   -- fails the same way rather than being made without the argument.

src/vunit_python_bridge/hdl/tools/python_pkg.vhd.in

@@ -326,6 +326,31 @@ package body python_pkg is
     return result;
   end;
 
+  -- The Python source text of a message, escaped for a double quoted Python
+  -- string. Quote, backslash and control characters are escaped; bytes >= 128
+  -- are kept so that Python decodes UTF-8 text.
+  function p_quoted(message : string) return string is
+    variable result : line;
+  begin
+    swrite(result, """");
+    for idx in message'range loop
+      case message(idx) is
+        when '"' | '\' => swrite(result, "\" & message(idx));
+        when LF => swrite(result, "\n");
+        when CR => swrite(result, "\r");
+        when others =>
+          if character'pos(message(idx)) < 32 or message(idx) = DEL then
+            swrite(result, "\x" & to_hstring(to_unsigned(character'pos(message(idx)), 8)));
+          else
+            swrite(result, string'(1 => message(idx)));
+          end if;
+      end case;
+    end loop;
+    swrite(result, """");
+
+    return result.all;
+  end;
+
   function arg(value : integer) return arg_t is
   begin
     return (p_positional_arg, to_string(value));
@@ -338,12 +363,12 @@ package body python_pkg is
 
   function arg(value : string) return arg_t is
   begin
-    return (p_positional_arg, '"' & value & '"');
+    return (p_positional_arg, p_quoted(value));
   end;
 
   function kwarg(kw : string; value : string) return arg_t is
   begin
-    return (kw, '"' & value & '"');
+    return (kw, p_quoted(value));
   end;
 
   function arg(value : integer_vector) return arg_t is
@@ -384,26 +409,6 @@ package body python_pkg is
     end if;
   end;
 
-  -- The Python source text of a message, escaped for a double quoted Python
-  -- string. A quote, a backslash and a line break are the characters that
-  -- cannot be written as they are.
-  function p_quoted(message : string) return string is
-    variable result : line;
-  begin
-    swrite(result, """");
-    for idx in message'range loop
-      case message(idx) is
-        when '"' | '\' => swrite(result, "\" & message(idx));
-        when LF => swrite(result, "\n");
-        when CR => swrite(result, "\r");
-        when others => swrite(result, string'(1 => message(idx)));
-      end case;
-    end loop;
-    swrite(result, """");
-
-    return result.all;
-  end;
-
   -- Report a value that could not be converted and give it a Python source
   -- text raising the same message, so that the call the argument is used in
   -- fails the same way rather than being made without the argument.

tests/tb_python_pkg.vhd

@@ -269,6 +269,12 @@ begin
 
         check_equal(eval("local_test()"), 1);
 
+      elsif run("Test string arguments are escaped") then
+        check_equal(call("len", arg(string'("a""b\c'd" & LF & HT))), 9);
+        check_equal(call("ord", arg(string'("\"))), 92);
+        check_equal(call("len", arg("é")), 1);
+        check_equal(call("ord", arg(string'(""""))), 34);
+
       elsif run("Test call functions") then
         check_equal(call("len", arg("Hello")), 5);
         check_equal(call("len", arg(integer_vector'(1, 2, 3))), 3);

tests/tb_python_pkg_bridge.vhd

@@ -364,6 +364,16 @@ begin
         check_equal(call_string("identity", arg(string'("hello"))), string'("hello"));
         check_equal(call_string("identity", arg(string'("café å"))), string'("café å"));
 
+      elsif run("Test string arguments are escaped") then
+        exec("def identity(x):" + "    return x");
+        exec("def kw_identity(*, v):" + "    return v");
+        check_equal(call_string("identity", arg(string'("a""b\c'd"))), string'("a""b\c'd"));
+        check_equal(call_string("identity", arg("l1" & LF & "l2" & HT & "end" & CR)), "l1" & LF & "l2" & HT & "end" & CR);
+        check_equal(call_string("identity", arg("é" & NUL & DEL)), "é" & NUL & DEL);
+        check_equal(call_string("kw_identity", kwarg("v", string'("\n""'"))), string'("\n""'"));
+        check_equal(call_string("repr", arg(string'("a\b"))), string'("'a\\b'"));
+        check_equal(to_call_str("f", arg(string'("""\")), kwarg("k", "x" & LF)), "f(""\""\\"", k=""x\n"")");
+
       elsif run("Test std_ulogic round trip of all 9 states") then
         exec("def identity(x):" + "    return x");
         for idx in std_ulogic_characters'range loop

tools/release.py

@@ -16,6 +16,7 @@
     python tools/release.py validate --tag v1.2.3
                                               check the tag against it as well, and that
                                               release_notes/unreleased.md was renamed
+                                              and has the news fragments
 """
 
 import argparse
@@ -26,6 +27,7 @@
 
 PYPROJECT = Path(__file__).parent.parent.resolve() / "pyproject.toml"
 UNRELEASED_NOTES = PYPROJECT.parent / "release_notes" / "unreleased.md"
+NEWS_FRAGMENTS = PYPROJECT.parent / "release_notes" / "news.d"
 
 # The versions a release may have: MAJOR.MINOR.PATCH, optionally a pre-release of it.
 RELEASE_VERSION = re.compile(r"\d+\.\d+\.\d+(a\d+|b\d+|rc\d+)?")
@@ -63,6 +65,12 @@ def validate(tag) -> None:
             "in the commit bumping the version."
         )
 
+    if tag is not None and any(NEWS_FRAGMENTS.glob("*.md")):
+        raise SystemExit(
+            f"{NEWS_FRAGMENTS} still has news fragments. Collect them into {UNRELEASED_NOTES} with "
+            f"towncrier build --version {project_version} before renaming it."
+        )
+
     print(f"Version {project_version}" + ("" if tag is None else f", tagged {tag}"))