Fix regression #26153 (in dotty-cps-async) use typer skolems as inline proxy's type - #26563
Fix regression #26153 (in dotty-cps-async) use typer skolems as inline proxy's type#26563tanishiking wants to merge 1 commit into
Conversation
d82e068 to
e7b6387
Compare
| * To avoid this problem, type `am$proxy` as the skolem `$1` created by typer. | ||
| * Then the expanded tree and the call types match. | ||
| */ | ||
| private val callValueSkolemss: List[List[Option[SkolemType]]] = |
There was a problem hiding this comment.
Collect skolems from Typer through SkolemizedArgs
| // For the unpickler to recover the above precise type instead of | ||
| // recomputing the wrong type, we insert a no-op cast on the expansion, | ||
| // so unpickler can pick that type. | ||
| val inlined = |
There was a problem hiding this comment.
This change is for adapting the tree/type for TASTy pickling, otherwise some tests fail with -Ytest-pickle.
f8b5a44 to
fc38252
Compare
fc38252 to
4696472
Compare
Fixes scala#26153 Fixes scala#26031 Fixes scala#26624 **problem** When an inline expansion has a type that depends on one of the inline method arguments, and that argument is not stable, there's a type mismatch error on inline function call. For example, ```scala trait Mon[F[_]] { type Arg } class Wrap[F[_], A]: inline def apply[T](inline f: A => T): F[T] = ... inline def async[F[_]](using am: Mon[F]) = new Wrap[F, am.Arg] object Test: val r = async[[A] =>> (String, A)](_ => 42) // given TupleMon[E] extends Mon[[A] ==> (E, A)] ``` The result of `async` depends on the argument `am`: `Wrap[F, am.Arg]`. At the call site, the given argument for `am` is not a stable path. When `TypeAssigner` computes the dependent result type, it skolemizes that argument. So the inline call is typed as something like: `Wrap[[A] =>> (String, A), ?1.Arg]` where: `?1` is some value of type `Mon[[A] =>> (String, A)]`. On the other hand, `Inlining` creates a runtime proxy for the same arugment: `am$proxy` = <actual given>. When the proxy is typed as `am$proxy: Mon[[A] ==> (String, A)]`, then types in the expanded tree is like `am$proxy.Arg`. While the call tree was typed as `?1.Arg`. As a result, compile fails: ```scala val r = async[[A] =>> (String, A)](_ => 42) ^^^^^^^ Found: ?1.Arg => Int Required: am$proxy1.Arg => Int where: ?1 is an unknown value of type Mon[[A] =>> (String, A)] ``` The same root issue also causes regression scala#26031 ```scala trait Ctx[F[_]] trait Mon[F[_]]: type Context <: Ctx[F] object Mon: type Aux[F[_], C <: Ctx[F]] = Mon[F] { type Context = C } class Wrap[F[_], C <: Ctx[F]](using val am: Mon.Aux[F, C]) transparent inline def doIt[F[_]](using am: Mon[F]) = new Wrap(using am) ``` The call tree fixes `C = ?1.Context`, so the expansion needs an argument of type `Mon.Aux[F, ?1.Context]`. A proxy typed only as `Mon[F]` does not prove that its `Context` member is `?1.Context`. **Fix** The idea is Use the skolem created by typer as the type of the inline proxy. `val am$proxy: ?1 = <actual given>.asInstanceOf[?1]`. The runtime value is still the proxy, but its type-level prefix is now the same skolem generated by typer. This fixes the mismatch in scala#26153, and also fixes scala#26031 because the proxy term itself now has the skolem type. The idea is from: > I was wondering it would be great if we can use something like `Wrap[F, m.Arg] forSome { val m: Mon[F] }` instead of `?`, which is too coarse. And ... I realized that typer already uses skolem type `?1.Arg` when the result type is dependent. > scala#26033 (comment) --- This commit also dealias `NamedTypes` when homogenizing for `-Ytest-pickler`, otherwise, skolem before pickle vs unpickled has different type printing `Predef.String` vs `String` (tests/run/i14020) **note** Original PR scala#26033 was to special-case `selectionType` for an `Inlined` receiver. if the saved type of the `Inlined` tree was avoided to `?`, use the expansion's type instead. That can fix the immediate receiver mismatch, but it is workaround-ish. It recovers `am$proxy.Arg` after avoidance, which isn't right. while typer already had the better stable prefix `?1.Arg`. Another fix I tried was to rewrite only type positions: `am$proxy.Arg` => `?1.Arg` This helps scala#26153, where the mismatch is in the expanded type. But it does not fix scala#26031. A term with type `Mon[F]` still does not conform, it's required refinements.
4696472 to
f47f2a4
Compare
jchyb
left a comment
There was a problem hiding this comment.
LGTM, although there are 2 things I'm wondering about:
- I suspect for nested skolem types (where a skolem type appears as part of another type) this might not be enough, although that's not a case I am able to think of (might as well not exist, just thinking out loud), and we can patch it later
- Were async (following the first example) a macro this might not work since we purposefully try to unwrap any skolem types we see there and don't support them in the API, but at this point I guess it's expected
|
Thanks a lot @jchyb for reviewing!
While types containing skolems (like, those created by another inline function call) could become proxy's types, what we want to match (with proxy's type) here is a skolem created for this application's dependent result type, which is always obtained as a top-level skolem type. So I don't think it's an issue 👍
Ah, it seems like this change doesn't fix the macro scenario, yes. And I'm not sure how can we fix that as well... |
This PR is based on #26579, actual change is in second commitdoneTBH, not very sure why this change result in this regression 🤔 Previous to the commit,
sameTypes(Nil, <nonEmpty>)always (wrongly) returnedtrue. Maybe the issues come to surface because of fixing the bug ofsameTypes?Fixes #26153
Fixes #26031
Fixes #26624
problem
When an inline expansion has a type that depends on one of the inline method arguments, and that argument is not stable, there's a type mismatch error on inline function call.
For example,
The result of
asyncdepends on the argumentam:Wrap[F, am.Arg]. At the call site, the given argument foramis not a stable path. WhenTypeAssignercomputes the dependent result type, it skolemizes that argument. So the inline call is typed as something like:Wrap[[A] =>> (String, A), ?1.Arg]where:?1is some value of typeMon[[A] =>> (String, A)].On the other hand,
Inliningcreates a runtime proxy for the same arugment:am$proxy= .When the proxy is typed as
am$proxy: Mon[[A] ==> (String, A)], then types in the expanded tree is likeam$proxy.Arg. While the call tree was typed as?1.Arg.As a result, compile fails:
The same root issue also causes regression #26031
The call tree fixes
C = ?1.Context, so the expansion needs an argument of typeMon.Aux[F, ?1.Context]. A proxy typed only asMon[F]does not prove that itsContextmember is?1.Context.Fix
The idea is Use the skolem created by typer as the type of the inline proxy.
val am$proxy: ?1 = <actual given>.asInstanceOf[?1]. The runtime value is still the proxy, but its type-level prefix is now the same skolem generated by typer.This fixes the mismatch in #26153, and also fixes #26031 because the proxy term itself now has the skolem type.
The idea is from:
note
Original PR #26033 was to special-case
selectionTypefor anInlinedreceiver. if the saved type of theInlinedtree was avoided to?, use the expansion's type instead.That can fix the immediate receiver mismatch, but it just recovers
am$proxy.Argafter avoidance, which I don't feel right ?Another fix I tried was to rewrite only type positions:
am$proxy.Arg=>?1.ArgThis helps #26153, where the mismatch is in the expanded type. But it does not fix #26031. A term with type
Mon[F]still does not conform, it's required refinements.Have you relied on LLM-based tools in this contribution?
Yes, and I checked the output by comprehending by myself
How was the solution tested?
New automated tests (including the issue's reproducer, if applicable)