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

Correctly synthesize manifest[T] when T is an alias #6380

Merged
merged 1 commit into from Mar 8, 2018

Conversation

lrytz
Copy link
Member

@lrytz lrytz commented Mar 5, 2018

This

class A[T]
object A {
  type T = A[_]
  manifest[T]
}

crashed the compiler. Comparing the AST generated for manifest[T] with
the working version manifest[A[_]] shows that a cast is inserted to
the classOf argument in the latter case, but not the former.

For manifest[T]:

scala.Predef.manifest[A.T](
  scala.reflect.ManifestFactory.classType[A.T](
    classOf[A],
    ...

For manifest[A[_]]

scala.Predef.manifest[A[_]](
  scala.reflect.ManifestFactory.classType[A[_]](
    classOf[A].asInstanceOf[Class[A[_]]],
    ...

My approach for fixing this was simply to see what makes the compiler
insert the cast. The condition is here:

private def manifestOfType(tp: Type, flavor: Symbol): SearchResult = {
  ...
  def mot(tp0: Type, from: List[Symbol], to: List[Type]): SearchResult = {
    val tp1 = tp0.dealias
    ...
    val classarg = tp.dealias match {
      case _: ExistentialType => gen.mkCast(classarg0, ClassType(tp))
      case _                  => classarg0
    }

tp is A.T, the type alias. In the first call to mot, tp0 is
A.T. tp1 = tp0.dealias is an ExistentialType, so mot called
recursively with tp0 = tp1.skolemizeExistential.

A cast seems to be needed if the original type tp is an existential
(not sure why that is), but we need to dealias. Not sure if we should
cast to tp or tp.dealias, I guess it doesn't matter.

Fixes scala/bug#9155

This

    class A[T]
    object A {
      type T = A[_]
      manifest[T]
    }

crashed the compiler. Comparing the AST generated for `manifest[T]` with
the working version `manifest[A[_]]` shows that a cast is inserted to
the `classOf` argument in the latter case, but not the former.

For `manifest[T]`:

    scala.Predef.manifest[A.T](
      scala.reflect.ManifestFactory.classType[A.T](
        classOf[A],
        ...

For `manifest[A[_]]`

    scala.Predef.manifest[A[_]](
      scala.reflect.ManifestFactory.classType[A[_]](
        classOf[A].asInstanceOf[Class[A[_]]],
        ...

My approach for fixing this was simply to see what makes the compiler
insert the cast. The condition is here:

    private def manifestOfType(tp: Type, flavor: Symbol): SearchResult = {
      ...
      def mot(tp0: Type, from: List[Symbol], to: List[Type]): SearchResult = {
        val tp1 = tp0.dealias
        ...
        val classarg = tp.dealias match {
          case _: ExistentialType => gen.mkCast(classarg0, ClassType(tp))
          case _                  => classarg0
        }

`tp` is `A.T`, the type alias. In the first call to `mot`, `tp0` is
`A.T`. `tp1 = tp0.dealias` is an `ExistentialType`, so `mot` called
recursively with `tp0 = tp1.skolemizeExistential`.

A cast seems to be needed if the original type `tp` is an existential
(not sure why that is), but we need to dealias. Not sure if we should
cast to `tp` or `tp.dealias`, I guess it doesn't matter.
@lrytz lrytz changed the title Necessary cast in generated classOf[T] manifest argument Correctly synthesize manifest[T] when T is an alias Mar 8, 2018
@lrytz
Copy link
Member Author

lrytz commented Mar 8, 2018

@adriaanm updated the commit message.

@lrytz lrytz added prio:blocker release blocker (used only by core team, only near release time) and removed WIP labels Mar 8, 2018
@lrytz lrytz requested a review from adriaanm March 8, 2018 12:33
@lrytz lrytz self-assigned this Mar 8, 2018
Copy link
Contributor

@adriaanm adriaanm left a comment

Choose a reason for hiding this comment

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

💯

@adriaanm adriaanm merged commit 5017ab0 into scala:2.12.x Mar 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
prio:blocker release blocker (used only by core team, only near release time)
Projects
None yet
3 participants