Skip to content

Fix type inference by exposing classless API #53

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

Merged
merged 4 commits into from
Jul 1, 2023

Conversation

JordanMartinez
Copy link
Contributor

Description of the change

Previously, compiler would fail to infer the type of Buffer.create 1 as Effect Buffer
because the Buffer API was exposed only via the multiparameter typeclass MonadBuffer.
Due to the functional dependency between the two parameters, the monadic type cannot be inferred
until the buffer type is known (either Buffer or STBuffer).:

import Node.Buffer as Buffer

-- Example 1
main :: Effect Unit
main = do
  x <- Buffer.create 1 -- compiler: is this `Int -> Effect Buffer` or `Int -> ST h (STBuffer h)?
  pure unit

The workaround was to add a type annotation, indicating the x is a Buffer:

import Node.Buffer as Buffer

-- Example 2
main :: Effect Unit
main = do
  x :: Buffer <- Buffer.create 1 -- compiler: Oh! It's `Int -> Effect Buffer`
  pure unit

This change does not break anyone's code if one was using a create (or another such typeclass member)
to get Int -> Effect Buffer. Rather, such users can now drop the :: Buffer annotation
(i.e. Example 1 above now compiles).

If one was using create to get forall m buf. MonadBuffer buf m => Int -> m buf,
then one will need to update their imports:

-import Node.Buffer (class MonadBuffer)
+import Node.Buffer.Class (class MonadBuffer)

Checklist:

  • Added the change to the changelog's "Unreleased" section with a reference to this PR (e.g. "- Made a change (#0000)")
  • Linked any existing issues or proposals that this pull request should close
  • Updated or added relevant documentation
  • Added a test for the contribution (if applicable)

@JordanMartinez
Copy link
Contributor Author

@thomashoneyman 🏓

unsafeFreeze ∷ ∀ (h ∷ Region). STBuffer h → ST h ImmutableBuffer
unsafeFreeze = Internal.unsafeFreeze

thaw ∷ ∀ (h ∷ Region). ImmutableBuffer → ST h (STBuffer h)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the Unicode?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than manually type out each signature, I used the IDE to supply it. But yeah, I forgot to remove this before opening the PR. Latest commit removes the unicode.

@JordanMartinez JordanMartinez merged commit f481921 into master Jul 1, 2023
@JordanMartinez JordanMartinez deleted the expose-classless-API branch July 1, 2023 14:01
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