Skip to content

Inline fixes #4651

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 4 commits into from
Closed

Inline fixes #4651

wants to merge 4 commits into from

Conversation

allanrenucci
Copy link
Contributor

@allanrenucci allanrenucci commented Jun 12, 2018

Cherry picked the inline fixes from #4490

odersky added 3 commits June 12, 2018 18:47
 1. Fix inline accessors for static methods
 2. Disable variance checks for inline accessors
@allanrenucci
Copy link
Contributor Author

allanrenucci commented Jun 12, 2018

@odersky Do the included test cases exercise the inline fixes?

When reverting last commit, the tests fail. So the answer is yes

Copy link
Contributor Author

@allanrenucci allanrenucci left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM

@@ -70,6 +71,11 @@ abstract class AccessProxies {
def accessorNameKind: ClassifiedNameKind
def needsAccessor(sym: Symbol)(implicit ctx: Context): Boolean

def ifNoHost(reference: RefTree)(implicit ctx: Context): Tree = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is overridden in both ProtectedAccesors and Inliner, shouldn't it be abstract?

@odersky
Copy link
Contributor

odersky commented Jun 26, 2018

I think we should just merge #4490 instead. It improves what we have so far, and Lst is useless without it.

@allanrenucci
Copy link
Contributor Author

Closed in favor of #4490

@allanrenucci allanrenucci deleted the inline-fixes branch June 28, 2018 12:25
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