Repository: amaranth-lang/amaranth
Pull request #967 (2023-11-26 08:31:21)

Add support for dumping structure fields in VCD
See #790.

This PR adds an entirely private API for describing formatting of values that is used in the standard library, in departure from our standing policy of not using private APIs in the standard library.

This is a temporary measure intended to get the version 0.4 released faster, as it has been years in the making. It is expected that this API will be made public in the version 0.5 after going through the usual RFC process.

This PR only adds VCD lines for fields defined in `lib.data.Layout` when using `sim.pysim`. The emitted RTLIL and Verilog remain the same. It is expected that when `sim.cxxsim` lands, RTLIL/Verilog output will include aliases for layout fields as well.

The value representation API also handles formatting of enumerations, with no changes visible to the designer. The implementation of `Signal(decoder=)` is changed as well to use the new API, with full backwards compatibility and no public API changes.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/967?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) Report
Attention: `29 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`79adbed`)](https://app.codecov.io/gh/amaranth-lang/amaranth/commit/79adbed3134a4490887a6b4450381217136ffe93?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) 83.67% compared to head [(`e46c84c`)](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/967?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) 83.31%.

| [Files](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/967?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) | Patch % | Lines |
|---|---|---|
| [amaranth/sim/pysim.py](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/967?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang#diff-YW1hcmFudGgvc2ltL3B5c2ltLnB5) | 68.42% | [14 Missing and 4 partials :warning: ](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/967?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) |
| [amaranth/hdl/\_repr.py](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/967?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang#diff-YW1hcmFudGgvaGRsL19yZXByLnB5) | 80.64% | [6 Missing :warning: ](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/967?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) |
| [amaranth/back/rtlil.py](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/967?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang#diff-YW1hcmFudGgvYmFjay9ydGxpbC5weQ==) | 28.57% | [4 Missing and 1 partial :warning: ](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/967?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main     #967      +/-   ##
==========================================
- Coverage   83.67%   83.31%   -0.37%     
==========================================
  Files          54       55       +1     
  Lines        7784     7867      +83     
  Branches     1912     1672     -240     
==========================================
+ Hits         6513     6554      +41     
- Misses       1062     1103      +41     
- Partials      209      210       +1     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/967?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang).


Patch:
diff --git a/amaranth/back/rtlil.py b/amaranth/back/rtlil.py
index e824919..bd6a3f8 100644
--- a/amaranth/back/rtlil.py
+++ b/amaranth/back/rtlil.py
@@ -5,7 +5,7 @@ import warnings
 import re
 
 from .._utils import bits_for, flatten
-from ..hdl import ast, ir, mem, xfrm
+from ..hdl import ast, ir, mem, xfrm, _repr
 from ..lib import wiring
 
 
@@ -337,12 +337,14 @@ class _ValueCompilerState:
             wire_name = signal.name
 
         is_sync_driven = signal in self.driven and self.driven[signal]
-        
+
         attrs = dict(signal.attrs)
-        if signal._enum_class is not None:
-            attrs["enum_base_type"] = signal._enum_class.__name__
-            for value in signal._enum_class:
-                attrs["enum_value_{:0{}b}".format(value.value, signal.width)] = value.name
+        for repr in signal._value_repr:
+            if repr.path == () and isinstance(repr.format, _repr.FormatEnum):
+                enum = repr.format.enum
+                attrs["enum_base_type"] = enum.__name__
+                for value in enum:
+                    attrs["enum_value_{:0{}b}".format(value.value, signal.width)] = value.name
 
         # For every signal in the sync domain, assign \sig's initial value (using the \init reg
         # attribute) to the reset value.
@@ -872,7 +874,7 @@ def _convert_fragment(builder, fragment, name_map, hierarchy):
 
             if sub_type == "$mem_v2" and "MEMID" not in sub_params:
                 sub_params["MEMID"] = builder._make_name(sub_name, local=False)
-            
+
             sub_ports = OrderedDict()
             for port, value in sub_port_map.items():
                 if not isinstance(subfragment, ir.Instance):
@@ -917,7 +919,7 @@ def _convert_fragment(builder, fragment, name_map, hierarchy):
                     stmt_compiler._has_rhs = False
                     stmt_compiler._wrap_assign = False
                     stmt_compiler(group_stmts)
-        
+
         # For every driven signal in the sync domain, create a flop of appropriate type. Which type
         # is appropriate depends on the domain: for domains with sync reset, it is a $dff, for
         # domains with async reset it is an $adff. The latter is directly provided with the reset
@@ -930,7 +932,7 @@ def _convert_fragment(builder, fragment, name_map, hierarchy):
             wire_curr, wire_next = compiler_state.resolve(signal)
 
             if not cd.async_reset:
-                # For sync reset flops, the reset value comes from logic inserted by 
+                # For sync reset flops, the reset value comes from logic inserted by
                 # `hdl.xfrm.DomainLowerer`.
                 module.cell("$dff", ports={
                     "\\CLK": wire_clk,
diff --git a/amaranth/hdl/_repr.py b/amaranth/hdl/_repr.py
new file mode 100644
index 0000000..adecc87
--- /dev/null
+++ b/amaranth/hdl/_repr.py
@@ -0,0 +1,46 @@
+from abc import ABCMeta, abstractmethod
+
+
+__all__ = ["Format", "FormatInt", "FormatEnum", "FormatCustom", "Repr"]
+
+
+class Format(metaclass=ABCMeta):
+    @abstractmethod
+    def format(self, value):
+        raise NotImplementedError
+
+
+class FormatInt(Format):
+    def format(self, value):
+        return f"{value:d}"
+
+
+class FormatEnum(Format):
+    def __init__(self, enum):
+        self.enum = enum
+
+    def format(self, value):
+        try:
+            return f"{self.enum(value).name}/{value:d}"
+        except ValueError:
+            return f"?/{value:d}"
+
+
+class FormatCustom(Format):
+    def __init__(self, formatter):
+        self.formatter = formatter
+
+    def format(self, value):
+        return self.formatter(value)
+
+
+class Repr:
+    def __init__(self, format, value, *, path=()):
+        from .ast import Value # avoid a circular dependency
+        assert isinstance(format, Format)
+        assert isinstance(value, Value)
+        assert isinstance(path, tuple) and all(isinstance(part, (str, int)) for part in path)
+
+        self.format = format
+        self.value  = value
+        self.path   = path
diff --git a/amaranth/hdl/ast.py b/amaranth/hdl/ast.py
index 461345e..6d5107e 100644
--- a/amaranth/hdl/ast.py
+++ b/amaranth/hdl/ast.py
@@ -7,6 +7,7 @@ from collections.abc import Iterable, MutableMapping, MutableSet, MutableSequenc
 from enum import Enum
 from itertools import chain
 
+from ._repr import *
 from .. import tracer
 from .._utils import *
 from .._utils import _ignore_deprecated
@@ -53,6 +54,9 @@ class ShapeCastable:
             raise TypeError(f"Class '{cls.__name__}' deriving from `ShapeCastable` must override "
                             f"the `const` method")
 
+    def _value_repr(self, value):
+        return (Repr(FormatInt(), value),)
+
 
 class Shape:
     """Bit width and signedness of a value.
@@ -1127,19 +1131,51 @@ class Signal(Value, DUID, metaclass=_SignalMeta):
 
         self.attrs = OrderedDict(() if attrs is None else attrs)
 
-        if decoder is None and isinstance(orig_shape, type) and issubclass(orig_shape, Enum):
-            decoder = orig_shape
-        if isinstance(decoder, type) and issubclass(decoder, Enum):
+        if decoder is not None:
+            # The value representation is specified explicitly. Since we do not expose `hdl._repr`,
+            # this is the only way to add a custom filter to the signal right now. The setter sets
+            # `self._value_repr` as well as the compatibility `self.decoder`.
+            self.decoder = decoder
+        else:
+            # If it's an enum, expose it via `self.decoder` for compatibility, whether it's a Python
+            # enum or an Amaranth enum. This also sets the value representation, even for custom
+            # shape-castables that implement their own `_value_repr`.
+            if isinstance(orig_shape, type) and issubclass(orig_shape, Enum):
+                self.decoder = orig_shape
+            else:
+                self.decoder = None
+            # The value representation is specified implicitly in the shape of the signal.
+            if isinstance(orig_shape, ShapeCastable):
+                # A custom shape-castable always has a `_value_repr`, at least the default one.
+                self._value_repr = tuple(orig_shape._value_repr(self))
+            elif isinstance(orig_shape, type) and issubclass(orig_shape, Enum):
+                # A non-Amaranth enum needs a value repr constructed for it.
+                self._value_repr = (Repr(FormatEnum(orig_shape), self),)
+            else:
+                # Any other case is formatted as a plain integer.
+                self._value_repr = (Repr(FormatInt(), self),)
+
+    @property
+    def decoder(self):
+        return self._decoder
+
+    @decoder.setter
+    def decoder(self, decoder):
+        # Compute the value representation that will be used by Amaranth.
+        if decoder is None:
+            self._value_repr = (Repr(FormatInt(), self),)
+            self._decoder = None
+        elif not (isinstance(decoder, type) and issubclass(decoder, Enum)):
+            self._value_repr = (Repr(FormatCustom(decoder), self),)
+            self._decoder = decoder
+        else: # Violence. In the name of backwards compatibility!
+            self._value_repr = (Repr(FormatEnum(decoder), self),)
             def enum_decoder(value):
                 try:
                     return "{0.name:}/{0.value:}".format(decoder(value))
                 except ValueError:
                     return str(value)
-            self.decoder = enum_decoder
-            self._enum_class = decoder
-        else:
-            self.decoder = decoder
-            self._enum_class = None
+            self._decoder = enum_decoder
 
     # Not a @classmethod because amaranth.compat requires it.
     @staticmethod
@@ -1914,3 +1950,6 @@ class SignalDict(_MappedKeyDict):
 class SignalSet(_MappedKeySet):
     _map_key   = SignalKey
     _unmap_key = lambda self, key: key.signal
+
+
+from ._repr import *
diff --git a/amaranth/lib/data.py b/amaranth/lib/data.py
index 7c23836..151696e 100644
--- a/amaranth/lib/data.py
+++ b/amaranth/lib/data.py
@@ -1,8 +1,10 @@
 from abc import ABCMeta, abstractmethod
+from enum import Enum
 from collections.abc import Mapping, Sequence
 import warnings
 
 from amaranth.hdl import *
+from amaranth.hdl._repr import *
 from amaranth.hdl.ast import ShapeCastable, ValueCastable
 
 
@@ -231,6 +233,21 @@ class Layout(ShapeCastable, metaclass=ABCMeta):
             int_value |= (key_value.value << field.offset) & mask
         return View(self, Const(int_value, self.as_shape()))
 
+    def _value_repr(self, value):
+        yield Repr(FormatInt(), value)
+        for key, field in self:
+            shape = Shape.cast(field.shape)
+            field_value = value[field.offset:field.offset+shape.width]
+            if shape.signed:
+                field_value = field_value.as_signed()
+            if isinstance(field.shape, ShapeCastable):
+                for repr in field.shape._value_repr(field_value):
+                    yield Repr(repr.format, repr.value, path=(key,) + repr.path)
+            elif isinstance(field.shape, type) and issubclass(field.shape, Enum):
+                yield Repr(FormatEnum(field.shape), field_value, path=(key,))
+            else:
+                yield Repr(FormatInt(), field_value, path=(key,))
+
 
 class StructLayout(Layout):
     """Description of a structure layout.
@@ -774,6 +791,9 @@ class _AggregateMeta(ShapeCastable, type):
             fields.update(init or {})
             return cls.as_shape().const(fields)
 
+    def _value_repr(cls, value):
+        return cls.__layout._value_repr(value)
+
 
 class Struct(View, metaclass=_AggregateMeta):
     """Structures defined with annotations.
diff --git a/amaranth/lib/enum.py b/amaranth/lib/enum.py
index 7800375..02e9dfe 100644
--- a/amaranth/lib/enum.py
+++ b/amaranth/lib/enum.py
@@ -2,6 +2,7 @@ import enum as py_enum
 import warnings
 
 from ..hdl.ast import Value, Shape, ShapeCastable, Const
+from ..hdl._repr import *
 
 
 __all__ = py_enum.__all__
@@ -150,6 +151,9 @@ class EnumMeta(ShapeCastable, py_enum.EnumMeta):
             member = cls(init)
         return Const(member.value, cls.as_shape())
 
+    def _value_repr(cls, value):
+        yield Repr(FormatEnum(cls), value)
+
 
 class Enum(py_enum.Enum):
     """Subclass of the standard :class:`enum.Enum` that has :class:`EnumMeta` as
diff --git a/amaranth/lib/io.py b/amaranth/lib/io.py
index a208163..54bd9c0 100644
--- a/amaranth/lib/io.py
+++ b/amaranth/lib/io.py
@@ -4,7 +4,7 @@ from .. import *
 with warnings.catch_warnings():
     warnings.filterwarnings(action="ignore", category=DeprecationWarning)
     from ..hdl.rec import *
-from ..lib.wiring import In, Out, Signature
+from ..lib.wiring import In, Out, Signature, flipped, FlippedInterface
 
 
 __all__ = ["pin_layout", "Pin"]
@@ -135,4 +135,7 @@ class Pin(Record):
         first_field, _, _ = next(iter(Pin(1, dir="o").layout))
         warnings.warn(f"`pin.eq(...)` is deprecated; use `pin.{first_field}.eq(...)` here",
                       DeprecationWarning, stacklevel=2)
-        return super().eq(other)
+        if isinstance(self, FlippedInterface):
+            return Record.eq(flipped(self), other)
+        else:
+            return Record.eq(self, other)
diff --git a/amaranth/lib/wiring.py b/amaranth/lib/wiring.py
index e34d895..624d98a 100644
--- a/amaranth/lib/wiring.py
+++ b/amaranth/lib/wiring.py
@@ -353,6 +353,12 @@ class Signature(metaclass=SignatureMeta):
     def members(self):
         return self.__members
 
+    @members.setter
+    def members(self, new_members):
+        # The setter is called when `sig.members += ...` is used.
+        if new_members is not self.__members:
+            raise AttributeError("property 'members' of 'Signature' object cannot be set")
+
     def __eq__(self, other):
         other_unflipped = other.flip() if type(other) is FlippedSignature else other
         if type(self) is type(other_unflipped) is Signature:
@@ -394,11 +400,26 @@ class Signature(metaclass=SignatureMeta):
                     dimension, *rest_of_dimensions = dimensions
                     for index in range(dimension):
                         yield from iter_dimensions(value[index], rest_of_dimensions,
-                                                   path=(path, index))
+                                                   path=(*path, index))
 
             yield from iter_dimensions(value, dimensions=member.dimensions, path=path)
 
     def is_compliant(self, obj, *, reasons=None, path=("obj",)):
+        if not hasattr(obj, "signature"):
+            if reasons is not None:
+                reasons.append(f"{_format_path(path)} does not have an attribute 'signature'")
+            return False
+        if not isinstance(obj.signature, Signature):
+            if reasons is not None:
+                reasons.append(f"{_format_path(path + ('signature',))} is expected to be "
+                               f"a signature, but it is a {obj.signature!r}")
+            return False
+        if self != obj.signature:
+            if reasons is not None:
+                reasons.append(f"{_format_path(path + ('signature',))} is expected to be equal "
+                               f"to this signature, {self!r}, but it is a {obj.signature!r}")
+            return False
+
         def check_attr_value(member, attr_value, *, path):
             if member.is_port:
                 try:
@@ -491,6 +512,19 @@ class Signature(metaclass=SignatureMeta):
         return super().__repr__()
 
 
+def _gettypeattr(obj, attr):
+    # Resolve the attribute on the object's class, without triggering the descriptor protocol for
+    # attributes that are class methods, etc.
+    for cls in type(obj).__mro__:
+        try:
+            return cls.__dict__[attr]
+        except KeyError:
+            pass
+    # Call `getattr` In case there is `__getattr__` on the metaclass, or just to generate
+    # an `AttributeError` with the standard message.
+    return getattr(type(obj), attr)
+
+
 # To simplify implementation and reduce API surface area `FlippedSignature` is made final. This
 # restriction could be lifted if there is a compelling use case.
 @final
@@ -505,6 +539,11 @@ class FlippedSignature:
     def members(self):
         return FlippedSignatureMembers(self.__unflipped.members)
 
+    @members.setter
+    def members(self, new_members):
+        if new_members.flip() is not self.__unflipped.members:
+            raise AttributeError("property 'members' of 'FlippedSignature' object cannot be set")
+
     def __eq__(self, other):
         if type(other) is FlippedSignature:
             # Trivial case.
@@ -524,15 +563,35 @@ class FlippedSignature:
     is_compliant = Signature.is_compliant
 
     # FIXME: document this logic
+
+    # Because we would like to forward attribute access (other than what is explicitly overridden)
+    # to the unflipped signature, including access via e.g. @property-decorated functions, we have
+    # to reimplement the Python decorator protocol here. Note that in all of these functions, there
+    # are two possible exits via `except AttributeError`: from `getattr` and from `.__get__()`.
+
     def __getattr__(self, name):
-        value = getattr(self.__unflipped, name)
-        if inspect.ismethod(value):
-            return types.MethodType(value.__func__, self)
-        else:
-            return value
+        try: # descriptor first
+            return _gettypeattr(self.__unflipped, name).__get__(self, type(self.__unflipped))
+        except AttributeError:
+            return getattr(self.__unflipped, name)
 
     def __setattr__(self, name, value):
-        return setattr(self.__unflipped, name, value)
+        if name == "members":
+            # Although `sig.flip().members` does not call `__getattr__` but directly invokes
+            # the descriptor of the `FlippedSignature.members` property, `sig.flip().members +=`
+            # does call `__setattr__`, and this must be special-cased for the setter to work.
+            FlippedSignature.members.__set__(self, value)
+        else:
+            try: # descriptor first
+                _gettypeattr(self.__unflipped, name).__set__(self, value)
+            except AttributeError:
+                setattr(self.__unflipped, name, value)
+
+    def __delattr__(self, name):
+        try: # descriptor first
+            _gettypeattr(self.__unflipped, name).__delete__(self)
+        except AttributeError:
+            delattr(self.__unflipped, name)
 
     def create(self, *, path=()):
         return flipped(self.__unflipped.create(path=path))
@@ -566,18 +625,35 @@ class FlippedInterface:
         return type(self) is type(other) and self.__unflipped == other.__unflipped
 
     # FIXME: document this logic
+
+    # See the note in `FlippedSignature`. In addition, these accessors also handle flipping of
+    # an interface member.
+
     def __getattr__(self, name):
-        value = getattr(self.__unflipped, name)
-        if inspect.ismethod(value):
-            return types.MethodType(value.__func__, self)
-        elif name in self.__unflipped.signature.members and \
-                self.__unflipped.signature.members[name].is_signature:
-            return flipped(value)
+        if (name in self.__unflipped.signature.members and
+                self.__unflipped.signature.members[name].is_signature):
+            return flipped(getattr(self.__unflipped, name))
         else:
-            return value
+            try: # descriptor first
+                return _gettypeattr(self.__unflipped, name).__get__(self, type(self.__unflipped))
+            except AttributeError:
+                return getattr(self.__unflipped, name)
 
     def __setattr__(self, name, value):
-        return setattr(self.__unflipped, name, value)
+        if (name in self.__unflipped.signature.members and
+                self.__unflipped.signature.members[name].is_signature):
+            setattr(self.__unflipped, name, flipped(value))
+        else:
+            try: # descriptor first
+                _gettypeattr(self.__unflipped, name).__set__(self, value)
+            except AttributeError:
+                setattr(self.__unflipped, name, value)
+
+    def __delattr__(self, name):
+        try: # descriptor first
+            _gettypeattr(self.__unflipped, name).__delete__(self)
+        except AttributeError:
+            delattr(self.__unflipped, name)
 
     def __repr__(self):
         return f"flipped({self.__unflipped!r})"
diff --git a/amaranth/sim/pysim.py b/amaranth/sim/pysim.py
index 7cfe340..f6d3e5c 100644
--- a/amaranth/sim/pysim.py
+++ b/amaranth/sim/pysim.py
@@ -5,7 +5,8 @@ from vcd import VCDWriter
 from vcd.gtkw import GTKWSave
 
 from ..hdl import *
-from ..hdl.ast import SignalDict
+from ..hdl._repr import *
+from ..hdl.ast import SignalDict, Slice, Operator
 from ._base import *
 from ._pyrtl import _FragmentCompiler
 from ._pycoro import PyCoroProcess
@@ -17,8 +18,24 @@ __all__ = ["PySimEngine"]
 
 class _VCDWriter:
     @staticmethod
-    def decode_to_vcd(signal, value):
-        return signal.decoder(value).expandtabs().replace(" ", "_")
+    def decode_to_vcd(format, value):
+        return format.format(value).expandtabs().replace(" ", "_")
+
+    @staticmethod
+    def eval_field(field, signal, value):
+        if isinstance(field, Signal):
+            assert field is signal
+            return value
+        elif isinstance(field, Const):
+            return field.value
+        elif isinstance(field, Slice):
+            sub = _VCDWriter.eval_field(field.value, signal, value)
+            return (sub >> field.start) & ((1 << (field.stop - field.start)) - 1)
+        elif isinstance(field, Operator) and field.operator in ('s', 'u'):
+            sub = _VCDWriter.eval_field(field.operands[0], signal, value)
+            return Const(sub, field.shape()).value
+        else:
+            raise NotImplementedError
 
     def __init__(self, fragment, *, vcd_file, gtkw_file=None, traces=()):
         if isinstance(vcd_file, str):
@@ -55,53 +72,59 @@ class _VCDWriter:
             return
 
         for signal, names in itertools.chain(signal_names.items(), trace_names.items()):
-            if signal.decoder:
-                var_type = "string"
-                var_size = 1
-                var_init = self.decode_to_vcd(signal, signal.reset)
-            else:
-                var_type = "wire"
-                var_size = signal.width
-                var_init = signal.reset
-
-            for (*var_scope, var_name) in names:
-                if re.search(r"[ \t\r\n]", var_name):
-                    raise NameError("Signal '{}.{}' contains a whitespace character"
-                                    .format(".".join(var_scope), var_name))
-
-                suffix = None
-                while True:
-                    try:
-                        if suffix is None:
-                            var_name_suffix = var_name
+            fields = []
+            self.vcd_vars[signal] = []
+            self.gtkw_names[signal] = []
+            for repr in signal._value_repr:
+                var_init = self.eval_field(repr.value, signal, signal.reset)
+                if isinstance(repr.format, FormatInt):
+                    var_type = "wire"
+                    var_size = repr.value.shape().width
+                else:
+                    var_type = "string"
+                    var_size = 1
+                    var_init = self.decode_to_vcd(repr.format, var_init)
+
+                vcd_var = None
+                for (*var_scope, var_name) in names:
+                    if re.search(r"[ \t\r\n]", var_name):
+                        raise NameError("Signal '{}.{}' contains a whitespace character"
+                                        .format(".".join(var_scope), var_name))
+
+                    field_name = var_name
+                    for item in repr.path:
+                        if isinstance(item, int):
+                            field_name += f"[{item}]"
+                        else:
+                            field_name += f".{item}"
+
+                    if vcd_var is None:
+                        vcd_var = self.vcd_writer.register_var(
+                            scope=var_scope, name=field_name,
+                            var_type=var_type, size=var_size, init=var_init)
+                        if var_size > 1:
+                            suffix = f"[{var_size - 1}:0]"
                         else:
-                            var_name_suffix = f"{var_name}${suffix}"
-                        if signal not in self.vcd_vars:
-                            vcd_var = self.vcd_writer.register_var(
-                                scope=var_scope, name=var_name_suffix,
-                                var_type=var_type, size=var_size, init=var_init)
-                            self.vcd_vars[signal] = vcd_var
+                            suffix = ""
+                        if repr.path:
+                            gtkw_field_name = '\\' + field_name
                         else:
-                            self.vcd_writer.register_alias(
-                                scope=var_scope, name=var_name_suffix,
-                                var=self.vcd_vars[signal])
-                        break
-                    except KeyError:
-                        suffix = (suffix or 0) + 1
+                            gtkw_field_name = field_name
+                        self.gtkw_names[signal].append(".".join((*var_scope, gtkw_field_name)) + suffix)
+                    else:
+                        self.vcd_writer.register_alias(
+                            scope=var_scope, name=field_name,
+                            var=vcd_var)
 
-                if signal not in self.gtkw_names:
-                    self.gtkw_names[signal] = (*var_scope, var_name_suffix)
+                self.vcd_vars[signal].append((vcd_var, repr))
 
-    def update(self, timestamp, signal, value):
-        vcd_var = self.vcd_vars.get(signal)
-        if vcd_var is None:
-            return
 
-        if signal.decoder:
-            var_value = self.decode_to_vcd(signal, value)
-        else:
-            var_value = value
-        self.vcd_writer.change(vcd_var, timestamp, var_value)
+    def update(self, timestamp, signal, value):
+        for (vcd_var, repr) in self.vcd_vars.get(signal, ()):
+            var_value = self.eval_field(repr.value, signal, value)
+            if not isinstance(repr.format, FormatInt):
+                var_value = self.decode_to_vcd(repr.format, var_value)
+            self.vcd_writer.change(vcd_var, timestamp, var_value)
 
     def close(self, timestamp):
         if self.vcd_writer is not None:
@@ -113,11 +136,8 @@ class _VCDWriter:
 
             self.gtkw_save.treeopen("top")
             for signal in self.traces:
-                if len(signal) > 1 and not signal.decoder:
-                    suffix = f"[{len(signal) - 1}:0]"
-                else:
-                    suffix = ""
-                self.gtkw_save.trace(".".join(self.gtkw_names[signal]) + suffix)
+                for name in self.gtkw_names[signal]:
+                    self.gtkw_save.trace(name)
 
         if self.vcd_file is not None:
             self.vcd_file.close()
diff --git a/docs/_static/custom.css b/docs/_static/custom.css
index 43d1abf..163b9fa 100644
--- a/docs/_static/custom.css
+++ b/docs/_static/custom.css
@@ -1,5 +1,10 @@
 /* Some of our section titles are looong */
-.wy-menu-vertical { width: 340px; }
+.wy-nav-side, .wy-side-scroll, .wy-menu-vertical { width: 340px; }
+.wy-side-nav-search { width: 325px; }
+.wy-nav-content-wrap { margin-left: 340px; }
+
+/* We don't have a version picker widget */
+.wy-nav-side { padding-bottom: 0; }
 
 /* Many of our diagnostics are even longer */
 .rst-content pre.literal-block, .rst-content div[class^="highlight"] pre, .rst-content .linenodiv pre { white-space: pre-wrap; }


Test patch:
diff --git a/tests/test_build_res.py b/tests/test_build_res.py
index b017c6e..edb32a0 100644
--- a/tests/test_build_res.py
+++ b/tests/test_build_res.py
@@ -315,3 +315,12 @@ class ResourceManagerTestCase(FHDLTestCase):
                 (r"^Cannot add clock constraint on \(sig clk100_0__i\), which is already "
                     r"constrained to 100000000\.0 Hz$")):
             self.cm.add_clock_constraint(clk100.i, 1e6)
+
+    def test_eq_deprecation(self):
+        user_led = self.cm.request("user_led", 0)
+        m = Module()
+        with self.assertWarns(DeprecationWarning):
+            m.d.sync += user_led.eq(1)
+        p = Pin(4, "o")
+        with self.assertWarns(DeprecationWarning):
+            m.d.sync += p.eq(1)
diff --git a/tests/test_lib_wiring.py b/tests/test_lib_wiring.py
index a1f5262..2ee96c2 100644
--- a/tests/test_lib_wiring.py
+++ b/tests/test_lib_wiring.py
@@ -374,6 +374,18 @@ class SignatureTestCase(unittest.TestCase):
                 r"^Cannot add members to a frozen signature$"):
             sig.members += {"b": Out(1)}
 
+    def test_members_plus_equal(self):
+        sig = Signature({})
+        # This invokes the setter of Signature.members.
+        sig.members += {"a": In(1)}
+        self.assertIn("a", sig.members)
+
+    def test_members_equal_wrong(self):
+        sig = Signature({})
+        with self.assertRaisesRegex(AttributeError,
+                r"property 'members' of 'Signature' object cannot be set"):
+            sig.members = SignatureMembers({})
+
     def assertFlattenedSignature(self, actual, expected):
         for (a_path, a_member, a_value), (b_path, b_member, b_value) in zip(actual, expected):
             self.assertEqual(a_path, b_path)
@@ -385,8 +397,8 @@ class SignatureTestCase(unittest.TestCase):
         intf = sig.create()
         self.assertFlattenedSignature(sig.flatten(intf), [
             (("a",), In(1), intf.a),
-            ((("b",), 0), Out(2), intf.b[0]),
-            ((("b",), 1), Out(2), intf.b[1])
+            (("b", 0), Out(2), intf.b[0]),
+            (("b", 1), Out(2), intf.b[1])
         ])
 
     def test_flatten_sig(self):
@@ -404,7 +416,32 @@ class SignatureTestCase(unittest.TestCase):
             (("d", "s"), In (1), intf.d.s),
         ])
 
+    def test_is_compliant_signature(self):
+        sig = Signature({})
+
+        obj1 = NS()
+        self.assertFalse(sig.is_compliant(obj1))
+        reasons = []
+        self.assertFalse(sig.is_compliant(obj1, reasons=reasons))
+        self.assertEqual(reasons, ["'obj' does not have an attribute 'signature'"])
+
+        obj = NS(signature=1)
+        self.assertFalse(sig.is_compliant(obj))
+        reasons = []
+        self.assertFalse(sig.is_compliant(obj, reasons=reasons))
+        self.assertEqual(reasons, ["'obj.signature' is expected to be a signature, but it is a 1"])
+
+        obj = NS(signature=Signature({"a": In(1)}))
+        self.assertFalse(sig.is_compliant(obj))
+        reasons = []
+        self.assertFalse(sig.is_compliant(obj, reasons=reasons))
+        self.assertEqual(reasons, [
+            "'obj.signature' is expected to be equal to this signature, "
+            "Signature({}), but it is a Signature({'a': In(1)})"
+        ])
+
     def assertNotCompliant(self, reason_regex, sig, obj):
+        obj.signature = sig
         self.assertFalse(sig.is_compliant(obj))
         reasons = []
         self.assertFalse(sig.is_compliant(obj, reasons=reasons))
@@ -462,25 +499,32 @@ class SignatureTestCase(unittest.TestCase):
         self.assertNotCompliant(
             r"^'obj\.a' does not have an attribute 'b'$",
             sig=Signature({"a": Out(Signature({"b": In(1)}))}),
-            obj=NS(a=Signal()))
+            obj=NS(a=NS(signature=Signature({"b": In(1)}))))
         self.assertTrue(
             Signature({"a": In(1)}).is_compliant(
-                NS(a=Signal())))
+                NS(signature=Signature({"a": In(1)}),
+                   a=Signal())))
         self.assertTrue(
             Signature({"a": In(1)}).is_compliant(
-                NS(a=Const(1))))
+                NS(signature=Signature({"a": In(1)}),
+                   a=Const(1))))
         self.assertTrue( # list
             Signature({"a": In(1).array(2, 2)}).is_compliant(
-                NS(a=[[Const(1), Const(1)], [Signal(), Signal()]])))
+                NS(signature=Signature({"a": In(1).array(2, 2)}),
+                   a=[[Const(1), Const(1)], [Signal(), Signal()]])))
         self.assertTrue( # tuple
             Signature({"a": In(1).array(2, 2)}).is_compliant(
-                NS(a=((Const(1), Const(1)), (Signal(), Signal())))))
+                NS(signature=Signature({"a": In(1).array(2, 2)}),
+                   a=((Const(1), Const(1)), (Signal(), Signal())))))
         self.assertTrue( # mixed list and tuple
             Signature({"a": In(1).array(2, 2)}).is_compliant(
-                NS(a=[[Const(1), Const(1)], (Signal(), Signal())])))
+                NS(signature=Signature({"a": In(1).array(2, 2)}),
+                   a=[[Const(1), Const(1)], (Signal(), Signal())])))
         self.assertTrue(
             Signature({"a": Out(Signature({"b": In(1)}))}).is_compliant(
-                NS(a=NS(b=Signal()))))
+                NS(signature=Signature({"a": Out(Signature({"b": In(1)}))}),
+                   a=NS(signature=Signature({"b": In(1)}),
+                        b=Signal()))))
 
     def test_repr(self):
         sig = Signature({"a": In(1)})
@@ -530,7 +574,7 @@ class FlippedSignatureTestCase(unittest.TestCase):
         sig = Signature({"a": In(1)}).flip()
         self.assertEqual(repr(sig), "Signature({'a': In(1)}).flip()")
 
-    def test_getattr_setattr(self):
+    def test_getsetdelattr(self):
         class S(Signature):
             def __init__(self):
                 super().__init__({})
@@ -539,12 +583,73 @@ class FlippedSignatureTestCase(unittest.TestCase):
             def f(self2):
                 self.assertIsInstance(self2, FlippedSignature)
                 return "f()"
+
         sig = S()
         fsig = sig.flip()
         self.assertEqual(fsig.x, 1)
         self.assertEqual(fsig.f(), "f()")
         fsig.y = 2
         self.assertEqual(sig.y, 2)
+        del fsig.y
+        self.assertFalse(hasattr(sig, "y"))
+
+    def test_getsetdelattr_property(self):
+        class S(Signature):
+            def __init__(self):
+                super().__init__({})
+                self.x_get_type = None
+                self.x_set_type = None
+                self.x_set_val = None
+                self.x_del_type = None
+
+            @property
+            def x(self):
+                self.x_get_type = type(self)
+
+            @x.setter
+            def x(self, val):
+                self.x_set_type = type(self)
+                self.x_set_val = val
+
+            @x.deleter
+            def x(self):
+                self.x_del_type = type(self)
+
+        sig = S()
+        fsig = sig.flip()
+        fsig.x
+        fsig.x = 1
+        del fsig.x
+        # Tests both attribute access through the descriptor, and attribute setting without one!
+        self.assertEqual(sig.x_get_type, type(fsig))
+        self.assertEqual(sig.x_set_type, type(fsig))
+        self.assertEqual(sig.x_set_val, 1)
+        self.assertEqual(sig.x_del_type, type(fsig))
+
+    def test_classmethod(self):
+        x_type = None
+        class S(Signature):
+            @classmethod
+            def x(cls):
+                nonlocal x_type
+                x_type = cls
+
+        sig = S({})
+        fsig = sig.flip()
+        fsig.x()
+        self.assertEqual(x_type, S)
+
+    def test_members_plus_equal(self):
+        sig = Signature({})
+        # This invokes the setter of FlippedSignature.members.
+        sig.flip().members += {"a": In(1)}
+        self.assertIn("a", sig.members)
+
+    def test_members_equal_wrong(self):
+        sig = Signature({})
+        with self.assertRaisesRegex(AttributeError,
+                r"property 'members' of 'FlippedSignature' object cannot be set"):
+            sig.flip().members = SignatureMembers({})
 
 
 class InterfaceTestCase(unittest.TestCase):
@@ -564,8 +669,8 @@ class FlippedInterfaceTestCase(unittest.TestCase):
             r"^flipped\(<.+?\.Interface object at .+>\)$")
         self.assertIs(flipped(tintf), intf)
 
-    def test_getattr_setattr(self):
-        class I(Interface):
+    def test_getsetdelattr(self):
+        class I:
             signature = Signature({})
 
             def __init__(self):
@@ -574,27 +679,82 @@ class FlippedInterfaceTestCase(unittest.TestCase):
             def f(self2):
                 self.assertIsInstance(self2, FlippedInterface)
                 return "f()"
+
         intf = I()
-        tintf = flipped(intf)
-        self.assertEqual(tintf.x, 1)
-        self.assertEqual(tintf.f(), "f()")
-        tintf.y = 2
+        fintf = flipped(intf)
+        self.assertEqual(fintf.x, 1)
+        self.assertEqual(fintf.f(), "f()")
+        fintf.y = 2
         self.assertEqual(intf.y, 2)
+        del fintf.y
+        self.assertFalse(hasattr(intf, "y"))
+
+    def test_getsetdelattr_property(self):
+        class I:
+            signature = Signature({})
+
+            def __init__(self):
+                self.x_get_type = None
+                self.x_set_type = None
+                self.x_set_val = None
+                self.x_del_type = None
+
+            @property
+            def x(self):
+                self.x_get_type = type(self)
+
+            @x.setter
+            def x(self, val):
+                self.x_set_type = type(self)
+                self.x_set_val = val
+
+            @x.deleter
+            def x(self):
+                self.x_del_type = type(self)
+
+        intf = I()
+        fintf = flipped(intf)
+        fintf.x
+        fintf.x = 1
+        del fintf.x
+        # Tests both attribute access through the descriptor, and attribute setting without one!
+        self.assertEqual(intf.x_get_type, type(fintf))
+        self.assertEqual(intf.x_set_type, type(fintf))
+        self.assertEqual(intf.x_set_val, 1)
+        self.assertEqual(intf.x_del_type, type(fintf))
+
+    def test_classmethod(self):
+        x_type = None
+        class I:
+            signature = Signature({})
+
+            def __init__(self):
+                pass
+
+            @classmethod
+            def x(cls):
+                nonlocal x_type
+                x_type = cls
+
+        intf = I()
+        fintf = flipped(intf)
+        fintf.x()
+        self.assertEqual(x_type, I)
 
     def test_flipped_wrong(self):
         with self.assertRaisesRegex(TypeError,
                 r"^flipped\(\) can only flip an interface object, not Signature\({}\)$"):
             flipped(Signature({}))
-    
+
     def test_create_subclass_flipped(self):
         class CustomInterface(Interface):
             def custom_method(self):
                 return 69
-        
+
         class CustomSignature(Signature):
             def create(self, *, path=()):
                 return CustomInterface(self, path=path)
-        
+
         flipped_interface = CustomSignature({}).flip().create()
         self.assertTrue(hasattr(flipped_interface, "custom_method"))
 
@@ -630,6 +790,10 @@ class FlippedInterfaceTestCase(unittest.TestCase):
         self.assertEqual(ifsub.g.members["h"].flow, In)
         self.assertEqual(flipped(ifsub).g.members["h"].flow, In)
 
+        # This should be a no-op! That requires hooking ``__setattr__``.
+        flipped(ifsub).a = flipped(ifsub).a
+        self.assertEqual(ifsub.a.signature.members["f"].flow, In)
+
 
 class ConnectTestCase(unittest.TestCase):
     def test_arg_handles_and_signature_attr(self):
@@ -801,9 +965,9 @@ class ConnectTestCase(unittest.TestCase):
         m = Module()
         connect(m,
                 p=NS(signature=Signature({"a": Out(Signature({"f": Out(1)}))}),
-                     a=NS(f=Signal(name='p__a'))),
+                     a=NS(signature=Signature({"f": Out(1)}), f=Signal(name='p__a'))),
                 q=NS(signature=Signature({"a": In(Signature({"f": Out(1)}))}),
-                     a=NS(f=Signal(name='q__a'))))
+                     a=NS(signature=Signature({"f": Out(1)}).flip(), f=Signal(name='q__a'))))
         self.assertEqual([repr(stmt) for stmt in m._statements], [
             '(eq (sig q__a) (sig p__a))'
         ])