Skip to content

Fix #2858: Handle intersection selection when symbol only exists on one side #2861

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
Jul 13, 2017

Conversation

smarter
Copy link
Member

@smarter smarter commented Jul 13, 2017

No description provided.

@smarter smarter requested a review from odersky July 13, 2017 09:40
@smarter smarter force-pushed the fix-intersection branch 2 times, most recently from f014a12 to a784b4a Compare July 13, 2017 09:41
@smarter smarter changed the title Fix #2858: Handle intersection when symbol only exists on one side Fix #2858: Handle intersection selection when symbol only exists on one side Jul 13, 2017
@odersky
Copy link
Contributor

odersky commented Jul 13, 2017

Also, I think at least one, ad probably both tests on line 431 and 432 are now redundant and should be dropped.

* The aim of these criteria is to give some disambiguation on access which
* - does not depend on textual order or other arbitrary choices
* - minimizes raising of doubleDef errors
*/
def preferSym(sym1: Symbol, sym2: Symbol) =
sym1.eq(sym2) ||
(sym1.exists && !sym2.exists) ||
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can just say !sym2.exists. They are different, so sym1.exists is redundant.

@smarter smarter force-pushed the fix-intersection branch from a784b4a to ad1e2e8 Compare July 13, 2017 18:19
@smarter
Copy link
Member Author

smarter commented Jul 13, 2017

Also, I think at least one, ad probably both tests on line 431 and 432 are now redundant and should be dropped.

I was able to drop these tests after changing preferSym to always check sym1.exists before doing other tests, this should have the same behavior.

@odersky
Copy link
Contributor

odersky commented Jul 13, 2017

LGTM

@odersky odersky merged commit c933e0d into scala:master Jul 13, 2017
@allanrenucci allanrenucci deleted the fix-intersection branch December 14, 2017 19:20
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