Bug 736

"Note If the right operand’s type is a subclass of the left operand’s type and that subclass provides a different implementation of the reflected method for the operation, this method will be called before the left operand’s non-reflected method. This behavior allows subclasses to override their ancestors’ operations."

ngghhh.... yyeah that makes sense.  i always wondered how that worked.
does it apply to Cat() though? i really don't know.

i made an arbitrary choice: however... whatever the choice is,
they are all going to be "bad"

s = SimdSignal(...)
Cat(Const(...), s)

or

Cat(Const(...), s)

these are both "valid", so which one should be acceptable?  which one
prohibited, the other allowed?

i believe my thinking here was (without documenting it), that *all*
of Cat()'s arguments are expected to be SimdSignals.  or, at least,
if the first one is, everything else follows.

a more advanced version would detect the type of non-SimdSignal
instances within the list and upcast them.

however what i didn't want to do was have that logic "pollute"
nmigen.ast.Cat with things that are SimdSignal-specific.

it's a tricky one, i'm not sure what the best solution is here,
and was going for "wait and see what emerges down the line" when
we have a lot more info about how this works in practice.

one obvious workaround: a SimdScope.Cat().  used as:

   with SimdScope(....) as s:
      .... s.Cat(...)

however it is... hmmm... it would be the first major ast Operator
that fell into this usage pattern, it has implications.

really don't know, here.

as in, rather than:

s = SimdSignal(...)
Cat(Const(...), s)

it would be e.g.:
s = SimdSignal(...)
c = SimdSignal(...)
comb += c.eq(Const(...))
Cat(c, s)

which "avoids" the problem that i think might turn out
to be unique to Cat()

(In reply to Luke Kenneth Casson Leighton from comment #2)
> as in, rather than:
> 
> s = SimdSignal(...)
> Cat(Const(...), s)
> 
> it would be e.g.:
> s = SimdSignal(...)
> c = SimdSignal(...)
> comb += c.eq(Const(...))
> Cat(c, s)
> 
> which "avoids" the problem that i think might turn out
> to be unique to Cat()

nope, it also occurs in Mux and Part.

s1 = Signal()
s2 = SimdSignal()
Mux(s1, Const(), s2)
Part(Const(), s2, 3, 5)

For Cat, we look for the most-derived subclass and use that. more when I have time.

mmm... __rCat__ hmmm... makes me slightly nervous that because
of the multiple arguments it doesn't quite fit the pattern created
by python x.__rXXX___ which seems to hsve been designed for 2-arg
operations.

still not a real clue here.

thinking of Cat(a, b, c, d, e, f) -> c.__Cat__((a, b), (d, e, f))

(In reply to Jacob Lifshay from comment #3)
> (In reply to Luke Kenneth Casson Leighton from comment #2)
> > as in, rather than:
> > 
> > s = SimdSignal(...)
> > Cat(Const(...), s)
> > 
> > it would be e.g.:
> > s = SimdSignal(...)
> > c = SimdSignal(...)
> > comb += c.eq(Const(...))
> > Cat(c, s)
> > 
> > which "avoids" the problem that i think might turn out
> > to be unique to Cat()
> 
> nope, it also occurs in Mux and Part.

ok.

> s1 = Signal()
> s2 = SimdSignal()
> Mux(s1, Const(), s2)

yeah this one i went with the rule "selector is the priority".

> Part(Const(), s2, 3, 5)
> 
> For Cat, we look for the most-derived subclass and use that.

ahh that makes sense.  except... oh i know.  a static SimdSignal.Cat.
find the most-derived class then call the static function with the
complete list.

def Cat(*arglist)
   kls = find_most_derived_class(arglist)
   kls.Cat(*arglist)


 
> more when I
> have time.

ok.

these are things i am happy to go with a workaround for the
first version, in order to keep the patch to nmigen to a bare
minimum.

the last thing we need right now is for that 400 line diff to
turn into 4,000.

also, remember, we need to get off critical path so if work
can proceed on the ALUs with workarounds such as converting Consts
to SimdSignal, and not using mixed Signal and SimdSignal, *great*.

the ALU conversion gives us data about what the priorities are:
what matters and what doesn't

pragmatic.

https://www.programiz.com/python-programming/methods/built-in/issubclass

walk the list (or other params), call issubclass.  made slightly more
complex by the fact that ValueCastable is not part of the Value chain,
in fact it throws a royal spanner in the works.  a partial hack may
be possible with: "if isinstance(1st, ValueCastable) and not isinstance(2nd,
ValueCastable) highest = 1st" and proceed from there