ValueCastable and ShapeCastable should override __instancecheck__

[whitequark]

Right now, isinstance(unsigned(1), ShapeCastable) or isinstance(Cat(), ValueCastable) or isinstance(1, ValueCastable) are all false, which is wrong.

[whitequark]

This requires defining a metaclass for ShapeCastable and ValueCastable, which can potentially have unexpected consequences. Needs investigating.

[whitequark]

On reflection, this may not be such a good idea for two reasons:

1. More metaclasses. Metaclasses should be avoided as much as possible; but more than this abstract principle, in this case it may actually cause issues, since our own shape-castable objects themselves have custom metaclasses, and this can get really messy.
2. Although it is tempting to say that if x is shape-castable, then isinstance(x, ShapeCastable) == True and correspondingly issubclass(type(x), ShapeCastable) == True, this violates two principles:
   a. If isinstance(x, ShapeCastable) == True when type(x) == int, it normally implies that x.as_shape() should succeed. But this is not the case for integers.
   b. Given class X(amaranth.lib.enum.Enum): pass, issubclass(X, ShapeCastable) == True will always be the case, but the enum X is not actually shape-castable since Shape.cast(X) will raise TypeError.

So maybe we should document that the only way to test if an object is shape-castable is through Shape.cast, and ShapeCastable is a protocol through which only custom shape-castable objects are defined.

[whitequark]

Looking deeper into it, ValueCastable is a protocol, and Python already has support for protocols such as Iterable. Existing protocols are implemented using abc.ABCMeta.__subclasshook__, which is actually very straightforward to use:

import abc


class ValueCastable(metaclass=abc.ABCMeta):
    @classmethod
    def __subclasshook__(cls, other):
        if other in (int,):
            return True
        return NotImplemented

print(issubclass(int, ValueCastable)) # => True
print(isinstance(1, ValueCastable)) # => True

This handles objection (1); it is clearly fine to use the Python mechanism for defining protocols.

Objection (2a) can be handled by saying that the only thing isinstance(x, ShapeCastable) implies is that Shape.cast(x) will succeed. This corresponds to the notion of shape-castable in the language guide, which is good, because having ShapeCastable be different from shape-castable is quite absurd. The code within Amaranth itself that distinguishes between ShapeCastable, Shape, and other shape-castable objects can use x.__class__.__mro__ to detect if ShapeCastable is an ancestor, instead of instanceof. The code outside of Amaranth should only call Shape.cast(x) on arbitrary shape-castable objects anyway, since nothing else is guaranteed about them.

Objection (2b) is handled by only returning True from __subclasshook__ if all of the enumeration members are constant-castable.

So I think we actually should do this change.

[whitequark]

We've discussed this issue on the 2023-05-15 weekly meeting. The disposition was to merge the PR implementing the proposed changes.

[whitequark]

RFC 35 will resolve this issue.

[whitequark]

Resolved by #983.