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

Realizability: extra tests and code fix from #5558 #5726

Merged
merged 4 commits into from
Jan 18, 2019

Conversation

Blaisorblade
Copy link
Contributor

@Blaisorblade Blaisorblade commented Jan 16, 2019

Add a test for realizability, and one to check that b3dd07c in #5568 isn't too liberal.

Also add a behavior improvement from #5558. Follow-up cleanups are in #5730, but I'd like to first get a review from others to ensure this is clear enough to people.

@Blaisorblade Blaisorblade added the fasttrack Simple fix. Reviewer should merge or apply additional changes directly. label Jan 16, 2019
Such a "variable" can only have one value, so this is okay.

This goes back to 9125f58.
I wrote this because I feared (incorrectly) exponential slowdowns in
realizability checking for this code. But debugging suggests that the
complexity of realizability checking is constant in the size of these
expressions (even after I disable caching of `Stable`).

Beware 1: these expressions almost smash the stack by sheer size.

Beware 2: this fails with `-Yno-deep-subtypes`, but simply because the checking
heuristics assumes people don't try to do this.

Beware 3: this also depends on unstable members with singletons types being
stable.
Stack consumption grew apparently.
@Blaisorblade Blaisorblade force-pushed the realizability-stray-tests branch from f0551ac to 0ee31f4 Compare January 17, 2019 00:12
@Blaisorblade Blaisorblade changed the title Realizability: stray extra tests from #5558 Realizability: extra tests and code fix from #5558 Jan 17, 2019
@Blaisorblade Blaisorblade requested review from odersky and removed request for nicolasstucki January 17, 2019 00:13
@Blaisorblade Blaisorblade removed exp:novice fasttrack Simple fix. Reviewer should merge or apply additional changes directly. labels Jan 17, 2019
@Blaisorblade Blaisorblade assigned odersky and unassigned Blaisorblade Jan 17, 2019
Copy link
Contributor

@odersky odersky left a comment

Choose a reason for hiding this comment

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

LGTM

@Blaisorblade Blaisorblade merged commit b5331a3 into scala:master Jan 18, 2019
@Blaisorblade Blaisorblade deleted the realizability-stray-tests branch January 18, 2019 16:19
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.

3 participants