Skip to content

Fix #3636: Better disambiguation in withPrefix #3656

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

Merged
merged 1 commit into from
Dec 12, 2017

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Dec 11, 2017

Use the kind of disambiguation in withPrefix that we apply already in memberDenot.

Use the kind of disambiguation in withPrefix that we
apply already in memberDenot.
@odersky odersky requested a review from liufengyun December 11, 2017 22:36
Copy link
Contributor

@liufengyun liufengyun left a comment

Choose a reason for hiding this comment

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

LGTM.

There's one thing I don't quite understand: is it safe to do overloading resolution for inlined names? Is it possible that an inlined name during overloading resolution gets bound to another symbol?

@odersky
Copy link
Contributor Author

odersky commented Dec 12, 2017

There's one thing I don't quite understand: is it safe to do overloading resolution for inlined names? Is it possible that an inlined name during overloading resolution gets bound to another symbol?

For inlining, they are always the same symbol, since the new prefix has the same widened type as the original one. But the mechanism we are using for inlining (TreeTypeMaps, which use in turn withPrefix) can rebind symbols. We could have fixed the problem by fixing the symbol explicitly when inlining. But I thought it was better to make the underlying mechanism better at disambiguating.

@odersky odersky merged commit 25e53b0 into scala:master Dec 12, 2017
@allanrenucci allanrenucci deleted the fix-#3636 branch December 12, 2017 07:59
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