-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix #14319: Use correct stat context when transforming Block #14411
Conversation
|
IDK but there are default args in self invocation at the akka failures.
is
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure what this is supposed to achieve. The previous fixes should already do the right thing. Maybe it's just ExtractAPI that needs to be fixed here.
A |
Thanks for explaining! Yes, I agree that makes sense. But it needs a different fix that does not append a list of statements on the right. And, one would need to troubleshoot why tests are failing. |
@odersky I will delete the changes to I will think about how to handle the last expr in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now LGTM.
@@ -1195,6 +1195,13 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { | |||
* - imports are reflected in the contexts of subsequent statements | |||
*/ | |||
class TreeMapWithPreciseStatContexts(cpy: TreeCopier = tpd.cpy) extends TreeMap(cpy): | |||
override def transform(tree: Tree)(using Context): Tree = tree match | |||
case Block(stats, expr) => | |||
val stats1 = transformStats(stats :+ expr, ctx.owner) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will lead to great tree node churn. Every block will get new statements on every transform, so everything containing a block will be copied, which means you get a complete copy of the AST for a macro transform. I don't think this is acceptable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is better to have an optional expr
argument in transformStats
. It's more convoluted but I don't see an alternative.
A better fix at #14523 |
Fix #14319
When transforming a Block, the last expression should have import information from statements above.