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

Type parameter ignored in selectDynamic invocation #6663

Closed
scabug opened this issue Nov 13, 2012 · 9 comments
Closed

Type parameter ignored in selectDynamic invocation #6663

scabug opened this issue Nov 13, 2012 · 9 comments
Assignees
Labels
Milestone

Comments

@scabug
Copy link

@scabug scabug commented Nov 13, 2012

As noticed by Dinko Srkoc in http://thread.gmane.org/gmane.comp.lang.scala.user/58246 sometimes the type parameter on a selectDynamic call gets lost.

With this definition:

import language.dynamics

class C(v: Any) extends Dynamic {
  def selectDynamic[T](n: String): Option[T] = Option(v.asInstanceOf[T])
  def applyDynamic[T](n: String)(): Option[T] = Option(v.asInstanceOf[T])
}

one gets this behaviour:

scala> new C(42).foo[Int].get
java.lang.ClassCastException: java.lang.Integer cannot be cast to scala.runtime.Nothing$

This happens because in Typers.scala, mkInvoke is called with the following cxTree:
new C.(42).foo[Int].get

Note that the outer element of that tree is a Select(..., "get"), and that's not handled by mkInvoke:

  val (outer, explicitTargs) = cxTree1 match {
    case TypeApply(fun, targs)              => (fun, targs)
    case Apply(TypeApply(fun, targs), args) => (Apply(fun, args), targs)
    case t                                  => (t, Nil)
  }

That can be fixed by adding another case:

  val (outer, explicitTargs) = cxTree1 match {
    case TypeApply(fun, targs)              => (fun, targs)
    case Apply(TypeApply(fun, targs), args) => (Apply(fun, args), targs)
    case Select(TypeApply(fun, targs), nme) => (Select(fun, nme), targs)
    case t                                  => (t, Nil)
  }

With that change, the examples work as expected:

scala> new C(42).foo[Int]().get
res0: Int = 42

scala> new C(42).foo[Int].get
res1: Int = 42 
@scabug
Copy link
Author

@scabug scabug commented Nov 13, 2012

Imported From: https://issues.scala-lang.org/browse/SI-6663?orig=1
Reporter: Jan Niehusmann (jannic)

@scabug
Copy link
Author

@scabug scabug commented Nov 13, 2012

@paulp said:
Nice job, you're almost there! Sending a pull request with the fix and a test case makes our lives tremendously easier.

@scabug
Copy link
Author

@scabug scabug commented Nov 14, 2012

Jan Niehusmann (jannic) said:
Maybe a duplicate of #6320

@scabug
Copy link
Author

@scabug scabug commented Nov 14, 2012

Jan Niehusmann (jannic) said:
There are similar cases not covered by the mentioned patch:

scala> new C(42).foo[String].get
java.lang.ClassCastException: java.lang.Integer cannot be cast to java.lang.String

scala> new C(42).foo[String].get : Int
res2: Int = 42

Here, the [String] parameter is ignored, again. This time because the cxTree starts with the type annotation: (new C.(42).foo[String].get: Int)

Is there an easy way to find everything which can possibly get attached to cxTree?

@scabug
Copy link
Author

@scabug scabug commented Nov 14, 2012

Jan Niehusmann (jannic) said:
scala/scala#1619

@scabug
Copy link
Author

@scabug scabug commented Nov 14, 2012

Jan Niehusmann (jannic) said:
I'd guess that there are other possible contents of cxTree not yet handled. Is there a better way to fix this than to add all such cases manually to mkInvoke?

@scabug
Copy link
Author

@scabug scabug commented Nov 14, 2012

Jan Niehusmann (jannic) said:
pull request against 2.10.x at scala/scala#1621

@scabug
Copy link
Author

@scabug scabug commented Nov 28, 2012

@paulp said:
That PR was merged, but this is not fixed in general; scala/scala#1681 should fix them all.

@scabug
Copy link
Author

@scabug scabug commented Dec 22, 2012

@paulp said:
edbcc64

@scabug scabug closed this Dec 22, 2012
@scabug scabug added this to the 2.10.1 milestone Apr 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants