Fix type inference for capture-polymorphic lambdas#25833
Fix type inference for capture-polymorphic lambdas#25833bracevac wants to merge 6 commits intoscala:mainfrom
Conversation
Fix scala#25830 **Problem.** `val convert = { [C^] => (xs: List[File^{C}]) => xs.map(_ => ()) }` had its capture-set type parameter `C` silently erased during capture checking, so `convert[{x}](files)` failed with `List[File^{}]` instead of `List[File^{x}]`. **Root cause.** Three places dropped or rewrote the `{C}` reference in the val's inferred type: 1. `CleanupRetains` in [CaptureOps.scala](compiler/src/dotty/tools/dotc/cc/CaptureOps.scala) rewrote every inferred `@retains[T]` to `@retains[Nothing]`. 2. `Setup.mapInferred` in [Setup.scala](compiler/src/dotty/tools/dotc/cc/Setup.scala) stripped `@retains` entirely and then `addVar` attached a fresh empty variable. 3. `needsVariable` said a const capture set containing a type-param reference still needed a fresh variable, severing the link to `C` across type-arg substitution. **Fix.** - `CleanupRetains` now tracks enclosing `LambdaType` binders and preserves `@retains[TypeParamRef]` whose binder is in scope (safe for the pickler). Other retains are still normalized to `@retains[Nothing]`, so unrelated tests like `cap-paramlists5.scala` and `nicolas1.scala` keep compiling. - `Setup.mapInferred.innerApply` has a new case that preserves a retains whose elements derive from `CapSet`, and the `TypeLambda` case keeps CapSet-derived retains on parameter bounds so `[C^]`'s `CapSet^{any}` upper bound survives. - `Setup.needsVariable` refuses to wrap a const set whose elements reference a capture-set type parameter.
odersky
left a comment
odersky
left a comment
There was a problem hiding this comment.
This looks uncomfortably shaky to me. I think the conclusion is that we don't really know how to handle local capset variables in polymorphic lambdas. Instead of trying half solutions I think it might be better to just refuse to handle them. Can we disallow polymorphic lambdas containing capset variables for now? Would tests break if we did that?
| // tpe stay erased — see nicolas1.scala, cap-paramlists5.scala. | ||
| case ref: TypeRef if ref.derivesFromCapSet => | ||
| val sym = ref.symbol | ||
| sym.isType && (binders.nonEmpty || sym.owner.isAnonymousFunction) |
There was a problem hiding this comment.
The logic behind this condition is unclear to me. If binders is non-empty, we treat all captset variables as local, not just variables referring to a binder? Why?
| case tp @ AnnotatedType(parent, annot: RetainingAnnotation) => | ||
| if Feature.ccEnabled then | ||
| if annot.symbol == defn.RetainsCapAnnot then tp | ||
| else if annot.retainedType.retainedElementsRaw.exists(isLocalCapSetParam) then |
There was a problem hiding this comment.
Why not forall? In fact, no matter what we do I fear we will always do the wrong thing for a capset that contains a local binder and also some other references.
| // original retains set so the reference to `C` survives later type-arg | ||
| // substitution. Without this, `{C}` would be silently erased to `{}`. | ||
| // See i25830. | ||
| val parent1 = apply(parent) match |
There was a problem hiding this comment.
I am a bit nervous that we bypass the logic for retained elements containing a capset variable. What about the other elements? What if the variable is not from a local binder? Aren't we overfitting the test cases here?
| if bnd.derivesFromCapSet then bnd else bnd.dropAllRetains | ||
| tp.derivedLambdaType( | ||
| paramInfos = tp.paramInfos.mapConserve(_.dropAllRetains.bounds), | ||
| paramInfos = tp.paramInfos.mapConserve(cleanBound(_).bounds), |
There was a problem hiding this comment.
Same thing here. Why exempt all capset variables?
odersky
left a comment
odersky
left a comment
There was a problem hiding this comment.
I think we have to conclude that we don't really know how to handle polymorphic lambdas over capset variables. Instead of trying half solutions, maybe it is better to reject for now such lambdas as an implementation restriction? Would any existing tests break if we did so?
We do have a couple. Some are purely for checking parsing, like the |
|
Hmm. Can we find another criterion that allows the current tests (in particular regions) and that lets us stay on safe ground? |
|
In the regions example we require a polymorphic lambda of type The problem lies with inference for lambdas that should have types like |
|
Superseded by #25853 |
2 participants
Fix #25830
Problem.
val convert = { [C^] => (xs: List[File^{C}]) => xs.map(_ => ()) }had its capture-set type parameterCsilently erased during capture checking, soconvert[{x}](files)failed withList[File^{}]instead ofList[File^{x}].Root cause. Three places dropped or rewrote the
{C}reference in the val's inferred type:CleanupRetainsin CaptureOps.scala rewrote every inferred@retains[T]to@retains[Nothing].Setup.mapInferredin Setup.scala stripped@retainsentirely and thenaddVarattached a fresh empty variable.needsVariablesaid a const capture set containing a type-param reference still needed a fresh variable, severing the link toCacross type-arg substitution.Fix.
CleanupRetainsnow tracks enclosingLambdaTypebinders and preserves@retains[TypeParamRef]whose binder is in scope (safe for the pickler). Other retains are still normalized to@retains[Nothing], so unrelated tests likecap-paramlists5.scalaandnicolas1.scalakeep compiling.Setup.mapInferred.innerApplyhas a new case that preserves a retains whose elements derive fromCapSet, and theTypeLambdacase keeps CapSet-derived retains on parameter bounds so[C^]'sCapSet^{any}upper bound survives.Setup.needsVariablerefuses to wrap a const set whose elements reference a capture-set type parameter.How much have you relied on LLM-based tools in this contribution?
No, LLMs rely on ME.
How was the solution tested?
New automated tests (including the issue's reproducer, if applicable)