Skip to content

Fix/overloaded access #46

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

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Mar 6, 2014

Review by @DarkDimius @samuelgruetter

I apologize for the many commits. I have too much on my plate right now to spend time squashing them.

odersky added 6 commits March 6, 2014 10:30
Rewrap needs to produce alternatives with signatures. Otgerwise the new denotation will simply overwrite the old because both the overloaded TermRef and the alternative will hash to the same unique TermRef.
Overloaded TermRefs do not have an info, and consequently do not support =:=. Yet in Typer#checkNewOrShadowed we compared termrefs with =:=. This gives an exception if the termrefs are overloaded. The fix is to provide a new method isSameRef in TypeComparer which is called instead of =:= in Typer#checkNewOrShadowed.
@@ -139,8 +138,7 @@ class Typer extends Namer with Applications with Implicits {
}
val where = if (ctx.owner.exists) s" from ${ctx.owner.enclosingClass}" else ""
val whyNot = new StringBuffer
val addendum =
alts foreach (_.isAccessibleFrom(pre, superAccess, whyNot))
alts foreach (_.isAccessibleFrom(pre, superAccess, whyNot))
Copy link

Choose a reason for hiding this comment

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

So this is executed just for side-effects?

@odersky
Copy link
Contributor Author

odersky commented Mar 6, 2014

Yes, it's just for side effects. The allocation of a ListBuffer in the previous line hints to that.

@odersky
Copy link
Contributor Author

odersky commented Mar 6, 2014

I repushed the branch after fixing merge errors.

@@ -245,7 +243,7 @@ class Typer extends Namer with Applications with Implicits {
* does properly shadow the new one from an outer context.
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

I have trouble understanding how checkNewOrShadowed works. It seems there are two types: a "previously found result from an inner context", and a "new one from an outer context". One is called found, and the other is called previous, I guess. But which one is which? Both ways do not (yet ;-)) make sense for me...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

found is new one.

@odersky
Copy link
Contributor Author

odersky commented Mar 6, 2014

Closing pull request and integrating in a new one.

@odersky odersky closed this Mar 6, 2014
noti0na1 added a commit to noti0na1/dotty that referenced this pull request Dec 3, 2019
Disallow comparison between Object and Null
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.

3 participants