Dotty: TASTY: PackageDef#unapply is unsafe

[pshirshov]

The following code would fail:

  private def packageToName(tree: Tree): List[String] = {
    tree match {
      case PackageDef(name, owner) =>
        packageToName(owner) ++ List(name)
      case o =>
        List.empty
    }
  }

with exception like

Error:(19, 33) Exception occurred while executing macro expansion.
java.lang.AssertionError: NoDenotation.owner
    at dotty.tools.dotc.core.SymDenotations$NoDenotation$.owner(SymDenotations.scala:2253)
    at dotty.tools.dotc.tastyreflect.ReflectionCompilerInterface.PackageDef_owner(ReflectionCompilerInterface.scala:177)
    at dotty.tools.dotc.tastyreflect.ReflectionCompilerInterface.PackageDef_owner(ReflectionCompilerInterface.scala:177)
    at scala.tasty.Reflection$PackageDefOps$.owner(Reflection.scala:615)
    at scala.tasty.Reflection$PackageDef$.unapply(Reflection.scala:621)
    at izumi.reflect.dottyreflection.Inspector.packageToName(Inspector.scala:147)
    at izumi.reflect.dottyreflection.Inspector.packageToName(Inspector.scala:148)
    at izumi.reflect.dottyreflection.Inspector.packageToName(Inspector.scala:148)
    at izumi.reflect.dottyreflection.Inspector.inspectSymbol(Inspector.scala:135)
    at izumi.reflect.dottyreflection.Inspector.prefixOf(Inspector.scala:158)
    at izumi.reflect.dottyreflection.Inspector.inspectSymbol(Inspector.scala:127)
    at izumi.reflect.dottyreflection.Inspector.inspectTree(Inspector.scala:119)
    at izumi.reflect.dottyreflection.Inspector.inspectSymbol(Inspector.scala:133)
    at izumi.reflect.dottyreflection.Inspector.prefixOf(Inspector.scala:158)
    at izumi.reflect.dottyreflection.Inspector.inspectSymbol(Inspector.scala:127)
    at izumi.reflect.dottyreflection.Inspector.inspectTree(Inspector.scala:119)
    at izumi.reflect.dottyreflection.Inspector.buildTypeRef(Inspector.scala:33)
    at izumi.reflect.dottyreflection.TypeInspections$.apply(TypeInspections.scala:10)
    at izumi.reflect.dottyreflection.Inspect$.inspectAny(LightTypeTagMaterializer.scala:21)
    val bazTag = Inspect.inspect[Baz]

Seems like it fails on root package which has no owner.

[nicolasstucki]

@pshirshov could you provide a self-contained code to reproduce this issue.

[griggt]

I was able to create a standalone reproducer for this issue:

// Main.scala
import scala.quoted._

object Foo {
  inline def foo[T <: AnyKind]: String = ${ bar[T] }

  def bar[T <: AnyKind : Type](using qctx: QuoteContext): Expr[String] = {
    import qctx.tasty.{Type => _, given _, _}

    def packageToName(tree: Tree): Unit = tree match {
      case PackageDef(_, owner) =>
        packageToName(owner)
    }

    val sym = implicitly[Type[T]].unseal.symbol
    if (!sym.isNoSymbol) {
      sym.tree match {
        case c: ClassDef =>
          if (!sym.maybeOwner.isNoSymbol) {
            sym.maybeOwner.tree match {
              case _: PackageDef =>
                packageToName(sym.maybeOwner.tree)
            }
          }
      }
    }

    ???
  }
}

```scala
// Test.scala
class A

object Test {
def test = Foo.foo[A]
}

```scala
[info] Compiling 2 Scala sources to /src/dotty-issues/i8521/standalone/target/scala-0.22/classes ...
[error] -- Error: /src/dotty-issues/i8521/standalone/src/main/scala/Test.scala:5:20 ----
[error] 5 |  def test = Foo.foo[A]
[error]   |             ^^^^^^^^^^
[error]   |Exception occurred while executing macro expansion.
[error]   |java.lang.AssertionError: NoDenotation.owner
[error]   |     at dotty.tools.dotc.core.SymDenotations$NoDenotation$.owner(SymDenotations.scala:2253)
[error]   |     at dotty.tools.dotc.tastyreflect.ReflectionCompilerInterface.PackageDef_owner(ReflectionCompilerInterface.scala:177)
[error]   |     at dotty.tools.dotc.tastyreflect.ReflectionCompilerInterface.PackageDef_owner(ReflectionCompilerInterface.scala:177)
[error]   |     at scala.tasty.Reflection$PackageDefOps$.owner(Reflection.scala:615)
[error]   |     at scala.tasty.Reflection$PackageDef$.unapply(Reflection.scala:621)
[error]   |     at Foo$.packageToName$1(Main.scala:11)
[error]   |     at Foo$.bar(Main.scala:22)
[error]   |
[error]   | This location contains code that was inlined from Test.scala:5
[error] one error found

The original bug appears to have been filed against 0.22.0-RC1. It still crashes in 0.28.0-bin-20200927-d084b8b-NIGHTLY:

[info] Compiling 2 Scala sources to /src/dotty-issues/i8521/standalone/target/scala-0.28/classes ...
[error] -- Error: /src/dotty-issues/i8521/standalone/src/main/scala/Test.scala:5:20 ----
[error] 5 |  def test = Foo.foo[A]
[error]   |             ^^^^^^^^^^
[error]   |Exception occurred while executing macro expansion.
[error]   |java.lang.AssertionError: NoDenotation.owner
[error]   |     at dotty.tools.dotc.core.SymDenotations$NoDenotation$.owner(SymDenotations.scala:2326)
[error]   |     at dotty.tools.dotc.quoted.QuoteContextImpl$tasty$SymbolMethodsImpl$.extension_owner(QuoteContextImpl.scala:2214)
[error]   |     at dotty.tools.dotc.quoted.QuoteContextImpl$tasty$SymbolMethodsImpl$.extension_owner(QuoteContextImpl.scala:2214)
[error]   |     at dotty.tools.dotc.quoted.QuoteContextImpl$tasty$PackageDefMethodsImpl$.extension_owner(QuoteContextImpl.scala:303)
[error]   |     at dotty.tools.dotc.quoted.QuoteContextImpl$tasty$PackageDefMethodsImpl$.extension_owner(QuoteContextImpl.scala:303)
[error]   |     at dotty.tools.dotc.quoted.QuoteContextImpl$tasty$PackageDef$.unapply(QuoteContextImpl.scala:298)
[error]   |     at dotty.tools.dotc.quoted.QuoteContextImpl$tasty$PackageDef$.unapply(QuoteContextImpl.scala:297)
[error]   |     at Foo$.packageToName$1(Main.scala:11)
[error]   |     at Foo$.bar(Main.scala:22)
[error]   |
[error]   | This location contains code that was inlined from Test.scala:5
[error] one error found

[nicolasstucki]

I see it now. We are trying to access the owner of the <empty package> or _root_ package which do not exist.

PackageDef is never a useful tree (PackageClause is the useful one), it was added to wrap around a package symbol to provide a package symbol like thing back when we tried to not have symbols. Now we can do all these operations on Symbol directly. It seems that the sensible thing to do is to remove the methods of PacakgeDef which basically leaves it a tree that does only contains a symbol reference. If we have that we should go all the way and just remove PackageDef. We would also need to add def members on Symbol.

[nicolasstucki]

The solution for the original code is to use the Symbol of the package instead and use maybeOwner