Skip to content

Instantiate polymorphic kinds when unwrapping newtypes while solving Coercible constraints#4040

Merged
hdgarrood merged 3 commits intopurescript:masterfrom
kl0tl:instantiate-polykinds-on-newtype-unwrapping
Apr 8, 2021
Merged

Instantiate polymorphic kinds when unwrapping newtypes while solving Coercible constraints#4040
hdgarrood merged 3 commits intopurescript:masterfrom
kl0tl:instantiate-polykinds-on-newtype-unwrapping

Conversation

@kl0tl
Copy link
Copy Markdown
Member

@kl0tl kl0tl commented Apr 3, 2021 •
edited
Loading

Description of the change

@thomashoneyman shared on Slack that the compiler is unable to derive a Newtype instance for newtypes applying a variable to a row:

newtype X r f = X (r ( field :: f Int ))
derive instance newtypeX :: Newtype (X r f) _
Error found:
in module Main
at src/Main.purs:6:1 - 6:46 (line 6, column 1 - line 6, column 46)

  No type class instance was found for
                                              
    Prim.Coerce.Coercible (r1                 
                             ( field :: f2 Int
                             )                
                          )                   
                          (r1                 
                             ( field :: f2 Int
                             )                
                          )                   
                                              
  The instance head contains unknown type variables. Consider adding a type annotation.

while solving type class constraint
                                            
  Prim.Coerce.Coercible (X @t3 r1 f2)       
                        (r1                 
                           ( field :: f2 Int
                           )                
                        )

The issue is easier to understand with an empty row, and compiling with --verbose-errors:

newtype X r = X (r ())
derive instance newtypeX :: Newtype (X r) _
Error found:
in module Main
at src/Main.purs:6:1 - 6:44 (line 6, column 1 - line 6, column 44)

  No type class instance was found for
                                       
    Prim.Coerce.Coercible (r1 (() @k)) 
                          (r1 (() @t0))

  The instance head contains unknown type variables. Consider adding a type annotation.

while solving type class constraint
                                     
  Prim.Coerce.Coercible (X @t0 r1)   
                        (r1 (() @t0))

Now we see the types differ in how their polymorphic kind is instantiated!

The unknown kind applied to the newtype in the original constraint is replaced by a k variable, bound in the (inferred) newtype kind signature. This pull request instantiates those variables when unwrapping newtypes, so that Coercible (X @t0 r1) (r1 (() @t0)) yields Coercible (r1 (() @t0)) (r1 (() @t0)) as expected.

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 JordanMartinez mentioned this pull request Apr 3, 2021
Closed
@JordanMartinez
Copy link
Copy Markdown
Contributor

JordanMartinez commented Apr 3, 2021

Restarting the build.

@JordanMartinez
Copy link
Copy Markdown
Contributor

JordanMartinez commented Apr 4, 2021

CI fails with this issue now:

purescript    > /home/travis/build/purescript/purescript/sdist-test/src/Language/PureScript/TypeChecker/Entailment/Coercible.hs:666:50: error: [-Wname-shadowing, -Werror=name-shadowing]
purescript    >     This binding for ‘k’ shadows the existing binding
purescript    >       bound at src/Language/PureScript/TypeChecker/Entailment/Coercible.hs:664:4
purescript    >     |
purescript    > 666 |       instantiatedKinds = zipWith (\(_, (kv, _)) k -> (kv, k)) kvs ks
purescript    >     |                                                  ^
purescript    >

@kl0tl kl0tl force-pushed the instantiate-polykinds-on-newtype-unwrapping branch from b6731ec to 6520f79 Compare April 4, 2021 13:58
@kl0tl
Copy link
Copy Markdown
Member Author

kl0tl commented Apr 4, 2021

I really should take the habit to compile with stack build --fast --pedantic 🙈

@JordanMartinez
Copy link
Copy Markdown
Contributor

JordanMartinez commented Apr 4, 2021

CI builds now. Is this ready to merge?

Since I'm not familiar with this repo's code base, I don't want to merge something that isn't "obviously" correct (e.g. #4042, #4041). I assume this is safe to do, but I'd rather someone else do it.

@rhendric rhendric mentioned this pull request Apr 4, 2021
Merged
4 tasks
@hdgarrood hdgarrood merged commit c626f32 into purescript:master Apr 8, 2021
@JordanMartinez JordanMartinez mentioned this pull request Jan 18, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rhendric rhendric rhendric approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kl0tl @JordanMartinez @rhendric @hdgarrood