Skip to content

Go back to inlining during typing #5382

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

Closed
wants to merge 1 commit into from

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Nov 4, 2018

This is a tentative PR to move inlining back to typer. This will be needed if we want to address #5381.

Reverts the following commits:

Join containsQuotesOrSplices and containsInlineCalls (reverted from commit 8cf1385)
Make InlineCalls an object (reverted from commit ad26554)
Remove Inlined and InlineProxy from TASTy (reverted from commit 976e095)
Move inlining inside Reify quotes

Split some neg tests as now only the first error is emitted. (reverted from commit 21e65f1)
Erase rhs of erased non inlined val/def in PostTyper (reverted from commit c12e875)
Add isInlineCall to TreeInfo (reverted from commit 954c25b)
Update doc (reverted from commit 0832e02)
Only run InlineCalls if the tree contains an inline call (reverted from commit f28e2e1)
Check if unpickled tree has inline nodes (reverted from commit 927ae4e)
Add a bit of documentation (reverted from commit da0c249)
Fix constant folding during inlining (reverted from commit 32c8798)
Move inline β-reduction after Pickler (reverted from commit c68dc1f)
Normalize call at Inlined node creation (reverted from commit 746fdd7)
Move inline β-reduction after post typer (reverted from commit 432eb0a)
Move inline β-reduction out of typer (reverted from commit 6acaf31)

Reverts the following commits:

Join containsQuotesOrSplices and containsInlineCalls (reverted from commit 8cf1385)
Make InlineCalls an object (reverted from commit ad26554)
Remove Inlined and InlineProxy from TASTy (reverted from commit 976e095)
Move inlining inside Reify quotes

Split some neg tests as now only the first error is emitted. (reverted from commit 21e65f1)
Erase rhs of erased non inlined val/def in PostTyper (reverted from commit c12e875)
Add isInlineCall to TreeInfo (reverted from commit 954c25b)
Update doc (reverted from commit 0832e02)
Only run InlineCalls if the tree contains an inline call (reverted from commit f28e2e1)
Check if unpickled tree has inline nodes (reverted from commit 927ae4e)
Add a bit of documentation (reverted from commit da0c249)
Fix constant folding during inlining (reverted from commit 32c8798)
Move inline β-reduction after Pickler (reverted from commit c68dc1f)
Normalize call at Inlined node creation (reverted from commit 746fdd7)
Move inline β-reduction after post typer (reverted from commit 432eb0a)
Move inline β-reduction out of typer (reverted from commit 6acaf31)
* Only a reference to the toplevel class from which the call was inlined.
* @param call Info about the original call that was inlined
* Until PostTyper, this is the full call, afterwards only
* a reference to the toplevel class from which the call was inlined.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to keep the original call somewhere? This information is important:

  • for incremental compilation, because the parameters of an inlined call may introduce dependencies that are not visible in the resulting inlined code
  • for the IDE, because the user works with trees before inlining.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A potential problem is that this could increase bytecode size significantly. E.g. looking at the trace calls in the compiler, which surround large bodies of code. If we keep the call that code would be duplicated in the Tasty tree.

Currently, we get the calls for trees that are in an edit buffer, but not for others. So findReferences would only show those references that survive in the inlined code, but not other references that are not inlined.

We should think a bit more how we can reconcile these concerns.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding bytecode size: if we have several nested trace-like calls the blowup could be exponential. You can see this sometimes when you print at phase frontend without -Yshow-no-inlines.

@odersky odersky removed the stat:wip label Nov 4, 2018
@odersky odersky mentioned this pull request Nov 4, 2018
@odersky odersky requested a review from nicolasstucki November 8, 2018 14:39
@odersky
Copy link
Contributor Author

odersky commented Nov 8, 2018

Now included in #5383

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants