Only evaluate transparent inline unapply once #19380
Conversation
| @@ -1446,7 +1446,9 @@ trait Applications extends Compatibility { | |||
| unapplyArgType | |||
|
|
|||
| val dummyArg = dummyTreeOfType(ownType) | |||
| val unapplyApp = typedExpr(untpd.TypedSplice(Apply(unapplyFn, dummyArg :: Nil))) | |||
| val unapplyApp = withMode(Mode.NoInline) { // transparent inline unapplies are exapnded when indexing the pattern see `indexPattern` in Typer. | |||
There was a problem hiding this comment.
This will not work. We need to refine the types for the transparent unapply at this point.
We might need to fully inline the call here, which is proving to be a bit more involved.
There was a problem hiding this comment.
I managed to make it work.
The trick is to type this application with the dummy argument without inlining a just after that inline the unapply function. We also need to adapt unapply function unapplyFn to make the new code use the inlined code. Note that the inlining of the unapply function must be done on the function that receives the dummy argument, trailing implicits are kept as is.
nicolasstucki
force-pushed
the
fix-19369
branch
2 times, most recently
from
38fea45 to
3edd16b
Compare
| val unapplyApp = typedExpr(untpd.TypedSplice(Apply(unapplyFn, dummyArg :: Nil))) | ||
| val (newUnapplyFn, unapplyApp) = | ||
| val unapplyAppCall = withMode(Mode.NoInline) { | ||
| typedExpr(untpd.TypedSplice(Apply(unapplyFn, dummyArg :: Nil))) |
There was a problem hiding this comment.
I prefer fewerBraces style instead, with a :. Did you consider that?
| * }.unapply(`dummy`)(using t1, ..) | ||
| * ``` | ||
| */ | ||
| def inlinedUnapplyFnAndApp(unapp: Tree): (Tree, Tree) = unapp match { |
There was a problem hiding this comment.
Consider moving this function further out, into the body of typedUnapply. It's a complicated operation for a special case, which makes it harder to follow the already complicated logic here.
There was a problem hiding this comment.
Moved out inlinedUnapplyFnAndApp. Also moved unapplyImplicits in a separate commit to cleanup the code even further.
|
Otherwise LGTM |
nicolasstucki
force-pushed
the
fix-19369
branch
from
3edd16b to
5648f12
Compare
nicolasstucki
force-pushed
the
fix-19369
branch
from
25d2ec1 to
535f230
Compare
) We needed to delay the inlining of the transparent inline when typing the unapply function application. We used the NoInline mode, but this also stopped the inlining of the arguments of the unapply. To fix this we target more precisely the inlining of the unapply method and not the implicit arguments. To do this we detect the dummy argument that is used type the unapply as an application, before it is transformed into a pattern. Fixes #19623 Fixes solution added in #19380
Backports #19380 to the LTS branch. PR submitted by the release tooling. [skip ci]
Fixes #19369