Skip to content

Commit

Permalink
hdl.ast.Signal: allow arbitrary Unicode for names, except separator/c…
Browse files Browse the repository at this point in the history
…ontrol
  • Loading branch information
tpwrules committed Mar 25, 2024
1 parent fa2adbe commit dbba30e
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 18 deletions.
25 changes: 24 additions & 1 deletion amaranth/_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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}")
3 changes: 1 addition & 2 deletions amaranth/hdl/_ast.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions amaranth/sim/pysim.py
Original file line number Diff line number Diff line change
Expand Up @@ -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" # :nocov:

field_name = var_name
for item in repr.path:
Expand Down
9 changes: 9 additions & 0 deletions tests/test_hdl_ast.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
13 changes: 0 additions & 13 deletions tests/test_sim.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit dbba30e

Please sign in to comment.