Repository: amaranth-lang/amaranth (python)

Issue:
`Signature.create`'s `path` argument should probably default to `tracer.get_var_name()`
This change doesn't require an RFC.

It is tagged for the 0.4 milestone because this would change the signature from:

```python
def create(self, *, path=()):
```

to

```python
def create(self, *, path=None, src_loc_at=0):
```

and affects downstream and example code (which was how this bug was discovered).

The `SignatureMembers.create` method (which actually creates the signals) should have the same modifications done to it, and forward `src_loc_at` to the created signals. Right now, the source locations for all `lib.wiring`-created signals will point within `lib.wiring`, which makes them useless. Ideally the source location of the `In(...)`/`Out(...)` expression would be also preserved, but we aren't set up to do that in a nice way at the moment.

While we're at it: `FlippedSignature.create` should probably forward all unrecognized args/kwargs, too.

Pull request:
lib.wiring: use tracer to obtain default Signature path and src_loc.
Fixes #987.

Patch:
diff --git a/amaranth/lib/wiring.py b/amaranth/lib/wiring.py
index 0d94201..18ceb58 100644
--- a/amaranth/lib/wiring.py
+++ b/amaranth/lib/wiring.py
@@ -5,6 +5,7 @@ import inspect
 import re
 import warnings
 
+from .. import tracer
 from ..hdl.ast import Shape, ShapeCastable, Const, Signal, Value, ValueCastable
 from ..hdl.ir import Elaboratable
 from .._utils import final
@@ -219,23 +220,27 @@ class SignatureMembers(Mapping):
             if member.is_signature:
                 yield from member.signature.members.flatten(path=(*path, name))
 
-    def create(self, *, path=()):
+    def create(self, *, path=None, src_loc_at=0):
+        if path is None:
+            path = (tracer.get_var_name(depth=2 + src_loc_at, default="$signature"),)
         attrs = {}
         for name, member in self.items():
-            def create_value(path):
+            def create_value(path, *, src_loc_at):
                 if member.is_port:
