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

Don’t consider type arguments inside a constraint as constraints. #1870

Merged
merged 1 commit into from Apr 29, 2017

Conversation

Projects
None yet
2 participants
@plietar
Copy link
Contributor

commented Apr 27, 2017

This better reflects their use. It allows arrow types and tuples to
appear in these positions. It also denies capability sets from appearing
in these positions. These have an extremely limited use, and are unlikely
to be used by anyone.

As a side effect, the default capability in these positions now
corresponds to the type default, rather than #any.

Also properly deny arrow types from appearing in constraints,
fixing #1809.


I'd like to add some positive tests for arrows and tuples in these positions,
but I'm not sure where they fit.

Don’t consider type arguments inside a constraint as constraints.
This better reflects their use. It allows arrow types and tuples to
appear in these positions. It also denies capability sets from appearing
in these positions. These have an extremely limited use, and are unlikely
to be used by anyone.

As a side effect, the default capability in these positions now
corresponds to the type default, rather than #any.

Also properly deny arrow types from appearing in constraints,
fixing #1809.

@plietar plietar force-pushed the plietar:fix-nested-constraints branch from 8eb3a8c to 3333e6d Apr 27, 2017

@jemc

This comment has been minimized.

Copy link
Member

commented Apr 29, 2017

Thanks!

I'd like to add some positive tests for arrows and tuples in these positions,
but I'm not sure where they fit.

I agree - I don't think any of the existing test files are a good fit. Probably the best bet would be to create a new test file called type_params.cc or something similar.

However, since this is already a complete fix for the other bugs you noticed, I'll go ahead and merge this PR. You can feel free to add the positive tests in another PR if you wish.

@jemc jemc merged commit bc479f4 into ponylang:master Apr 29, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

ponylang-main added a commit that referenced this pull request Apr 29, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.