Can I add typehints?

[RobertBaruch]

What are your thoughts about adding PEP 484 / PEP 526 type hints to the amaranth.* code? I know that many functions are documented in docstrings, but this lets static analyzers have a go at validating user-written code. If you're game, I'd like to try.

[RobertBaruch]

Example. mypy is unhappy about Shape.cast in ast.py:

            elif isinstance(obj, ShapeCastable):
                new_obj = obj.as_shape()

ShapeCastable has no as_shape method.

If ShapeCastable were implemented like this:

class ShapeCastable(ABC):
  @abstractmethod
  def as_shape() -> Shape:
    ...

I think static analysis would pass (and ShapeCastable would look more Pythonic?)

[RobertBaruch]

Another case where static analysis gets mad: Value.cast (according to the code) takes a Value, int, Enum, or ValueCastable.

The Slice constructor takes a value parameter, which I'm trying to find the type of. The code first does

n = len(value)

Then later on does

self.value = Value.cast(value)

The static analyzer gets mad because you can't take the len of an int or a ValueCastable.

[whitequark]

One particular issue is that currently, there is no good way to add typing to Amaranth values, modules, interfaces directly. So far my position was that type hints would be a nice to have, but right now it's not clear that benefits outweigh the drawbacks.

[RobertBaruch]

I'm not sure I understand? I'm just talking about typehinting the core code, like this:

ValueCastableT = Union["Value", int, Enum, "ValueCastable"]

class Value(metaclass=ABCMeta):
    src_loc: Optional[Tuple[str, int]] = None

    @staticmethod
    def cast(obj: ValueCastableT) -> Union["Value", "Const"]:
        """Converts ``obj`` to an Amaranth value.

This way, if you're using an IDE that supports static analysis (e.g. vscode), it will immediately complain when you try to write code like Value.cast("aaaaa") instead of having to wait until runtime to raise an exception.

Another benefit is static analysis of the core code itself. For example, in Signal.like, we have the docs saying that other is a Value. However, the code says:

        if name is not None:
            new_name = str(name)
        elif name_suffix is not None:
            new_name = other.name + str(name_suffix)

But Value does not have a name attribute.

[whitequark]

The thing is that people will want to do something like i_data: In[Signal[16]] which you can sort of do, but then it becomes an i_data: In[Signal[TypeVar("width")]] which is much less feasible. And I'm still not sure what to do about this.

(Otherwise you can't make e.g. FIFOInterface a type.)

[RobertBaruch]

Oh, I wasn't talking about going all the way like that. i_data: Signal is good enough. As for FIFOInterface... it's a type. Maybe not parameterized, but it's a type. My proposal is just to annotate the internals, for example:

class FIFOInterface:
  def __init__(self, *, width: int, depth: int, fwft: bool):
    ...

So that if I try this:

  w = Const(3)
  d = Const(5)
  x = SyncFIFO(w, d)

The IDE will immediately complain, rather than having to wait until runtime to get an exception.

[tilk]

I currently use type stubs + Pyright in my (unfortunately currently private) project. I can share them if you're interested. Unfortunately, I had to use recursive type definitions, which are unsupported by mypy, so that things like record layouts can be typechecked.

Off-topic remark: I'm a huge fan of #693, because Amaranth records suck badly.

[implr]

Another +1 - I actually considered adding types to Glasgow, then realized it makes no sense without Amaranth first.

I think #725 (comment) is the way to go - start with simple types with liberal use of Any and no generics. That would already make mypy happy in projects that import amaranth, more fancy stuff can be added later.

[whitequark]

@implr One consideration is that both RFC 1 and RFC 2 put non-type things into the annotation syntactic position.

I actively want to use both in Glasgow, too.