Skip to content
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

Regression in raw types handling #7482

Closed
scabug opened this issue May 15, 2013 · 6 comments
Closed

Regression in raw types handling #7482

scabug opened this issue May 15, 2013 · 6 comments
Assignees
Milestone

Comments

@scabug
Copy link

scabug commented May 15, 2013

Caused by me in 9e101a3de8 .

% scala-hash 9e101a3de8

scala> val v: java.util.Vector[String] = new java.util.Vector[String](5)
v: java.util.Vector[String] = []

scala> val v: java.util.Vector[String] = new java.util.Vector[String](5)
<console>:7: error: type mismatch;
 found   : Int(5)
 required: java.util.Collection[_ <: String]
       val v: java.util.Vector[String] = new java.util.Vector[String](5)
                                                                      ^
@scabug
Copy link
Author

scabug commented May 15, 2013

Imported From: https://issues.scala-lang.org/browse/SI-7482?orig=1
Reporter: @paulp
Affected Versions: 2.11.0-M2

@scabug
Copy link
Author

scabug commented May 16, 2013

@retronym said:
Using ArrayList rather than Vector (which is only generic in Java 7+).

import scala.tools.partest.ReplTest

object Test extends ReplTest {
  override def code = """
  val v: java.util.ArrayList[String] = new java.util.ArrayList[String](5)
  val v: java.util.ArrayList[String] = new java.util.ArrayList[String](5)
  """
}

On the compilation of the second line, the symbol for the constructor [E](Int)ArrayList[E] has a corrupt info.

// unwanted existential!
TypeHistory(typer:7,(x$1: Int)java.util.ArrayList[_],null)
// an overloaded constuctor
TypeHistory(posterasure:5,(x$1: java.util.Collection)java.util.ArrayList,TypeHistory(namer:7,(x$1: java.util.Collection[_ <: E])java.util.ArrayList[E],null))

Overload resolution discounts the Int-accepting constructor, and ends up reporting "found: Int, required: Collection".

Here's a symptomatic fix:

https://github.com/retronym/scala/compare/ticket/7482

I'm going to look a bit harder for the underlying problem, but if I can't find it, I think we should go with this fix for 2.11.0-M3. modifyInfoConserve might still make sense on performance grounds.

@scabug
Copy link
Author

scabug commented May 16, 2013

@retronym said:
It seems that a call to cookJavaRawInfo during typechecking in erasure is responsible for the unwanted existential. It considers the erased result type of the constructor to be a raw, expands it to the existential, and then calls setInfo (via modifyInfo, which is rather a destructive thing to do, as it discards the symbols type history.

I still can't quite trace out why the old code was immune to this effect. But I can see a few ways that we can remedy this:

  • don't cook raw types if phase.erasedTypes. This seems like a clear win to me.
  • use updateInfo rather than modifyInfo. It seems like we should treat any calls to modify/setInfo with some suspicion. Are there cases when they are necessary for correctness?
  • Change modifyInfo (or make a variant) to be a no-op if the function doesn't transform the type.

@scabug
Copy link
Author

scabug commented May 17, 2013

@paulp said:
"don't cook raw types if phase.erasedTypes. This seems like a clear win to me"

Forget win, isn't it necessary for correctness? The raw type IS the type after erasure.

"Change modifyInfo (or make a variant) to be a no-op if the function doesn't transform the type."

Huh, I could swear I did that long ago. Must have died in a branch.

@scabug
Copy link
Author

scabug commented May 17, 2013

@paulp said:
Also, thanks a lot for looking at it, that information will save me a ton of time whatever happens.

@scabug
Copy link
Author

scabug commented May 17, 2013

@paulp said:
37884ec

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants