Skip to content

Move solved type classes into Prim modules & bump test deps#3312

Merged
LiamGoodacre merged 12 commits into
purescript:masterfrom
LiamGoodacre:feature/expand-prim
Apr 24, 2018
Merged

Move solved type classes into Prim modules & bump test deps#3312
LiamGoodacre merged 12 commits into
purescript:masterfrom
LiamGoodacre:feature/expand-prim

Conversation

@LiamGoodacre

@LiamGoodacre LiamGoodacre commented Apr 19, 2018

Copy link
Copy Markdown
Member

WIP
Can split this into two PRs if that makes more sense but things aren't going to work until the test deps are consistent.
Next I'll update our tests to switch from Eff to Effect.
Resolves #3179

@garyb

garyb commented Apr 19, 2018

Copy link
Copy Markdown
Member

I think doing it this way is fine 👍 I have a branch with updated dependencies too, but until the Prim/Prelude record instances are in that doesn't work anyway.

@kritzcreek

Copy link
Copy Markdown
Member

I read the code that's there so far and didn't spot any obvious oversights 👍

Comment thread src/Language/PureScript/Docs/Prim.hs Outdated
, "[the Custom Type Errors guide](https://github.com/purescript/documentation/blob/master/guides/Custom-Type-Errors.md)."
kindOrdering :: Declaration
kindOrdering = primKindOf (P.primSubName "Ordering") "Ordering" $ T.unlines
[ "The `Ordering` kind represents the three possibilites of comparing twos"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oops twos -> two

Comment thread src/Language/PureScript/Environment.hs Outdated
@@ -349,9 +365,9 @@ primKinds =
primTypes :: M.Map (Qualified (ProperName 'TypeName)) (Kind, TypeKind)
primTypes =
M.fromList

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Should have Partial : kindConstraint for consistency.

@LiamGoodacre

Copy link
Copy Markdown
Member Author

Updated a whole bunch of tests. 😓

Getting cannot find Prim.Symbol and others like that, so I've probably missed updating something.
Will try investigating tomorrow evening.

Updated purescript-typelevel-prelude to reexport stuff from Prim submodules - but that needs more work and somewhat integrating with prelude.

@LiamGoodacre

Copy link
Copy Markdown
Member Author

Also haven't fixed tests that involve ST/Ref as I'm not sure what we're doing there.

@garyb

garyb commented Apr 20, 2018

Copy link
Copy Markdown
Member

In what sense? The ST & Ref libraries have been updated (ST being its own monad now).

@LiamGoodacre

Copy link
Copy Markdown
Member Author

Right! I'm being a moron 😸

@LiamGoodacre

Copy link
Copy Markdown
Member Author

Think there was one that used runPure, so that will need rethinking.

@garyb

garyb commented Apr 20, 2018

Copy link
Copy Markdown
Member

I guess that'll become unsafePerformEffect? 😱

Unless it's ST, in which case there is an ST.run, which is like runPure after runST / pureST.

@LiamGoodacre

LiamGoodacre commented Apr 20, 2018

Copy link
Copy Markdown
Member Author

So there's this test: examples/failing/Eff.purs

-- @shouldFailWith TypesDoNotUnify
module Main where

import Prelude
import Control.Monad.Eff
import Control.Monad.ST
import Control.Monad.Eff.Console

test = pureST (do
         ref <- newSTRef 0
         log "ST"
         modifySTRef ref $ \n -> n + 1
         readSTRef ref)

Which I believe was testing that you can't log in a pure ST.

Thoughts on what this should change to?

@LiamGoodacre

Copy link
Copy Markdown
Member Author

Similarly the passing one: examples/passing/Eff.purs

module Main where

import Prelude
import Control.Monad.Eff
import Control.Monad.ST
import Control.Monad.Eff.Console (log, logShow)

test1 = do
  log "Line 1"
  log "Line 2"

test2 = runPure (runST (do
          ref <- newSTRef 0.0
          _ <- modifySTRef ref $ \n -> n + 1.0
          readSTRef ref))

test3 = pureST (do
          ref <- newSTRef 0.0
          _ <- modifySTRef ref $ \n -> n + 1.0
          readSTRef ref)

main = do
  test1
  logShow test2
  logShow test3
  log "Done"

Wondering if these tests should just be deleted, given their focus is actually on Eff.

@garyb

garyb commented Apr 20, 2018

Copy link
Copy Markdown
Member

Wondering if these tests should just be deleted, given their focus is actually on Eff.

Yeah that sounds reasonable to me. We probably have quite a lot of redundant tests, but no harm having them unless like this they don't make sense anymore.

@LiamGoodacre

Copy link
Copy Markdown
Member Author

Thanks @kritzcreek for finishing off fixing up the tests!

@garyb garyb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🎉

@LiamGoodacre LiamGoodacre merged commit 92d7b6b into purescript:master Apr 24, 2018
@LiamGoodacre LiamGoodacre deleted the feature/expand-prim branch May 9, 2021 09:11
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.

3 participants