Allow appliedToTypeTrees to operate on block expressions#26259
Allow appliedToTypeTrees to operate on block expressions#26259SolalPirelli wants to merge 1 commit into
appliedToTypeTrees to operate on block expressions#26259Conversation
| object toplevel { | ||
|
|
||
| @deprecated("pls work this out") | ||
| def ??? = scala.Predef.`???` |
There was a problem hiding this comment.
Can we at least remove dead code from the non-minimized test?
| if targs.isEmpty then tree else TypeApply(tree, targs) | ||
| if targs.isEmpty then tree else tree match | ||
| case Block(stmts, expr) if stmts.nonEmpty => Block(stmts, TypeApply(expr, targs)) | ||
| case _ => TypeApply(tree, targs) |
There was a problem hiding this comment.
This fix is suspicious. What if it's an If where the two branches need a type application? What is it about Blocks that deserves a special-case here, but not other constructs? Also why do this for type arguments but not term arguments?
If there is a specific call site that for some reason creates a Block, then that call site is probably the correct place to take care of it.
There was a problem hiding this comment.
I agree it's not an amazing fix, but the bit where a Block is created seems quite far from this, and I don't see a way to not create a Block there given the need for a temp local to use the argument twice (once as an arg, once as an arg of the default value function of the using)
There was a problem hiding this comment.
I'm not saying the call site should not create a Block. But when it creates a Block, it should make sure to push any type application inside the block itself.
There was a problem hiding this comment.
The Block is created in typedUnadapted, and this method here is called in adapt, both from the bit of typed that does adapt(typedUnadapted(...), ...).
I agree in principle we shouldn't have to add a special case here but I don't see how to avoid it without changing a lot of logic.
2 participants
Fixes #26238
See the -min test for an explanation of what's going on
How much have you relied on LLM-based tools in this contribution?
Not at all
How was the solution tested?
New automated tests (including the issue's reproducer, if applicable)