Skip to content

Simplify InterceptedMethods. Fix #439 #485

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
Apr 16, 2015

Conversation

DarkDimius
Copy link
Contributor

It's now a single pattern-match, instead of series of nested ifs.
review by @smarter

} /* else if (isPrimitiveValueClass(qual.tpe.typeSymbol)) {
lazy val Select(qual, _) = tree.fun
val Any_## = this.Any_##
val Any_!= = this.Any_!=
Copy link
Member

Choose a reason for hiding this comment

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

Why not:

val Any_## = defn.Any_##
val Any_!= = defn.Any_!=

@smarter
Copy link
Member

smarter commented Apr 16, 2015

The documentation for the class says:

  • x == y for == in class Any becomes x equals y with equals in class Object.

But this is no longer the case, this comment needs to be updated.

@smarter
Copy link
Member

smarter commented Apr 16, 2015

I wonder if poundPoundValue could be rewritten with applyOverloaded.

@DarkDimius
Copy link
Contributor Author

It could. But I would better leave it as is. We do not need full overload resolution(which could be resource-intensive)

* - `x == y` for == in class Any becomes `x equals y` with equals in class Object.
* - `x != y` for != in class Any becomes `!(x equals y)` with equals in class Object.
* - `x.##` for ## in other classes becomes calls to ScalaRunTime.hash,
* - `x != y` for != in class Any becomes `!(x != y)` with != in class Any.
Copy link
Member

Choose a reason for hiding this comment

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

Typo, should be !(x == y) with == in ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes. Thanks for noticing.
I'd better go and have some rest.

@smarter
Copy link
Member

smarter commented Apr 16, 2015

LGTM

DarkDimius added a commit that referenced this pull request Apr 16, 2015
@DarkDimius DarkDimius merged commit ad5b08c into scala:master Apr 16, 2015
@smarter smarter mentioned this pull request Apr 23, 2015
@allanrenucci allanrenucci deleted the interceptedM branch December 14, 2017 19:23
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