-                    return Signal(member.shape, reset=member.reset,
+                    return Signal(member.shape, reset=member.reset, src_loc_at=1 + src_loc_at,
                                   name="__".join(str(item) for item in path))
                 if member.is_signature:
-                    return member.signature.create(path=path)
+                    return member.signature.create(path=path, src_loc_at=1 + src_loc_at)
                 assert False # :nocov:
-            def create_dimensions(dimensions, *, path):
+            def create_dimensions(dimensions, *, path, src_loc_at):
                 if not dimensions:
-                    return create_value(path)
+                    return create_value(path, src_loc_at=1 + src_loc_at)
                 dimension, *rest_of_dimensions = dimensions
-                return [create_dimensions(rest_of_dimensions, path=(*path, index))
+                return [create_dimensions(rest_of_dimensions, path=(*path, index),
+                                          src_loc_at=1 + src_loc_at)
                         for index in range(dimension)]
-            attrs[name] = create_dimensions(member.dimensions, path=(*path, name))
+            attrs[name] = create_dimensions(member.dimensions, path=(*path, name),
+                                            src_loc_at=1 + src_loc_at)
         return attrs
 
     def __repr__(self):
@@ -503,8 +508,8 @@ class Signature(metaclass=SignatureMeta):
                     continue
         return result
 
-    def create(self, *, path=()):
-        return PureInterface(self, path=path)
+    def create(self, *, path=None, src_loc_at=0):
+        return PureInterface(self, path=path, src_loc_at=1 + src_loc_at)
 
     def __repr__(self):
         if type(self) is Signature:
@@ -593,18 +598,19 @@ class FlippedSignature:
         except AttributeError:
             delattr(self.__unflipped, name)
 
-    def create(self, *, path=()):
-        return flipped(self.__unflipped.create(path=path))
+    def create(self, *args, path=None, src_loc_at=0, **kwargs):
+        return flipped(self.__unflipped.create(*args, path=path, src_loc_at=1 + src_loc_at,
+                                               **kwargs))
 
     def __repr__(self):
         return f"{self.__unflipped!r}.flip()"
 
 
 class PureInterface:
-    def __init__(self, signature, *, path):
+    def __init__(self, signature, *, path=None, src_loc_at=0):
         self.__dict__.update({
             "signature": signature,
-            **signature.members.create(path=path)
+            **signature.members.create(path=path, src_loc_at=1 + src_loc_at)
         })
 
     def __repr__(self):
@@ -864,7 +870,7 @@ class Component(Elaboratable):
             if hasattr(self, name):
                 raise NameError(f"Cannot initialize attribute for signature member {name!r} "
                                 f"because an attribute with the same name already exists")
-        self.__dict__.update(self.signature.members.create())
+        self.__dict__.update(self.signature.members.create(path=()))
 
     @property
     def signature(self):
diff --git a/amaranth/vendor/_gowin.py b/amaranth/vendor/_gowin.py
index 7ccec51..9409cf2 100644
--- a/amaranth/vendor/_gowin.py
+++ b/amaranth/vendor/_gowin.py
@@ -227,7 +227,6 @@ class GowinPlatform(TemplatedPlatform):
                 read_ilang {{file}}
             {% endfor %}
             read_ilang {{name}}.il
-            delete w:$verilog_initial_trigger
             {{get_override("script_after_read")|default("# (script_after_read placeholder)")}}
             synth_gowin {{get_override("synth_opts")|options}} -top {{name}} -json {{name}}.syn.json
             {{get_override("script_after_synth")|default("# (script_after_synth placeholder)")}}
diff --git a/amaranth/vendor/_intel.py b/amaranth/vendor/_intel.py
index d26fd67..8f2ecc2 100644
--- a/amaranth/vendor/_intel.py
+++ b/amaranth/vendor/_intel.py
@@ -197,7 +197,6 @@ class IntelPlatform(TemplatedPlatform):
                 read_ilang {{file}}
             {% endfor %}
             read_ilang {{name}}.il
-            delete w:$verilog_initial_trigger
             {{get_override("script_after_read")|default("# (script_after_read placeholder)")}}
             synth_intel_alm {{get_override("synth_opts")|options}} -top {{name}}
             {{get_override("script_after_synth")|default("# (script_after_synth placeholder)")}}
diff --git a/amaranth/vendor/_lattice_ecp5.py b/amaranth/vendor/_lattice_ecp5.py
index bd2e8f7..261ccbe 100644
--- a/amaranth/vendor/_lattice_ecp5.py
+++ b/amaranth/vendor/_lattice_ecp5.py
@@ -117,7 +117,6 @@ class LatticeECP5Platform(TemplatedPlatform):
                 read_ilang {{file}}
             {% endfor %}
             read_ilang {{name}}.il
-            delete w:$verilog_initial_trigger
             {{get_override("script_after_read")|default("# (script_after_read placeholder)")}}
             synth_ecp5 {{get_override("synth_opts")|options}} -top {{name}}
             {{get_override("script_after_synth")|default("# (script_after_synth placeholder)")}}
diff --git a/amaranth/vendor/_lattice_ice40.py b/amaranth/vendor/_lattice_ice40.py
index ee27354..34f2363 100644
--- a/amaranth/vendor/_lattice_ice40.py
+++ b/amaranth/vendor/_lattice_ice40.py
@@ -119,7 +119,6 @@ class LatticeICE40Platform(TemplatedPlatform):
                 read_ilang {{file}}
             {% endfor %}
             read_ilang {{name}}.il
-            delete w:$verilog_initial_trigger
             {{get_override("script_after_read")|default("# (script_after_read placeholder)")}}
             synth_ice40 {{get_override("synth_opts")|options}} -top {{name}}
             {{get_override("script_after_synth")|default("# (script_after_synth placeholder)")}}


Test patch:
diff --git a/tests/test_lib_wiring.py b/tests/test_lib_wiring.py
index ac6e2f0..9bdef91 100644
--- a/tests/test_lib_wiring.py
+++ b/tests/test_lib_wiring.py
@@ -279,9 +279,9 @@ class SignatureMembersTestCase(unittest.TestCase):
         self.assertEqual(list(attrs.keys()), ["a", "s"])
         self.assertIsInstance(attrs["a"], Signal)
         self.assertEqual(attrs["a"].shape(), unsigned(1))
-        self.assertEqual(attrs["a"].name, "a")
+        self.assertEqual(attrs["a"].name, "attrs__a")
         self.assertEqual(attrs["s"].b.shape(), unsigned(2))
-        self.assertEqual(attrs["s"].b.name, "s__b")
+        self.assertEqual(attrs["s"].b.name, "attrs__s__b")
 
     def test_create_reset(self):
         members = SignatureMembers({
@@ -301,7 +301,7 @@ class SignatureMembersTestCase(unittest.TestCase):
         for x in members["a"]:
             for y in x:
                 self.assertIsInstance(y, Signal)
-        self.assertEqual(members["a"][1][2].name, "a__1__2")
+        self.assertEqual(members["a"][1][2].name, "members__a__1__2")
 
     def test_repr(self):
         self.assertEqual(repr(SignatureMembers({})),
@@ -773,8 +773,8 @@ class FlippedInterfaceTestCase(unittest.TestCase):
                 return 69
 
         class CustomSignature(Signature):
-            def create(self, *, path=()):
-                return CustomInterface(self, path=path)
+            def create(self, *, path=None, src_loc_at=0):
+                return CustomInterface(self, path=path, src_loc_at=1 + src_loc_at)
 
         flipped_interface = CustomSignature({}).flip().create()
         self.assertTrue(hasattr(flipped_interface, "custom_method"))