Improve Verilog identifier naming, fix two bugs, add tests, refactor duplicated

code:

Update `_NameSanitizer` to try cleaning up an invalid name before giving up and
auto-generating a new name. The cleaning process just replaces non-word
characters `[A-Za-z0-9_]` with underscores, and also tries adding a prefix
underscore. This makes the generated Verilog output easier to read, especially
when using `wire_struct` or `wire_matrix`, which both add non-word characters
to wire names.

Remove `_NameSanitizer`'s `map_valid_vals` and `allow_duplicates` options,
which don't seem to be used.

Bugs fixed:

- `output_verilog_testbench` should not re-initialize RomBlocks. This appears
  to be an existing bug.
- Consts inputs to bit-slices must be declared.

Add tests for these bugs.

Also move the Verilog export code to a new `class _VerilogOutputter`. This lets
`output_to_verilog` and `output_verilog_testbench` share more code.

Also remove `OutputToVerilog`. It is deprecated, and the forwarding function
was broken, so it's pretty clear nobody uses this.

Author: fdxmw

pyrtl/__init__.py

@@ -103,7 +103,6 @@
 from .importexport import (
     input_from_verilog,
     output_to_verilog,
-    OutputToVerilog,
     output_verilog_testbench,
     input_from_blif,
     output_to_firrtl,
@@ -242,7 +241,6 @@
     # importexport
     "input_from_verilog",
     "output_to_verilog",
-    "OutputToVerilog",
     "output_verilog_testbench",
     "input_from_blif",
     "output_to_firrtl",

pyrtl/core.py

@@ -1206,40 +1206,43 @@ def __init__(
         self,
         identifier_regex_str,
         internal_prefix="_sani_temp",
-        map_valid_vals=True,
         extra_checks=lambda _string: True,
-        allow_duplicates=False,
     ):
         if identifier_regex_str[-1] != "$":
             identifier_regex_str += "$"
         self.identifier = re.compile(identifier_regex_str)
         self.val_map = {}
-        self.map_valid = map_valid_vals
         self.extra_checks = extra_checks
-        self.allow_dups = allow_duplicates
         super().__init__(internal_prefix)
 
     def __getitem__(self, item):
         """Get a value from the sanitizer"""
-        if not self.map_valid and self.is_valid_str(item):
-            return item
         return self.val_map[item]
 
     def is_valid_str(self, string):
         return self.identifier.match(string) and self.extra_checks(string)
 
     def make_valid_string(self, string=""):
         """Inputting a value for the first time."""
-        if not self.is_valid_str(string):
-            if string in self.val_map and not self.allow_dups:
-                msg = f"Value {string} has already been given to the sanitizer"
-                raise IndexError(msg)
-            internal_name = super().make_valid_string()
-            self.val_map[string] = internal_name
-            return internal_name
-        if self.map_valid:
+        if self.is_valid_str(string):
             self.val_map[string] = string
-        return string
+            return string
+
+        if string in self.val_map:
+            msg = f"Value {string} has already been given to the sanitizer"
+            raise IndexError(msg)
+        # Try replacing non-word characters with ``_``.
+        internal_name = re.sub(r"\W", "_", string)
+
+        if not self.is_valid_str(internal_name) or internal_name in self.val_map:
+            # If that didn't work, try prepending ``_``.
+            internal_name = f"_{internal_name}"
+            while not self.is_valid_str(internal_name) or internal_name in self.val_map:
+                # If that didn't work, generate names starting with ``internal_prefix``
+                # until we find something acceptable.
+                internal_name = super().make_valid_string()
+        self.val_map[string] = internal_name
+        return internal_name
 
 
 class _PythonSanitizer(_NameSanitizer):