-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix JSR-45 repositioning #10801
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 JSR-45 repositioning #10801
Conversation
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'm glad we could fix this in time!
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.
LGTM
@@ -252,7 +253,8 @@ object Inliner { | |||
case tree: Bind => finalize(untpd.Bind(tree.name, transform(tree.body))(curSource)) | |||
case tree: TypeTree => finalize(tpd.TypeTree(tree.tpe)) | |||
case tree: DefTree => super.transform(tree).setDefTree | |||
case _ => super.transform(tree) | |||
case EmptyTree => tree | |||
case _ => fixSpan(super.transform(tree)) |
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 thought finalize
will ensure fixSpan
is called. Just curious if there is something to fix for TreeCopier
.
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.
Probably not. This kind of span/source change is only done here. When we will finally support JSR-45 we will remove all this code anyway.
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 was thinking about overridding def postProcess
in TreeCopier
:
class Reposition extends TreeMap(cpyWithNewSource) {
override val cpy: TypedTreeCopier = new TypedTreeCopier() {
protected def postProcess(tree: Tree, copied: untpd.Tree): copied.ThisTree[T] = ...
protected def postProcess(tree: Tree, copied: untpd.MemberDef): copied.ThisTree[T] = ...
}
}
No need to address in this PR.
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.
Maybe. I would not worry to much about this code and just port the support JSR-45 from Scala 2.
Alternative to #10799 that fixes the root cause of the issue.