-
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
Pickler
intoInlineCalls
phase which only runs if there is something to inline.