Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More polymorphic type for Array.alloc and HashMap.empty #410

Closed
wants to merge 2 commits into from

Conversation

andreasabel
Copy link
Contributor

Replacing response type Ur b by simply b also type checks and enables uses like

let (i, _) = HashMap.empty 100 (\ env -> runState (eval e) env)

(see issue #404). Without this PR, the respective code would be

let Ur i = HashMap.empty 100 (\ env -> move (fst (runState (eval e) env)))

with fst :: Consumable b => (a, b) -> a.
The old typing of empty forces the insertion of move which can only happen after consuming the env via fst (because HashMap does not implement Consumable).
The new code seem to be better, allowing env to be garbage-collected unforced once we want to dispose of it.

Caveat: I am relying on the linear type checker (which might be in alpha state, dunno), so there might be reasons unknown to me why this generalization is invalid. Test suite passes, though.

Also in this PR: Restrict tasty-hedgehog < 1.2 to make cabal build succeed.

Replacing response type `Ur b` by simply `b` also checks and enables
uses like
```haskell
let (i, _) = HashMap.empty 100 (\ env -> runState (eval e) env)
```
(see issue tweag#404).

Caveat: I am relying on the linea type checker (which might be in
alpha state, dunno), so there might be reasons unknown to me why this
generalization is invalid.  Test suite passes, though.
@andreasabel andreasabel changed the title More polymorphic type for Array.allow and HashMap.empty More polymorphic type for Array.alloc and HashMap.empty Apr 12, 2022
@aspiwack
Copy link
Member

Unfortunately, this is very unsafe. You could do

bad :: Int -> a -> Array a
bad n x = Array.alloc n x id

@andreasabel
Copy link
Contributor Author

Ok, so the type checker isn't sound yet.
Was unlikely anyway that the simplification I suggested would have been overlooked...

@aspiwack
Copy link
Member

The typechecker is sound. But the implementation of Array uses unsafePerformIO, which isn't. The careful choice of API is what makes the use of unsafePerformIO ok. Just like any abstract type really.

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

Successfully merging this pull request may close these issues.

2 participants