Skip to content
This repository has been archived by the owner on Sep 7, 2018. It is now read-only.

Remove uses of data-default #122

Open
thoughtpolice opened this issue Sep 3, 2017 · 1 comment
Open

Remove uses of data-default #122

thoughtpolice opened this issue Sep 3, 2017 · 1 comment

Comments

@thoughtpolice
Copy link
Contributor

While looking at the type signature of window, it uses data-default in order to provide default instances for various types.

However, there are no non Default versions of these functions, which I find extremely odd! I was going to try adding them. But I considered "when you have a non-Default version that takes an initial value, why even bother with a default version when you can just say window clk rst def sig anyway?

But thinking about that, I thought "why even say def when normally I would say like, 0 anyway?* Thinking more, I decided to start looking around, and there aren't really that many uses of default around the place? There are mostly some basic instances for Vec and other things like Index, but almost all of these uses are trivial, and I'm not sure they buy us anything anyway. Actual clash code doesn't really use it -- and in the case it does, like window, it's actually kind of annoying!

For reference, at $WORK right now, we have thousands of lines of Clash code and supporting Haskell code and we don't even use default anywhere in the actual Clash code, and only use it once in supporting Haskell code. We would not miss it.

There are plenty of reasons to dislike default: mostly, it's a kind of meaningless class that's hard to control, and has no laws. Personally I have always found something like Monoid at least to be much more useful. In the above example, for instance, it's limiting window, and adding other versions of window is just pointless duplication. Adding newtypes is a bit difficult (due to the way the compiler works regarding coercions right now, as @christiaanb and I have talked about) and awkward way of working around this just for brevity in 1% of scenarios, and it's not like you save that much anyway.

Plus, for things like Index or Unsigned -- they already have Num instances anyway with fromInteger, so writing '0' is shorter than def anyway :)

(This also reduces the dependency footprint of clash-prelude somewhat which is always great as well.)

@Jhana1
Copy link
Contributor

Jhana1 commented Sep 10, 2017

I use default quite a bit in my code. I'd be disappointed to see it go, but I'd probably just rewrite them myself.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants