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

Ban unstable variables #4018

Merged
merged 32 commits into from
Mar 8, 2022
Merged

Ban unstable variables #4018

merged 32 commits into from
Mar 8, 2022

Conversation

SeanTAllen
Copy link
Member

@SeanTAllen SeanTAllen commented Feb 16, 2022

This PR introduces checks around variables and other unaliasing points, to reject programs which attempt to create variables whose capabilities are inherently unsound. The checks about compatibility of intersections have been moved to this check after investigation of the origin, as it is not needed for soundness, but this can be reinstated in the original location if helpful for engineering purposes.

Fixes #3931

Some generic code may still produce unsoundness after instantiating, but this change is hopefully a step-forwarding to introducing the necessary constraints.

@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Feb 16, 2022
@SeanTAllen
Copy link
Member Author

SeanTAllen commented Feb 16, 2022

I've opened a new PR for this as ponycheck goes boom with this change.

See .https://ponylang.zulipchat.com/#narrow/stream/190359-ci/topic/Breakage.20against.20main.20failures

@SeanTAllen SeanTAllen removed the request for review from jasoncarr0 February 16, 2022 02:03
@SeanTAllen SeanTAllen added the changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge label Feb 16, 2022
@jasoncarr0
Copy link
Contributor

This last fix avoids the crash, but it doesn't seem to always be the right behavior so I'll need more here. We can compare to Array.init, which requires ephemerality to make iso^^ impossible because of the subtyping. But some generics actually want to have iso^^=iso^, e.g. the issue in itertools. So most likely the consume_type will need to be split in two for the use case of generic reification to have the latter behavior.

Otherwise suppose we have a signature like:
map[B]({(A!): B^}): Iter[B^]
This is reasonable but if we instantiate B with (A iso^ | None) then we can't use a function which returns A iso^ since it will be stripped away and replaced with None.

On the other hand, Array.init requires the behavior that invalid capabilities are removed.

SeanTAllen pushed a commit that referenced this pull request Feb 22, 2022
#4026)

This fixes a small stdlib issue discovered during: #4018

The signatures for map and map_stateful in itertools were overly strict, requiring `B^` instead of just `B`, but only returned `Iter[B]`. Thus it had to be instantiated with `iso^` in some cases, exercising a bug with double ephemerals.

This is not a breaking change.
@SeanTAllen SeanTAllen removed the discuss during sync Should be discussed during an upcoming sync label Feb 22, 2022
@jasoncarr0 jasoncarr0 force-pushed the ban-unstable-variables branch from a86de83 to f3fe9d1 Compare February 27, 2022 02:15
@jasoncarr0
Copy link
Contributor

jasoncarr0 commented Feb 27, 2022

This is good to go as far as I can tell. Review could benefit by making sure there's no case missing from the tests.

@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Feb 27, 2022
src/libponyc/type/alias.c Outdated Show resolved Hide resolved
@SeanTAllen SeanTAllen force-pushed the ban-unstable-variables branch from 9d9afa7 to 9888b96 Compare March 1, 2022 21:06
@SeanTAllen
Copy link
Member Author

I rebased this against main

@SeanTAllen
Copy link
Member Author

Failing test is unrelated.

@jasoncarr0 jasoncarr0 requested a review from jemc March 1, 2022 22:56
@SeanTAllen SeanTAllen merged commit e9db754 into main Mar 8, 2022
@SeanTAllen SeanTAllen deleted the ban-unstable-variables branch March 8, 2022 18:33
@ponylang-main ponylang-main removed the discuss during sync Should be discussed during an upcoming sync label Mar 8, 2022
github-actions bot pushed a commit that referenced this pull request Mar 8, 2022
github-actions bot pushed a commit that referenced this pull request Mar 8, 2022
@SeanTAllen SeanTAllen mentioned this pull request Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ephemeral RefCap shouldn't be an assignable refcap type
4 participants