Export Unit's constructor for use in pattern matches.

[lunaris]

I think it's helpful to be able to pattern match on values of type Unit (even if we always know what they are) in order to guide type inference in cases such as:

psci> :t \x y z (Unit {}) -> ...
forall t1 t2 t3. t1 -> t2 -> t3 -> Unit -> ...

The alternative is having to write (_ :: Unit). This is admittedly only slightly longer, but I don't find the use of a type annotation very natural here. I'd be interested to hear your thoughts.

[lunaris]

Seems my build is failing, embarrassingly, with:

src/Prelude.js: line 49, col 5, Incompatible values for the 'strict' and 'globalstrict' linting options. (21% scanned).

I don't believe this is related to my patch though?

[michaelficarra]

See #56. Amending and pushing should trigger Travis again.

[hdgarrood]

I think I've seen lots of FFI code that simply returns undefined for Unit values. Could pattern matching like this cause problems in these cases?

[michaelficarra]

I think the pattern match would actually be written

\x y z Unit -> ...

which shouldn't cause problems based on unit's value.

[lunaris]

That doesn't work though right? Since Unit is a newtype over {}? (Not sure why it's not data Unit = Unit; perhaps efficiency or the like, but that's how I understand it currently.

[hdgarrood]

Yeah, @lunaris is right; Unit is currently newtype Unit = Unit {}. If we are going to export the constructor, we might also consider changing the declaration to data Unit = Unit, because that would be less noisy, and you then wouldn't have to choose whether to use {} or _ in patterns for the argument.

[lunaris]

Ultimately this was my hope all along, but I figured this was the smallest possible PR and didn't want to talk about the bikeshed's colour at the same time as proposing we leave the door open 😄

[garyb]

Changing the declaration to that probably will stop pattern matching from working for undefined / {} values, as we do an instanceof check for constructors. That doesn't happen for newtypes as they have no runtime representation... and I think that pattern matching on the current Unit is only safe when optimizations are enabled in the compiler.

☔

[hdgarrood]

So if we say that we leave it as it is, and say that you have to use (_ :: Unit) in patterns, does that work? I actually think it might be a good thing that the :: is there - if I see a :: inside a pattern, that makes me think that this type signature is probably a hint for the compiler.

[paf31]

Type annotations won't affect code gen of binders. I think the point is that if you match the Unit constructor on x after something like x <- log "foo", it might fail since the FFI definition doesn't technically return a Unit, it returns undefined, which we have traditionally glossed over.

[paf31]

I think the way to remedy this is to just say Unit has an abstract representation, and you can't match on it. I would probably change it to a foreign import data declaration. I think typed binders are the correct solution then, for the original problem.

[lunaris]

I'm guessing this PR should be closed then? Though, if I understand things correctly, given you're striving not to have a "built-in" Prelude, does it need to be documented that any Prelude users provide themselves has a constraint that Unit not be pattern matched lest undefined be encountered?

[hdgarrood]

I don't think we would need that, because all the Unit values that I've encountered in the wild use Prelude.Unit, which wouldn't unify with a Unit from an alternative prelude. If we were going to write a guide for creating alternative preludes, then perhaps that would be a good place to write such a thing (although I'm not sure we have a pressing need for one of those right now). Anyway, I don't think the intention of separating Prelude from the compiler was to allow a proliferation of alternative preludes (not that that would be a bad thing), but rather just so that the compiler and the prelude could be changed and versioned independently.

[lunaris]

My mistake; I only got this impression because the Prelude isn't imported by default. Shall I close this then?