Skip to content

Fix/#569 new ref array #572

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
May 21, 2015
Merged

Fix/#569 new ref array #572

merged 3 commits into from
May 21, 2015

Conversation

odersky
Copy link
Contributor

@odersky odersky commented May 15, 2015

Review by @DarkDimius

@DarkDimius
Copy link
Contributor

Why is this fix better? AFAIK, this is the first time we break refference equality on symbols.

@DarkDimius
Copy link
Contributor

Ohh. I get it now. It's to make sure that nobody actually will do anything with symbol that originates from source.

Wouldn't this create problems, if we compile dotty itself with Arrays.scala and have plenty of types withFixedSym holding old symbol?

@odersky
Copy link
Contributor Author

odersky commented May 15, 2015

I don't think so. newRefArray is a dummy. References to the original symbol should kept as they are because the backend will have to interprete them specially, and eliminate the reference to newRefArray. I tried to simply drop the definition of newRefArray in erasure altogether, but that broke bridge generation. I think the current version is the least invasive fix.

@scala-jenkins
Copy link

Can one of the admins verify this patch?

odersky added 3 commits May 21, 2015 09:51
@odersky @DarkDimius
Fix scala#569 - newRefArray
07f346e 
newRefArray needs to be treated specially at and after erasure because
it is the only source-defined method that's still polymoprhic.
@odersky @DarkDimius
Better fix for $569
d01a0c9 
We now provide a ndew symbol for the `newRefArray` definition, which makes
any further treatment after erasure unnecessary.
@odersky @DarkDimius
Fix comment
36ab621
@DarkDimius DarkDimius force-pushed the fix/#569-newRefArray branch from ddbc1d1 to 36ab621 Compare May 21, 2015 07:51
@DarkDimius
Copy link
Contributor

/rebuild

DarkDimius added a commit that referenced this pull request May 21, 2015
@DarkDimius
Merge pull request #572 from dotty-staging/fix/#569-newRefArray
ca07c71 
Fix/#569 new ref array
@DarkDimius DarkDimius merged commit ca07c71 into scala:master May 21, 2015
@allanrenucci allanrenucci deleted the fix/#569-newRefArray branch December 14, 2017 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
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 @DarkDimius @scala-jenkins