Fixes for cleanup retains scheme #21350
Conversation
These arguments tell the whole truth; they cannot possibly be decorated with another capture set. So we should not add a capture set variable.
These need to be handled like reach capabilities. Fixes scala#21347
- Cleanup all inferred types, not just result types of ValDefs and DefDefs. - To compensate, map overriding ValDefs and DefDefs to have declared result types. - Make type trees generated for varargs inferred.
This is now redundant since result types of overriding ValDefs and DefDefs are now mapped to TypeTrees in PostTyper.
odersky
force-pushed
the
fix-cleanupRetains
branch
from
11e2986 to
a8cc133
Compare
|
This should solve the problems with varargs we were seeing recently. |
There was a problem hiding this comment.
This looks good to me. Cleaning up all types sounds safer.
On a more general note, it is however not clear to me that this is yet enough: can't non-sensical inferred types have escaped outside of trees before they are cleaned up, for example in symbol's denotations? Wouldn't there be even more to cleanup?
| 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.
Also map annotations embedded in type?
Does that only transform outermost annotations? Shouldn't this be a TypeMap?
| * retains annotations come from the explicitly declared parent types, so should | ||
| * be kept. | ||
| */ | ||
| private def makeOverrideTypeDeclared(symbol: Symbol, tpt: Tree)(using Context): Tree = |
There was a problem hiding this comment.
So if I understand correctly, an overridden member:
- is typed normally like any definition during
Typer, - if its type tree is an
InferredTypeTree, it get switched to aTypeTreehere duringPostTyper, - gets its typed checked against the overloaded definition only later during
RefChecks.
Seems to make sense ✅
Only trees are pickled, so that means if we make sure all critical type trees are InferredTypeTrees we should be OK. |
Based on #21309.