-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Refcatorings in TASTy reflection interface #5398
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
Refcatorings in TASTy reflection interface #5398
Conversation
83285a2
to
e8c6db5
Compare
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.
Nice refactoring 👍
If we can rename ReflectionCore
to something simpler, that would be better.
Can we discuss these renames during the next meeting? It would be nice to reach a consensus and then try to avoid breaking the reflect API in the future. I am personally not convinced by the following:
|
|
|
What would be simpler than |
@nicolasstucki I was thinking something like |
|
11711ed
to
0493c53
Compare
Aslo add `showCode` on Tree, TypeTree, ...
Remove the need to handle path dependent tasty.Reflection
689c3fd
to
876118e
Compare
Rebased |
Remove the need to handle path dependent tasty.Reflection
876118e
to
030adb4
Compare
Fix conflicts after rebase |
I should merge this before the meeting to get unblocked. WDYT @liufengyun. If needed we can rename methods later. |
FYI people seem to be familiar with the |
This PR focuse on refactorings that have been on hold to avoid rebasing other PRs. Now is a good moment to do it as other PRs won't be too hard to rebase on this.
tasty.Tasty
totasty.Reflection
toTasty/toExpr
(quoted to tasty trees / tasty trees to quoted) to relfect/reify in thetasty.Reflection
interfaceShow
toPrinter
showCode
to tasty treesutil.{TreeAcumulator, TreeTraverser}
to not need and explicit singleton type parameter ([T <: Tasty with Singleton](tasty: T)
). Move them insidetasty.Reflection
.