-
Notifications
You must be signed in to change notification settings - Fork 1.1k
TASTY: PackageDef#unapply is unsafe #8521
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
Comments
@pshirshov could you provide a self-contained code to reproduce this issue. |
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)
}
}
}
}
???
}
} // Test.scala
class A
object Test {
def test = Foo.foo[A]
} [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 [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
|
I see it now. We are trying to access the owner of the
|
The solution for the original code is to use the |
PackageDef was originally added to try to encode trees in the API without the use of symbols. Now that we have symbols all the package def logic is redundant. At the time we assumed that we would be able to have a tree for all Symbol, but this has not been the case since the introduction of NoSymbol. Now we handle the `tree` of a package def the same way we did the one of a NoSymbol. * Remove `PackageDef` * Add `members` method on `Symbol` * Disallow package symbols in `Symbol.tree` as done with `NoSymbol`
PackageDef was originally added to try to encode trees in the API without the use of symbols. Now that we have symbols all the package def logic is redundant. At the time we assumed that we would be able to have a tree for all Symbol, but this has not been the case since the introduction of NoSymbol. Now we handle the `tree` of a package def the same way we did the one of a NoSymbol. * Remove `PackageDef` * Add `members` method on `Symbol` * Disallow package symbols in `Symbol.tree` as done with `NoSymbol`
Fix #8521: Remove PackageDef and use Symbol directly
Uh oh!
There was an error while loading. Please reload this page.
The following code would fail:
with exception like
Seems like it fails on root package which has no owner.
The text was updated successfully, but these errors were encountered: