ponyc icon indicating copy to clipboard operation
ponyc copied to clipboard

Type param constraints now respect default cap of the type.

Open jemc opened this issue 7 years ago • 6 comments

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 avatar Apr 30 '18 03:04 jemc

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 avatar Apr 30 '18 03:04 jemc

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

jemc avatar Oct 03 '18 19:10 jemc

I rebased this against master.

SeanTAllen avatar Sep 25 '20 02:09 SeanTAllen

@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.

SeanTAllen avatar Jan 29 '24 02:01 SeanTAllen

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

SeanTAllen avatar Jan 30 '24 19:01 SeanTAllen

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.

jemc avatar Jan 30 '24 19:01 jemc