clash-lang / clash-compiler

Haskell to VHDL/Verilog/SystemVerilog compiler
https://clash-lang.org/
Other
1.43k stars 151 forks source link

Drop `1 <= n` constraint from `Foldable (Vec n)` and from `Traversable (Vec n)`. #2563

Closed christiaanb closed 1 year ago

christiaanb commented 1 year ago

The Foldable.{foldr1,foldl1,maximum,minimum} functions now throw an error at run-/simulation-time, and also at HDL-generation time, for vectors of length zero.

Also moves clashCompileError from Clash.Explicit.Prelude to Clash.Magic.

Still TODO:

christiaanb commented 1 year ago

It was from a discussion brought on by @gergoerdi on the #clash slack FP channel, so perhaps he can chime in as well.

It has also annoyed me that functions using sum or foldMap or traverse accrue an unnecessary 1 <= n constraint. But the run-time error turning into false in the worst case (Vec 0 Bool for VHDL), or X in the “best” case at HDL generation case has always put my off into making the change in this PR. But now that we have clashCompileError we won’t get surprising HDL (thanks @leonschoorl for the suggestion)

Note that the foldr1 and foldl1 exported by Clash.Prelude are still the total ones, so users would have to go out of their way to get the partial versions from Data.Foldable. That being said, I should probably add total versions of maximum and minimum to this PR and have them exported from Clash.Prelude.

Finally, base-4.18 (bundled with GHC 9.6) does introduce a Data.Foldable1 for non-empty structures. So I could add an instance for that in this PR as well. We will still need the Data.Foldable instance though since it is a super-class constraint for Data.Traversable.

gergoerdi commented 1 year ago

I don't understand why Foldable is a superclass of Foldable1. Doesn't that defeat its purpose..?

I think it makes sense: Foldable1 means that it can be e.g. foldr1'd, i.e. it can be folded when expecting at least one element to be there. Foldable means it can be e.g. foldr'd, i.e. it can be folded without expecting at least on element.

If you can be folded when there are some extra expectations, then you can be folded when there aren't any.

christiaanb commented 1 year ago

@martijnbastiaan maximumBy and minimumBy are not exported by Prelude, so there’s no need to override them in Clash.Prelude. The non-empty Vec head and last versions were always exported from Clash.Prelude.

martijnbastiaan commented 1 year ago

Right @gergoerdi. I was thinking along the lines of "now there's no way you can avoid implementing partial functions", but that's not really relevant.