-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Move inline β-reduction out of typer #5216
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
Move inline β-reduction out of typer #5216
Conversation
The issue is that we created bindings of the form `val xyz_this: => XYZ = ...`
246df88 to
4b9cc39
Compare
4b9cc39 to
432eb0a
Compare
7ff4e80 to
c68dc1f
Compare
63c8038 to
32c8798
Compare
3c9f76c to
8543a33
Compare
456f075 to
db7e183
Compare
|
test performance please |
|
performance test scheduled: 1 job(s) in queue, 0 running. |
|
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/5216/ to see the changes. Benchmarks is based on merging with master (7463afe) |
db7e183 to
927ae4e
Compare
When inlining a `this` binding that referes to a result typed with a match type, this type must be normalized and used.
0101eb6 to
c6c5389
Compare
Previously the `implicitly[C]` was refining its return type of `ev` by inlining. As we are now inlining after typer we refine the return type explicitly to `ev.type`.
4c4733c to
0832e02
Compare
|
test performance please |
|
performance test scheduled: 1 job(s) in queue, 0 running. |
|
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/5216/ to see the changes. Benchmarks is based on merging with master (7a45a4a) |
6eebf53 to
894ecee
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.
If no Inlined nodes can appear before InlineCalls, I think that you can also remove some cases from the sbt phases.
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 would argue that the rhs of erased types should be erased in PostTyper (and don't need an inline expansion either). Otherwise LGTM.
| /** Transforms the rhs tree into a its default tree if it is in an `erased` val/def. | ||
| * Performed to shrink the tree that is known to be erased later. | ||
| */ | ||
| private def normalizeErasedRhs(rhs: Tree, sym: Symbol)(implicit ctx: Context) = |
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.
What happens to erased right hand sides now? Are they kept? Why can't they be dropped here?
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.
The right-hand sides are kept as they were before. Implicit arguments to erased methods are removed early (as (???: T)).
This logic was only droping nodes that where inlined in an erased rhs, the rest was unaffected.
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 tried to erase the rhs of erased val/def that are not inline. But YCheck complained on most erased tests. I will investigate further.
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 managed to implement it, now rhs of erased non inlined methods are removed in PostTyper
8d3c7a2 to
444943a
Compare
6d9474b to
c12e875
Compare
|
@odersky the last commit erases the RHS of erased def/val in PostTyper. |
Move inlining after
PicklerintoInlineCallsphase which only runs if there is something to inline.