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

Type param constraints now respect default cap of the type. #2675

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jemc
Copy link
Member

@jemc jemc commented Apr 30, 2018

Prior to this change, any type named in a type constraint would
get an implicit cap of #any if no explicit cap was named.
The exception to this rule is primitives, which will use an implicit
cap of val in a type constraint (though I'm not sure why you'd
ever use a primitive as a type constraint to begin with).
In both cases, the default cap declared by the user in the type
declaration was being ignored, and this is the only context where
that happens, violating the principle of least surprise.
I also don't think this deviation is documented anywhere.

As a veteran pony programmer, this still surprises and annoys me
regularly when I run into it; I commonly make this mistake and
have to recompile with an explicit cap that matches my declared default cap.

Furthermore, using #any as a type parameter constraint is rarely what
you actually want (not constraining the cap at all turns out to not
let you do very much), so it doesn't make much sense to be the
implicit cap. To illustrate this, there isn't a single example of
a type constraint having a final cap of #any in the standard library.
The lines that have been affected by this change are all cases of using
type intersections where only one of the terms was being used to constrain
the cap.

After this change the default cap declared by the user will be
universally respected.

This is a breaking change for anyone who is relying on this behaviour.

@jemc jemc added needs discussion during sync changelog - changed Automatically add "Changed" CHANGELOG entry on merge labels Apr 30, 2018
@jemc jemc self-assigned this Apr 30, 2018
@jemc
Copy link
Member Author

jemc commented Apr 30, 2018

I believe this to be a "principle of least surprise" bug, so I didn't do an RFC for it at this time.

However, I do want to discuss it a bit before merging, to see if there is any dissent from that interpretation.

@jemc
Copy link
Member Author

jemc commented Oct 3, 2018

We discussed it today on sync, and no one raised objections about wanting this to go through the RFC process.

@SeanTAllen
Copy link
Member

I rebased this against master.

Base automatically changed from master to main February 8, 2021 23:02
Prior to this change, any type named in a type constraint would
get an implicit cap of `#any` if no explicit cap was named.
The exception to this rule is primitives, which will use an implicit
cap of `val` in a type constraint (though I'm not sure why you'd
ever use a primitive as a type constraint to begin with).
In both cases, the default cap declared by the user in the type
declaration was being ignored, and this is the only context where
that happens, violating the principle of least surprise.
I also don't think this deviation is documented anywhere.

As a veteran pony programmer, this still surprises and annoys me
regularly when I run into it; I commonly make this mistake and
have to recompile with an explicit cap that matches my declared default cap.

Furthermore, using #any as a type parameter constraint is rarely what
you actually want (not constraining the cap at all turns out to not
let you do very much), so it doesn't make much sense to be the
implicit cap. To illustrate this, there isn't a single example of
a type constraint having a final cap of `#any` in the standard library.
The lines that have been affected by this change are all cases of using
type intersections where only one of the terms was being used to constrain
the cap.

After this change the default cap declared by the user will be
universally respected.

This is a breaking change for anyone who is relying on this behaviour.
@SeanTAllen SeanTAllen force-pushed the change/type-param-constraint-default-cap branch from 8454286 to b0912eb Compare January 29, 2024 02:06
@SeanTAllen
Copy link
Member

@jemc do you know why we never merged this?

Also, there are two failing tests now, would they be expected and need to be updated for this change?

[  FAILED  ] IftypeTest.ThenClause_CapConstraint
[  FAILED  ] IftypeTest.ElseClause_NoCapConstraint

It appears that yes, they should be failing and we need to update.

@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Jan 29, 2024
@SeanTAllen
Copy link
Member

If this was going to be merged, it needs release notes.

@SeanTAllen SeanTAllen removed the discuss during sync Should be discussed during an upcoming sync label Jan 30, 2024
@jemc
Copy link
Member Author

jemc commented Jan 30, 2024

The failing test appears to be a real failure of this change. And off the top of my head I'm not sure exactly how that case should be handled. It needs further thinking here.

@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Jan 30, 2024
@SeanTAllen SeanTAllen removed the discuss during sync Should be discussed during an upcoming sync label Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - changed Automatically add "Changed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants