Skip to content

Fix #2698: Move eqXYZ instances to Eq #2786

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jun 23, 2017
Merged

Fix #2698: Move eqXYZ instances to Eq #2786

merged 3 commits into from
Jun 23, 2017

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Jun 20, 2017

No description provided.

@dwijnand
Copy link
Member

I think one might want to opt-out of more than just the eqAny implicit, for instance the numeric type Eqs.

If the root issue is auto-completion pollution, then perhaps a solution that doesn't affect the ability to opt-out is to give them more contorted names. Maybe something along the lines of $dottyEqByteNum.

@odersky
Copy link
Contributor Author

odersky commented Jun 22, 2017 •
edited
Loading

I think we are going to move everything back to Eq. The problem of having eqAny in Predef is that it will always be tried first and succeed. Then there's a sub-search whether its operands support Eq, which will also succeed . Only after going through 2 or 3 full implicit searches will we conclude that eqAny is not applicable, and will start to search for something more specific.

If we move eqAny back to Eq the problem goes away because we find it then through the implicit scope, together with all other choices, and we eliminate it immediately, because it is less specific.

This means that unimporting cannot be used to disable eqAny. We can think about other ways to achieve the same effect, though.

odersky added 2 commits June 22, 2017 19:26
@odersky
Reorganize Eq checking
adec842 
Two main changes:

 - We treat `Eq` as coherent, i.e Eq searches are never ambiguous.
   Instead, the first found instance is returned. This would be
   compatible with an eventual user-declarable coherence construct,
   but does not require it.

 - Instead of invalidating found `eqAny` instances after the fact,
   we turn things around: `eqAny` is no longer defined `implicit`,
   but can nevertheless me synthesized for validEqAny` args, similar
   to how `ClassTag` is synthesized. This simplifies the logic and
   leads to better debugability of implicit searches.
@odersky
Update reference
b8b201d
@odersky odersky mentioned this pull request Jun 23, 2017
Closed
@odersky
Copy link
Contributor Author

odersky commented Jun 23, 2017

@OlivierBlanvillain Can you take another look? I did a lot of changes after your last review.

OlivierBlanvillain
OlivierBlanvillain reviewed Jun 23, 2017
View reviewed changes
Copy link
Contributor

@OlivierBlanvillain OlivierBlanvillain left a comment

Choose a reason for hiding this comment

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

I'm fine with the changes, I would be in favor of strict defaults (and an import language.loseEquality), but I guess that discussion is not directly related to the changes of this PR. LGTM!

@odersky odersky merged commit 73618f8 into scala:master Jun 23, 2017
@allanrenucci allanrenucci deleted the fix-#2698 branch December 14, 2017 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@OlivierBlanvillain OlivierBlanvillain OlivierBlanvillain approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@odersky @dwijnand @OlivierBlanvillain