-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Adjustments to the capability trilogy #23428
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
Conversation
To be merged after #23427. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves the conversion of capabilities by turning invariant cap occurrences into Freshes and converting fresh caps of def methods with only type parameters into result caps. Key changes include:
- Converting invariants to Fresh ownership in capToFresh.
- Adapting toResultInResults to also cover def methods with type parameters.
- Refactoring internal utilities to use deep boxing (boxDeeply) and adjusting variance handling in capability mapping.
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
tests/pos-custom-args/captures/singleton-subtyping.scala | Updates to capability conversion and singleton subtyping tests. |
tests/neg-custom-args/captures/i16725.scala | Adjusted inline error comments for wrapped IO usage. |
compiler/src/dotty/tools/dotc/cc/Setup.scala | Replaced usages of box with boxDeeply and refined defaultApply behavior. |
compiler/src/dotty/tools/dotc/cc/Capability.scala | Refined the variance check in capability mapping from <= to <. |
... | Other files show updates to test expectations and diagnostic messages. |
Comments suppressed due to low confidence (3)
tests/neg-custom-args/captures/i16725.scala:10
- The inline error comment on this line is now misleading since the expected behavior has changed; please update or remove the comment to reflect the current expected behavior.
wrapper.value: io =>
compiler/src/dotty/tools/dotc/cc/Capability.scala:696
- The change from 'if variance <= 0' to 'if variance < 0' refines the handling of invariant positions; please verify that this update aligns with the intended capability variance semantics.
if variance < 0 then t
compiler/src/dotty/tools/dotc/cc/Setup.scala:228
- Replacing 'box' with 'boxDeeply' improves the safety of nested capture conversions; double-check that this change preserves the intended behavior across all application sites.
tp.derivedAppliedType(tycon, args.mapConserve(_.boxDeeply))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise LGTM
It was unsound, but fixed now on main.
Fixes #23421.
This PR includes the following two changes:
capToFresh
, turncap
s in invariant occurrences also toFresh
es. Previously, in the follow code:The
cap
is kept intact since it is invariant. Now, it gets converted into aFresh
owned byxs
.2. In
toResultInResults
, convert fresh caps ofdef
-method with only type parameters into result caps, just like parameterless defs. Specifically, for the following code:The
^
will be turned into a result cap, just like what happens for a parameterless def:The
^
is a result cap that gets instantiated to a new fresh at each application ofmkRef
. Doing the same thing forempty
is the arguably more desirable behavior.