m-labs / nmigen

A refreshed Python toolbox for building complex digital hardware. See https://gitlab.com/nmigen/nmigen
https://nmigen.org
Other
652 stars 55 forks source link

Signal decoder inference for Enums doesn't work for Layout members #333

Open shawnanastasio opened 4 years ago

shawnanastasio commented 4 years ago

When creating a signal with shape set to an Enum-derived type, the decoder function is automatically inferred to a built-in function that turns the Enum value and name into a nice human-readable string.

When defining a layout with an field's shape set to an Enum-derived type though, the decoder inference doesn't happen. As far as I can see, this is because the Layout constructor will cast all fields' shapes with Shape.cast, resulting in the original Enum type being lost.

https://github.com/m-labs/nmigen/blob/8f5a253b22cd4ebcd56304a3662f4c70e3b34ed5/nmigen/hdl/rec.py#L48-L50

This results in an actual Shape object being passed to the Signal constructor, and thus the built in Enum decoder can't be used.

I can think of a few ways to fix this, but wanted to hear the maintainers' thoughts to see if they have a preferred way of going about it.

My first idea is to not cast shapes in the Layout constructor, but instead check whether the user-provided type is a valid shape, probably by calling Shape.cast, discarding the value, and checking for an exception.

The other idea is to add a field to Shape which stores the original type that a given object was casted from. This would also require some extra logic in the Signal constructor to not only check the shape's type when checking for default decoders, but also checking the original type of casted Shape objects.

Curious to hear any thoughts on this.