search

clash-lang / clash-protocols

a battery-included library for dataflow protocols
Other
19 stars 7 forks source link

Decide what to do with Ack #99

Open t-wallet opened 2 months ago

t-wallet commented 2 months ago

Similarly to https://github.com/clash-lang/clash-protocols/issues/87, we need to make a decision on what to do with the Ack type. There are three options:

  1. Keep it as is.
  2. Change it to Data Ack (fwdType :: Type) = Ack Bool. This ensures that mismatched Bwd and Fwd channels are not connected on the type level.
  3. Remove it entirely and just make the Bwd of Df a Bool.

I'd like to hear your opinions on this!

t-wallet commented 2 weeks ago

My thoughts on this:

I'm not a big fan of Ack. You always deconstruct or coerce it to a boolean anyway, so the newtype provides little type safety for the extra layer of indirection. Its name can also be confusing to beginners, as acking NoData is perfectly fine. It's more of a ready signal.

Option 2 only seems useful to me if you don't use circuit-notation.

So I would prefer removing it altogether. Unfortunately this is currently not feasible due to circuit-notation requiring a Default instance.

martijnbastiaan commented 2 weeks ago

So I would prefer removing it altogether. Unfortunately this is currently not feasible due to circuit-notation requiring a Default instance.

Let's fix this. Default is an awful class.

t-wallet commented 2 weeks ago

So I would prefer removing it altogether. Unfortunately this is currently not feasible due to circuit-notation requiring a Default instance.

Let's fix this. Default is an awful class.

It will be fixed when https://github.com/cchalmers/circuit-notation/pull/25 is merged.