-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Fix inferred capture-polymorphic function literals #25936
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
Closed
Closed
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
0dab73f
Remove CleanupRetains invocation from inlineCall
bracevac d39d372
Fix and restrict inferred capture-polymorphic function literals
bracevac 3198b73
Detect poly-fn literals nested in Function1s
bracevac 856f147
Don't use tree attachment to guide CleanupRetains
odersky d960d08
Address review comments
bracevac File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -338,14 +338,38 @@ class Setup extends PreRecheck, SymTransformer, SetupAPI: | |
|
|
||
| def innerApply(tp: Type) = | ||
| val tp1 = tp match | ||
| case AnnotatedType(parent, annot: RetainingAnnotation) => | ||
| val elems = annot.retainedType.retainedElementsRaw | ||
| def isRetainedParamRef(elem: Type): Boolean = elem match | ||
| case _: ParamRef => true | ||
| // CleanupRetains may also preserve capset TypeRefs (e.g. to a | ||
| // type member of a synthetic poly-function class that aliases an | ||
| // outer apply method's type parameter). | ||
| case ref: TypeRef => ref.derivesFromCapSet | ||
| case _ => false | ||
| def promote(caps: List[Capability]) = | ||
| // Refs preserved by CleanupRetains are part of the inferred | ||
| // polymorphic function interface. Keep them as a Const | ||
| // CapturingType and strip the empty Var that `apply(parent)` | ||
| // would have layered underneath. | ||
| val parent1 = apply(parent) match | ||
| case CapturingType(p, refs) if refs.elems.isEmpty && !refs.isConst => p | ||
| case other => other | ||
| CapturingType(parent1, CaptureSet(caps*)) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This might be too simplistic. There are a lot of subtleties and corner cases whan mapping a retains to a capture set. We should use the same code as in |
||
| try | ||
| if elems.nonEmpty && elems.forall(isRetainedParamRef) then | ||
| promote(elems.map(_.toCapability)) | ||
| else apply(parent) | ||
| catch case _: IllegalCaptureRef => | ||
| apply(parent) | ||
| case AnnotatedType(parent, annot) | ||
| if annot.symbol.isRetains || annot.symbol == defn.InferredAnnot => | ||
| // Drop explicit retains and @inferred annotations | ||
| // Drop non-RetainingAnnotation retains (e.g. pickle-read) and @inferred. | ||
| apply(parent) | ||
| case tp: TypeLambda => | ||
| // Don't recurse into parameter bounds, just cleanup any stray retains annotations | ||
| // Leave parameter bounds alone; CleanupRetains already filtered them. | ||
| tp.derivedLambdaType( | ||
| paramInfos = tp.paramInfos.mapConserve(_.dropAllRetains.bounds), | ||
| paramInfos = tp.paramInfos, | ||
| resType = this(tp.resType)) | ||
| case tp @ RefinedType(parent, rname, rinfo) => | ||
| val saved = refiningNames | ||
|
|
@@ -879,6 +903,9 @@ class Setup extends PreRecheck, SymTransformer, SetupAPI: | |
| * - If type is a capturing type that has already a capture set variable or has | ||
| * the universal capture set, it does not need a variable. | ||
| */ | ||
| private def isNonEmptyParamRefSet(refs: CaptureSet)(using Context): Boolean = | ||
| !refs.elems.isEmpty && refs.elems.forall(_.core.isInstanceOf[ParamRef]) | ||
|
|
||
| def needsVariable(tp: Type)(using Context): Boolean = | ||
| tp.typeParams.isEmpty && tp.match | ||
| case tp: (TypeRef | AppliedType) => | ||
|
|
@@ -899,6 +926,9 @@ class Setup extends PreRecheck, SymTransformer, SetupAPI: | |
| needsVariable(parent) | ||
| && refs.isConst // if refs is a variable, no need to add another | ||
| && !refs.isUniversal // if refs is {caps.any}, an added variable would not change anything | ||
| // Non-empty Const sets that contain only parameter refs must stay Const: | ||
| // a Var's elements don't rewrite under SubstParamsMap. See i25830. | ||
| && !isNonEmptyParamRefSet(refs) | ||
| case AnnotatedType(parent, _) => | ||
| needsVariable(parent) | ||
| case _ => | ||
|
|
||
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| import language.experimental.captureChecking | ||
| import caps.* | ||
|
|
||
| class File extends SharedCapability | ||
|
|
||
| // TODO these cases are currently restricted by the implementation, but we should consider allowing some of them in the future. See i25830 for details. | ||
| // The general case for capture-polymorphic lambdas is non-trivial to implement and we want to get the implementation right before allowing more cases. | ||
| // On the other hand, they are currently rarely needed in practice and the workaround is not too bad, so we can live with the restriction for now. | ||
|
|
||
| def mixedExternal() = | ||
| val external = File() | ||
| val f = // error | ||
| { [C^] => (xs: List[File^{C, external}]) => xs } | ||
|
|
||
| def externalOnly() = | ||
| val external = File() | ||
| val f = // error | ||
| { [C^] => (xs: List[File^{external}]) => xs } | ||
|
|
||
| def mixedExternalWithLowerBoundedParam() = | ||
| val external = File() | ||
| val f = // error | ||
| { [C^, D^ >: {C}] => (xs: List[File^{D, external}]) => xs } | ||
|
|
||
| def mixedExternalInLaterParamList() = | ||
| val external = File() | ||
| val f = // error | ||
| { [C^] => (xs: List[File^{C}]) => (ys: List[File^{C, external}]) => ys } | ||
|
|
||
| def enclosingParam(external: File^) = | ||
| val f = // error | ||
| { [C^] => (xs: List[File^{C}]) => (ys: List[File^{external}]) => ys } | ||
|
|
||
| def unsupportedDef() = | ||
| val external = File() | ||
| def f = // error | ||
| { [C^] => (xs: List[File^{C, external}]) => xs } | ||
|
|
||
| def unsupportedInsideAnonymousFunction() = | ||
| List(File()).map: external => | ||
| val f = // error | ||
| { [C^] => (xs: List[File^{C}]) => (ys: List[File^{external}]) => ys } | ||
|
|
||
| def externalInBound() = | ||
| val external = File() | ||
| val f = // error | ||
| { [C^, D^ <: {C, external}] => (xs: List[File^{D}]) => xs } | ||
|
|
||
| def nestedCapsetBinders() = | ||
| val f = // error | ||
| { [C^] => (xs: List[File^{C}]) => [D^] => (ys: List[File^{C, D}]) => ys } | ||
|
|
||
| def literalNestedInFunction1() = | ||
| val external = File() | ||
| val f = // error | ||
| (i: Int) => { [C^] => (xs: List[File^{C, external}]) => xs } |
This file was deleted.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
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.
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.
The treatment here assumes that we have cleaned up the type with
CleanupRetainsbefore. But that's not true if the type arose from inlining, since we just droppedCleanupRetainsthere. We probably need to bring backCleanupRetainsfor inlined code.