-
Notifications
You must be signed in to change notification settings - Fork 1.1k
TASTy reflect use complete definition trees #4968
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
Conversation
71202fc to
1b0297e
Compare
3736296 to
1e40bd1
Compare
1e40bd1 to
0fde9fa
Compare
|
Rebased |
f8c5b25 to
41bc57a
Compare
9aacd4c to
6446173
Compare
Currently only works with -Yretain-tree. Also fix an issue with TASTy reflect
6446173 to
0289c31
Compare
|
Rebased |
|
@odersky, now this PR used the |
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
| val Inlined: InlinedExtractor | ||
| abstract class InlinedExtractor { | ||
| def unapply(tree: Tree)(implicit ctx: Context): Option[(Option[Term], List[Definition], Term)] | ||
| def unapply(tree: Tree)(implicit ctx: Context): Option[(Option[Parent], List[Definition], Term)] |
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.
If I understand correctly, Parent is the type of the tree in the parent list. Using here, it's a little difficult to understand.
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.
Yes, I will change Parent to TermOrTypeTree in another PR. When bootstrapped it will be Term | TypeTree.
TASTy reflect trees are loaded by finding the tree from the current top-level class tree. This only works when
-Yretain-treesis enabled, otherwise the three is still synthesized.