Refactorings around CaptureRef #20893
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- isBoxedCaptured no longer requires the construction of intermediate capture sets. - isAlwaysEmpty is also true for solved variables that have no elements
- Use a uniform criterion when to add them - Don't add them for @constructorOnly or @cc.untrackedCaptures arguments @untrackedCaptures is a new annotation
- Improve error messages - Better propagation of @uncheckedCaptures - -un-deprecacte caps.unsafeUnbox and friends.
We go back to the original lifetime restriction that box/unbox cannot apply to universal capture sets, and drop the later restriction that type variable instantiations may not deeply capture cap. The original restriction is proven to be sound and is probably expressive enough when we add reach capabilities. This required some changes in tests and also in the standard library. The original restriction is in place for source <= 3.2 and >= 3.5. Source 3.3 and 3.4 use the alternative restriction on type variable instances. Some neg tests have not been brought forward to 3.4. They are all in tests/neg-customargs/captures and start with //> using options -source 3.4 We need to look at these tests one-by-one and analyze whether the new 3.5 behavior is correct.
The previous scheme relied on subtle and unstated assumptions between symbol updates and re-checking. If they were violated some definitions could not be rechecked at all. The new scheme is more robust. We always re-check except when the checker implementation returns true for `skipRecheck`. And that test is based on an explicitly maintained set of completed symbols.
Enabled from 3.5. There are still a number of open questions - Clarify type inference with existentials propagating into capture sets. Right now, no pos or run test exercises this. - Also map arguments of function to existentials (at least double flip ones). - Adapt reach capabilities and drop previous restrictions.
Fixes the problem in effect-swaps.scala
This gives us the necessary levers to switch to existential capabilities. # Conflicts: # compiler/src/dotty/tools/dotc/cc/CaptureOps.scala
An expandedUniversalSet was the same as `{cap}` but not reference-equal
to CaptureSet.universal. This construct was previously needed to avoid
multiple expansions, but this does not seem to be the case any longer so
the construct can be dropped.
# Conflicts: # compiler/src/dotty/tools/dotc/cc/Setup.scala
The encoding of (x: T) => U in capture checked code has changed.
Previously: T => U' { def apply(x: T): U }
Now: (T -> U' { def apply(x: T): U })^{cap}
We often handle dependent functions by transforming the apply method and then mapping
back to a function type using `.toFunctionType`. But that would always generate a
pure function, so the impurity info could get lost.
Still missing: Mapping parameters
Fixes crash with opaque types reported by @natsukagami
- Add existentials to inferred types - Map existentials in one compared type to existentials in the other - Also: Don't re-analyze existentials in mapCap.
There was a forgotten case.
These should be no longer necessary with existentials.
A parameter accessor with a nonempty deep capture set needs to be tracked in refinements even if it is pure, as long as it might contain captures that can be referenced using a reach capability.
When we take `{elem} <: B ++ C` where `elem` is not yet included in `B ++ C`,
B is a constant and C is a variable, propagate with `{elem} <: C`.
Likewise if C is a constant and B is a variable.
This tries to minimize the slack between a union and its operands.
Note: Propagation does not happen very often in our test suite so far: Once in pos tests and
15 times in scala2-library-cc.
Don't treat user-defined capabilities deriving from caps.Capability as maximal. That was a vestige from when we treated capability classes natively. It caused code that should compile to fail because if `x extends Capability` then `x` could not be widened to `x*`. As a consequence we have one missed error in effect-swaps again, which re-establishes the original (faulty) situation.
We might want to treat it specially since a RefiningVar should ideally be closed for further additions when the constructor has been analyzed.
Use the syntactic sugar instead of expanding with capsOf
Make all operations final methods on Type or CaptureRef
odersky
changed the title
Move extension methods on CaptureRef into CaptureRef itself or into CaptureOps
odersky
force-pushed
the
captureref-refactor
branch
from
830caa2 to
c971af4
Compare
|
I integrated this PR in #20566 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Builds on #20566
Breaks out CaptureRef into a separate file and concentrates all operations on CaptureRef in it.
Moves extension methods on CaptureRef into CaptureRef itself or into CaptureOps.