Drop retains annotations in inferred type trees #20057
Conversation
| @@ -48,7 +48,7 @@ import staging.StagingLevel | |||
| import reporting.* | |||
| import Nullables.* | |||
| import NullOpsDecorator.* | |||
| import cc.{CheckCaptures, isRetainsLike} | |||
| import cc.{CheckCaptures, isRetainsLike, cleanupRetains} | |||
There was a problem hiding this comment.
Looks like this import is not needed
| // See #20035. | ||
| private def cleanupRetainsAnnot(symbol: Symbol, tpt: Tree)(using Context): Tree = | ||
| tpt match | ||
| case tpt: InferredTypeTree if !symbol.allOverriddenSymbols.hasNext => |
There was a problem hiding this comment.
Would be good to explain the condition. Something like:
// if there are overridden symbols, the annotation comes from an explicit type of the overridden symbol, and should be retained.
| def apply(tp: Type): Type = cleanupRetains(tp, this) | ||
|
|
||
| /** Drop retains annotations in the type. */ | ||
| def cleanupRetains(tp: Type, theMap: CleanupRetains | Null = null)(using Context): Type = |
There was a problem hiding this comment.
You don't need the theMap trick. Just define a normal typemap like:
class CleanupRetains(using Context) extends TypeMap:
def apply(tp: Type): Type = tp match
case RetainingType(tp, _) => tp
case _ => mapOver(tp)and use that instead of the cleanupRetains method in PostTyper.
The theMap trick is used if we want to avoid creating the type map for simple types because the operation is very hot. But to pay off, you then need to actually handle simple types in the associated methods. Here, cleanupRetains is fairly warm but not as hot as methods that use the theMap trick such as substitutions.
| @@ -454,7 +454,7 @@ class Setup extends PreRecheck, SymTransformer, SetupAPI: | |||
| case _ => false | |||
|
|
|||
| def signatureChanges = | |||
| tree.tpt.hasRememberedType && !sym.isConstructor || paramSignatureChanges | |||
| (tree.tpt.hasRememberedType || tree.tpt.isInstanceOf[InferredTypeTree]) && !sym.isConstructor || paramSignatureChanges | |||
There was a problem hiding this comment.
Why is the added condition needed?
There was a problem hiding this comment.
After a second thought this feels like a hack. The actual problem this tries to solve is that, for instance, given a ValDef value x: A^{y}, the PostTyper cleans up the capture set in the type tree of the definition but the info of the symbol x is not updated.
Later on Setup tries to read the info of that symbol here, and if the capture set is invalid, the compiler crashes.
No description provided.