Skip to content

Add type refinement for abstract type bindings #4688

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 5 commits into from
Closed

Add type refinement for abstract type bindings #4688

wants to merge 5 commits into from

Conversation

nicolasstucki
Copy link
Contributor

No description provided.

@nicolasstucki nicolasstucki self-assigned this Jun 19, 2018
@smarter
Copy link
Member

smarter commented Jun 19, 2018

If I can write any type on the rhs of Refined, isn't this completely unsound?

@nicolasstucki
Copy link
Contributor Author

There are still some constraints that must be added.

@nicolasstucki nicolasstucki force-pushed the add-type-refinement-for-abstract-type-bindings branch from 7e2a1ad to 4c28eb1 Compare June 21, 2018 11:20
@Blaisorblade
Copy link
Contributor

But do we have a way to add constraints that makes this sound? I thought the plan was to warn the user that they use an escape hatch, just like asInstanceOf. Maybe call the member UnsafeRefined to make this more visible?

@nicolasstucki nicolasstucki force-pushed the add-type-refinement-for-abstract-type-bindings branch 2 times, most recently from a0aeea7 to 1426e07 Compare June 21, 2018 13:35
@nicolasstucki nicolasstucki force-pushed the add-type-refinement-for-abstract-type-bindings branch 3 times, most recently from 006ee48 to 1582173 Compare July 26, 2018 14:27
Blaisorblade
Blaisorblade previously requested changes Aug 1, 2018
View reviewed changes
Copy link
Contributor

@Blaisorblade Blaisorblade left a comment

Choose a reason for hiding this comment

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

We currently prefer a different approach to the problem — we lose guarantees on the values, but we get type soundness back, and @nicolasstucki's on it. Up to him to close or update this PR.

@nicolasstucki
Copy link
Contributor Author

@Blaisorblade the current implementation in this PR is a partial implementation of the new scheme.

@nicolasstucki nicolasstucki force-pushed the add-type-refinement-for-abstract-type-bindings branch from 1582173 to 43835b3 Compare August 2, 2018 06:13
@nicolasstucki nicolasstucki force-pushed the add-type-refinement-for-abstract-type-bindings branch 3 times, most recently from f909b8e to cd8fd76 Compare August 13, 2018 14:25
@nicolasstucki
Copy link
Contributor Author

The unboxed result of RefinedScrutinee has the expected performance. Here is a small experiment https://gist.github.com/nicolasstucki/96347efecaa5f6fb71d7146635c079c3

@Blaisorblade
Copy link
Contributor

FWIW: last I tried to write value class decorators with a single member, I ran into #4583, which seems hard to avoid.

@nicolasstucki nicolasstucki force-pushed the add-type-refinement-for-abstract-type-bindings branch from dfb8174 to 67bb6e6 Compare October 5, 2018 11:20
@nicolasstucki nicolasstucki mentioned this pull request Nov 22, 2018
Closed
20 tasks
@nicolasstucki
Copy link
Contributor Author

Will reopen it when I'll have time to work on it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Blaisorblade Blaisorblade Blaisorblade left review comments

Assignees

@nicolasstucki nicolasstucki

Labels
stat:revisit stat:wip
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nicolasstucki @smarter @Blaisorblade