Skip to content

Fix #7048: Check for splice stability #8097

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

Merged
merged 2 commits into from
Jan 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions compiler/src/dotty/tools/dotc/transform/PCPCheckAndHeal.scala
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import dotty.tools.dotc.util.SourcePosition
import dotty.tools.dotc.util.Spans._
import dotty.tools.dotc.transform.SymUtils._
import dotty.tools.dotc.transform.TreeMapWithStages._
import dotty.tools.dotc.typer.Checking
import dotty.tools.dotc.typer.Implicits.SearchFailureType
import dotty.tools.dotc.typer.Inliner

Expand All @@ -31,7 +32,7 @@ import scala.annotation.constructorOnly
*
* Type healing consists in transforming a phase inconsistent type `T` into a splice of `implicitly[Type[T]]`.
*/
class PCPCheckAndHeal(@constructorOnly ictx: Context) extends TreeMapWithStages(ictx) {
class PCPCheckAndHeal(@constructorOnly ictx: Context) extends TreeMapWithStages(ictx) with Checking {
import tpd._

private val InAnnotation = Property.Key[Unit]()
Expand Down Expand Up @@ -212,7 +213,8 @@ class PCPCheckAndHeal(@constructorOnly ictx: Context) extends TreeMapWithStages(
val reqType = defn.QuotedTypeClass.typeRef.appliedTo(tp)
val tag = ctx.typer.inferImplicitArg(reqType, pos.span)
tag.tpe match
case _: TermRef =>
case tp: TermRef =>
checkStable(tp, pos)
Some(tag.select(tpnme.splice))
case _: SearchFailureType =>
levelError(sym, tp, pos,
Expand Down
1 change: 1 addition & 0 deletions compiler/test/dotc/pos-test-pickling.blacklist
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ t3486
t3612.scala
reference
scala-days-2019-slides
i7048e.scala

# Stale symbol: package object scala
seqtype-cycle
Expand Down
39 changes: 39 additions & 0 deletions tests/neg/i7048e.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import scala.quoted.{given, _}

abstract class Test {
type T

val T: Type[T]
val getT: Type[T] = T // need this to avoid getting `null`
given Type[T] = getT

def foo with QuoteContext: Expr[Any] = {

val r = '{Option.empty[T]} // error

{
val t: Test = this
import t.{given}
println(summon[Type[t.T]].show)
// val r = '{Option.empty[t.T]} // access to value t from wrong staging level
val r2 = '{Option.empty[${t.T}]} // works
}

{
val r1 = '{Option.empty[${T}]} // works
val r2 = '{Option.empty[List[${T}]]} // works
// val r3 = '{summon[Type[${T}]]} // access to Test.this from wrong staging level
val r4 = '{summon[${T} <:< Any]} // error
}

{
val s = '{Option.empty[${T}]}
val r = '{identity($s)} // works
val r2 = '{identity(${s: Expr[Option[T]]})} // error // error
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not obvious why this is not accepted by the check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

32 |      val r2 = '{identity[Option[T]](${s: Expr[Option[T]]})} // error // error
   |                                     ^^^^^^^^^^^^^^^^^^^^^
   |    (Test.this.given_Type_T : => quoted.Type[Test.this.T]) is not stable

All these can be fixed by making the given stable

val getT: Type[T] = T // need this to avoid getting `null`
given getT.type = getT

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added an extra test for it

Copy link
Contributor

Choose a reason for hiding this comment

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

Why it has to be stable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Type splices require a stable prefix to be correctly encoded. It also avoids having to deal with side effects in the transformation performed in ReifyQuotes. Most code receives the quoted type as a parameter and hence is stable.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's a restriction in TASTy? This is unprecedented in Scala, as usually only the path in types needs to be stable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a path in a type. A type that is inferred and then healed.

}

r
}
}

@main def main = ()
39 changes: 39 additions & 0 deletions tests/pos/i7048e.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import scala.quoted.{given, _}

abstract class Test {
type T

val T: Type[T]
val getT: Type[T] = T // need this to avoid getting `null`
given getT.type = getT

def foo with QuoteContext: Expr[Any] = {

val r = '{Option.empty[T]}

{
val t: Test = this
import t.{given}
println(summon[Type[t.T]].show)
// val r = '{Option.empty[t.T]} // access to value t from wrong staging level
val r2 = '{Option.empty[${t.T}]}
}

{
val r1 = '{Option.empty[${T}]} // works
val r2 = '{Option.empty[List[${T}]]} // works
// val r3 = '{summon[Type[${T}]]} // access to Test.this from wrong staging level
val r4 = '{summon[${T} <:< Any]}
}

{
val s = '{Option.empty[${T}]}
val r = '{identity($s)} // works
val r2 = '{identity(${s: Expr[Option[T]]})}
}

r
}
}

@main def main = ()