hdl.ast.Signal: allow arbitrary Unicode for names, except separator/control

Author: tpwrules

amaranth/_utils.py

@@ -3,12 +3,13 @@
 import warnings
 import linecache
 import re
+import unicodedata
 from collections import OrderedDict
 from collections.abc import Iterable
 
 
 __all__ = ["flatten", "union", "final", "deprecated", "get_linter_options",
-           "get_linter_option"]
+           "get_linter_option", "validate_name"]
 
 
 def flatten(i):
@@ -101,3 +102,25 @@ def get_linter_option(filename, name, type, default):
         except ValueError:
             return default
     assert False
+
+_invalid_categories = {
+    "Zs", # space separators
+    "Zl", # line separators
+    "Zp", # paragraph separators
+    "Cc", # control codepoints (e.g. \0)
+    "Cs", # UTF-16 surrogate pair codepoints (no thanks WTF-8)
+    "Cn", # unassigned codepoints
+}
+
+def validate_name(name, what, none_ok=False, empty_ok=False):
+    if not isinstance(name, str):
+        if name is None and none_ok: return
+        raise TypeError(f"{what} must be a string, not {name!r}")
+    if name == "" and not empty_ok:
+        raise NameError(f"{what} must be a non-empty string")
+
+    # RTLIL allows bytes >= 33. In the same spirit we allow all characters that
+    # Unicode does not declare as separator or control.
+    for c in name:
+        if unicodedata.category(c) in _invalid_categories:
+            raise NameError(f"{what} {name!r} contains whitespace/control character {c!r}")

amaranth/hdl/_ast.py

@@ -1919,12 +1919,11 @@ def __init__(self, shape=None, *, name=None, init=None, reset=None, reset_less=F
                  attrs=None, decoder=None, src_loc_at=0):
         super().__init__(src_loc_at=src_loc_at)
 
-        if name is not None and not isinstance(name, str):
-            raise TypeError(f"Name must be a string, not {name!r}")
         if name is None:
             self.name = tracer.get_var_name(depth=2 + src_loc_at, default="$signal")
         else:
             self.name = name
+        validate_name(self.name, "Name", empty_ok=True)
 
         orig_shape = shape
         if shape is None:

amaranth/sim/pysim.py

@@ -117,9 +117,9 @@ def __init__(self, design, *, vcd_file, gtkw_file=None, traces=(), fs_per_delta=
 
                 vcd_var = None
                 for (*var_scope, var_name) in names:
+                    # Shouldn't happen, but avoid producing a corrupt file.
                     if re.search(r"[ \t\r\n]", var_name):
-                        raise NameError("Signal '{}.{}' contains a whitespace character"
-                                        .format(".".join(var_scope), var_name))
+                        assert False, f"invalid name {var_name!r} made it to writer"
 
                     field_name = var_name
                     for item in repr.path:

tests/test_hdl_ast.py

@@ -1182,6 +1182,15 @@ def test_name(self):
         self.assertEqual(s2.name, "sig")
         s3 = Signal(name="")
         self.assertEqual(s3.name, "")
+        s4 = Signal(name="$\\1a!\U0001F33C")
+        self.assertEqual(s4.name, "$\\1a!\U0001f33c") # Astral plane emoji "Blossom"
+
+    def test_wrong_name(self):
+        for bad in [" ", "\r", "\n", "\t", "\0", "\u009d"]: # Control character OSC
+            name = f"sig{bad}"
+            with self.assertRaises(NameError,
+                    msg="Name {name!r} contains whitespace/control character {bad!r}"):
+                Signal(name=name)
 
     def test_init(self):
         s1 = Signal(4, init=0b111, reset_less=True)

tests/test_sim.py

@@ -1230,19 +1230,6 @@ def process():
         sim.add_testbench(process)
         sim.run()
 
-    def test_bug_595(self):
-        dut = Module()
-        dummy = Signal()
-        with dut.FSM(name="name with space"):
-            with dut.State(0):
-                dut.d.comb += dummy.eq(1)
-        sim = Simulator(dut)
-        with self.assertRaisesRegex(NameError,
-                r"^Signal 'bench\.top\.name with space_state' contains a whitespace character$"):
-            with open(os.path.devnull, "w") as f:
-                with sim.write_vcd(f):
-                    sim.run()
-
     def test_bug_588(self):
         dut = Module()
         a = Signal(32)