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

Unsound use of ClassTag with arrays in Reflection interface #8993

Closed
nicolasstucki opened this issue May 18, 2020 · 2 comments
Closed

Unsound use of ClassTag with arrays in Reflection interface #8993

nicolasstucki opened this issue May 18, 2020 · 2 comments
Assignees
Milestone

Comments

@nicolasstucki
Copy link
Contributor

scala.tasty.Reflection provides given instances of ClassTag to be able to pattern match (ClassTag.unapply).
But if they are used those as ClassTag (i.e. for arrays) we hit an unsoundness when type types in Reflection are implemented with the same class.
The solution is not to give implicit ClassTags for those types, or only give one for the root type (Tree, Type, Position, ...).
Therefore the ClassTag concept should be separate from the type testing logic (see #7555).

Here is a minimal self contained example of how this unsoundness can be triggered

package x

@main def Foo = {
  val refl: Refl = new ReflImpl
  import refl._
  val arr = Array[refl.Tree]()
  arr match
    case arr: Array[refl.Term] => throw new Exception() // some elements of the array may to be refl.Term
    case _ => // ok
}

trait Refl {
  type Tree
  type Term <: Tree
  def treeCT: scala.reflect.ClassTag[Tree]
  def termCT: scala.reflect.ClassTag[Term]
  given scala.reflect.ClassTag[Tree] = treeCT
  given scala.reflect.ClassTag[Term] = termCT
}


object ReflImpl {
  class Tree
  def treeCT: scala.reflect.ClassTag[Tree] = summon[scala.reflect.ClassTag[Tree]]
}
class ReflImpl extends Refl {
  type Tree = ReflImpl.Tree
  type Term = ReflImpl.Tree
  def treeCT: scala.reflect.ClassTag[Tree] = ReflImpl.treeCT
  def termCT: scala.reflect.ClassTag[Term] = ReflImpl.treeCT
}
@nicolasstucki
Copy link
Contributor Author

Requires a fix for #7554. #7555 would allow fixing this issue

@nicolasstucki
Copy link
Contributor Author

This was fixed

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

1 participant