-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Fixes for cleanup retains scheme #21350
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
e98b503
Fix setup of CapSet arguments.
odersky 02b1b6d
Implement caps.Contains
odersky 618bbc5
Handle local type parameters in markFree
odersky 8584a9a
Fixes for cleanup retains scheme
odersky a8cc133
Drop redundant exact distinction in Setup
odersky 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 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 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 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 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 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 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 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 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 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 |
---|---|---|
|
@@ -303,20 +303,19 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase => | |
if !tree.symbol.is(Package) then tree | ||
else errorTree(tree, em"${tree.symbol} cannot be used as a type") | ||
|
||
// Cleans up retains annotations in inferred type trees. This is needed because | ||
// during the typer, it is infeasible to correctly infer the capture sets in most | ||
// cases, resulting ill-formed capture sets that could crash the pickler later on. | ||
// See #20035. | ||
private def cleanupRetainsAnnot(symbol: Symbol, tpt: Tree)(using Context): Tree = | ||
/** Make result types of ValDefs and DefDefs that override some other definitions | ||
* declared types rather than InferredTypes. This is necessary since we otherwise | ||
* clean retains annotations from such types. But for an overriding symbol the | ||
* retains annotations come from the explicitly declared parent types, so should | ||
* be kept. | ||
*/ | ||
private def makeOverrideTypeDeclared(symbol: Symbol, tpt: Tree)(using Context): Tree = | ||
tpt match | ||
case tpt: InferredTypeTree | ||
if !symbol.allOverriddenSymbols.hasNext => | ||
// if there are overridden symbols, the annotation comes from an explicit type of the overridden symbol | ||
// and should be retained. | ||
val tm = new CleanupRetains | ||
val tpe1 = tm(tpt.tpe) | ||
tpt.withType(tpe1) | ||
case _ => tpt | ||
if symbol.allOverriddenSymbols.hasNext => | ||
TypeTree(tpt.tpe, inferred = false).withSpan(tpt.span).withAttachmentsFrom(tpt) | ||
case _ => | ||
tpt | ||
|
||
override def transform(tree: Tree)(using Context): Tree = | ||
try tree match { | ||
|
@@ -432,7 +431,7 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase => | |
registerIfHasMacroAnnotations(tree) | ||
checkErasedDef(tree) | ||
Checking.checkPolyFunctionType(tree.tpt) | ||
val tree1 = cpy.ValDef(tree)(tpt = cleanupRetainsAnnot(tree.symbol, tree.tpt), rhs = normalizeErasedRhs(tree.rhs, tree.symbol)) | ||
val tree1 = cpy.ValDef(tree)(tpt = makeOverrideTypeDeclared(tree.symbol, tree.tpt), rhs = normalizeErasedRhs(tree.rhs, tree.symbol)) | ||
if tree1.removeAttachment(desugar.UntupledParam).isDefined then | ||
checkStableSelection(tree.rhs) | ||
processValOrDefDef(super.transform(tree1)) | ||
|
@@ -441,7 +440,7 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase => | |
checkErasedDef(tree) | ||
Checking.checkPolyFunctionType(tree.tpt) | ||
annotateContextResults(tree) | ||
val tree1 = cpy.DefDef(tree)(tpt = cleanupRetainsAnnot(tree.symbol, tree.tpt), rhs = normalizeErasedRhs(tree.rhs, tree.symbol)) | ||
val tree1 = cpy.DefDef(tree)(tpt = makeOverrideTypeDeclared(tree.symbol, tree.tpt), rhs = normalizeErasedRhs(tree.rhs, tree.symbol)) | ||
processValOrDefDef(superAcc.wrapDefDef(tree1)(super.transform(tree1).asInstanceOf[DefDef])) | ||
case tree: TypeDef => | ||
registerIfHasMacroAnnotations(tree) | ||
|
@@ -524,12 +523,12 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase => | |
report.error(em"type ${alias.tpe} outside bounds $bounds", tree.srcPos) | ||
super.transform(tree) | ||
case tree: TypeTree => | ||
tree.withType( | ||
tree.tpe match { | ||
case AnnotatedType(tpe, annot) => AnnotatedType(tpe, transformAnnot(annot)) | ||
case tpe => tpe | ||
} | ||
) | ||
val tpe = if tree.isInferred then CleanupRetains()(tree.tpe) else tree.tpe | ||
tree.withType: | ||
tpe match | ||
case AnnotatedType(parent, annot) => | ||
AnnotatedType(parent, transformAnnot(annot)) // TODO: Also map annotations embedded in type? | ||
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.
Does that only transform outermost annotations? Shouldn't this be a |
||
case _ => tpe | ||
case Typed(Ident(nme.WILDCARD), _) => | ||
withMode(Mode.Pattern)(super.transform(tree)) | ||
// The added mode signals that bounds in a pattern need not | ||
|
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
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
-- Error: tests/neg-custom-args/captures/i21313.scala:6:27 ------------------------------------------------------------- | ||
6 |def foo(x: Async) = x.await(???) // error | ||
| ^ | ||
| (x : Async) is not a tracked capability | ||
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/i21313.scala:15:12 --------------------------------------- | ||
15 | ac1.await(src2) // error | ||
| ^^^^ | ||
| Found: (src2 : Source[Int, caps.CapSet^{ac2}]^?) | ||
| Required: Source[Int, caps.CapSet^{ac1}]^ | ||
| | ||
| longer explanation available when compiling with `-explain` |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
import caps.CapSet | ||
|
||
trait Async: | ||
def await[T, Cap^](using caps.Contains[Cap, this.type])(src: Source[T, Cap]^): T | ||
|
||
def foo(x: Async) = x.await(???) // error | ||
|
||
trait Source[+T, Cap^]: | ||
final def await(using ac: Async^{Cap^}) = ac.await[T, Cap](this) // Contains[Cap, ac] is assured because {ac} <: Cap. | ||
|
||
def test(using ac1: Async^, ac2: Async^, x: String) = | ||
val src1 = new Source[Int, CapSet^{ac1}] {} | ||
ac1.await(src1) // ok | ||
val src2 = new Source[Int, CapSet^{ac2}] {} | ||
ac1.await(src2) // error |
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.
So if I understand correctly, an overridden member:
Typer
,InferredTypeTree
, it get switched to aTypeTree
here duringPostTyper
,RefChecks
.Seems to make sense ✅