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

Update for names in new prelude #1963

Closed
wants to merge 2 commits into from
Closed

Update for names in new prelude #1963

wants to merge 2 commits into from

Conversation

garyb
Copy link
Member

@garyb garyb commented Mar 17, 2016

Resolves #1953

This has changes for deriving, but also further updates to the inliner to reflect the new (as of yet unmerged) boolean and number class hierarchies.

@garyb garyb force-pushed the 1953-new-prelude-names branch from 85de71a to b70569e Compare March 17, 2016 12:18
@hdgarrood
Copy link
Contributor

s/euclidian/euclidean/ ?

@garyb
Copy link
Member Author

garyb commented Mar 17, 2016

Uh, whoops.

@texastoland
Copy link

new (as of yet unmerged) boolean and number class hierarchies.

@garyb Where my I read about this?

@hdgarrood
Copy link
Contributor

@AppShipIt purescript/purescript-prelude#61

@garyb
Copy link
Member Author

garyb commented Mar 17, 2016

@AppShipIt

@texastoland
Copy link

Thanks both I don't know why I didn't consider checking PRs there!!

@paf31
Copy link
Contributor

paf31 commented Mar 17, 2016

Looks good, thanks! We also need to update $PSCI.Support for changes in -console.

@garyb
Copy link
Member Author

garyb commented Mar 17, 2016

Ah good point. I haven't checked what the error in the build is here yet either, will do both things later on today.

@garyb garyb self-assigned this Mar 17, 2016
@garyb garyb force-pushed the 1953-new-prelude-names branch 2 times, most recently from 2e49b90 to 7fe6f06 Compare March 20, 2016 16:30
@garyb
Copy link
Member Author

garyb commented Mar 20, 2016

Ok, these changes are done now. The tests now use the new Prelude also, so the deriving tests work, and I had to update some stuff that has changed (no return, (++), print -> logShow, etc).

The psci tests fail, but I'm not sure if it's the tests that are the problem, or if it is actually psci:

### Failure in: completionTests:18
expected: [":type unit"]
 but got: []
### Failure in: completionTests:19
expected: [":type EQ"]
 but got: []
### Failure in: completionTests:20
expected: [":kind Control.Monad.Eff.Pure"]
 but got: []
### Failure in: completionTests:21
expected: [":kind Ordering"]
 but got: []
### Failure in: completionTests:31
expected: ["unit"]
 but got: []
### Failure in: completionTests:33
expected: ["GT"]
 but got: []
Cases: 37  Tried: 37  Errors: 0  Failures: 6

Some of these failures seem like they always should have been failures, since they're trying to autocomplete things that have not been imported (:type unit, :type EQ)? But then some of the others seem like they definitely should work (:kind Control.Monad.Eff.Pure) so I don't really know. Maybe it's a mixture of test and psci problems?

@garyb garyb removed their assignment Mar 20, 2016
@hdgarrood
Copy link
Contributor

I thought those tests should have imported Prelude open already? So I'd expect them all to work.

@garyb
Copy link
Member Author

garyb commented Mar 20, 2016

Maybe I'm misreading, but it looks like the Prelude import:

, ("import Prelude ", [])

Is after the :type tests that assume it has been imported:

, (":type uni", [":type unit"])

@garyb
Copy link
Member Author

garyb commented Mar 20, 2016

Ah, unless that's just the completions and the import is already done.

In that case, psci doesn't understand re-exports?

@paf31
Copy link
Contributor

paf31 commented Mar 20, 2016

So if you do import Prelude in PSCi now, you don't see anything? 😟

@garyb
Copy link
Member Author

garyb commented Mar 20, 2016

I hadn't checked actually. It would appear that psci does indeed not understand re-exports - > :t unit works, but it won't autocomplete it.

@garyb
Copy link
Member Author

garyb commented Mar 20, 2016

:browse Prelude is empty too, I assume it should print "everything"?

@paf31
Copy link
Contributor

paf31 commented Mar 20, 2016

Should we release 0.8.3 without this? If we wait, we'll have to also wait for a decent subset of the existing libraries to be updated too.

@garyb
Copy link
Member Author

garyb commented Mar 20, 2016

What do you mean? We can't really update the libraries until this is in, as neither inlining nor deriving will work for the new prelude, and the old prelude cannot be made warning-free for 0.8 without breaking changes.

@garyb
Copy link
Member Author

garyb commented Mar 20, 2016

Oh, sorry, I just parsed the sentence the wrong way I think.

I'm not sure how hard this is to fix, I assume it's just reading the exports from the module directly rather than using the desugared scope - but we did some work to make that useful elsewhere, as psc-publish uses it too now, so maybe the fix won't be too tricky?

@paf31
Copy link
Contributor

paf31 commented Mar 22, 2016

I'm just thinking there is enough to make a 0.8.3 release now, and we could make a 0.8.4 release whenever the core libraries come out of RC mode.

@garyb garyb force-pushed the 1953-new-prelude-names branch from 7fe6f06 to c389de9 Compare March 25, 2016 15:47
@garyb garyb force-pushed the 1953-new-prelude-names branch from c389de9 to 27c99c6 Compare April 3, 2016 11:21
@garyb
Copy link
Member Author

garyb commented Apr 3, 2016

Hmm, we have a bit of an issue here actually... Deriving (generic or ord/eq) will no longer work with the pre-1.0 libraries if we release this now. I guess that means we'll have to release this as 0.9.

Perhaps we could do that soon(ish) and have a truncated 0.8 cycle? It'd be good to get at least 0.8.4 out with the non-broken psc-bundle, etc. before 0.9, but maybe that should be the last one.

@paf31
Copy link
Contributor

paf31 commented Apr 3, 2016

That makes sense. I'd like to see if I can fix up let generalization before 0.8.4, since it's related to a potentially rather bad soundness bug in 0.8.3. Also, if we're releasing 0.9.0, we should get busy breaking all the stuff we said we were going to break 😄

@texastoland
Copy link

Agreed on 0.9.0 if it's okay because mine are breaking too.

@paf31 paf31 modified the milestones: 0.9.0, 0.10.0 Apr 5, 2016
@paf31 paf31 mentioned this pull request Apr 13, 2016
15 tasks
@paf31 paf31 added this to the 0.9.0 milestone Apr 24, 2016
@garyb garyb closed this Apr 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants