From 525e8e7b8759c6201c4bfed56226d4b402d3fa4f Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Wed, 4 Dec 2019 12:08:41 +0100 Subject: [PATCH 1/3] Fix #7405: Allow types references at a lower level To be able to have references to type tags of types defined in a higher level we need to generalize the concept of type healing. In this case we instead of needing to keep a type tag for the next stage it should be possible to refer to types for a next level. This is already the case inside of top level splices in inline methods (macros) but it should be possible to do in any code. --- .../tools/dotc/transform/PCPCheckAndHeal.scala | 4 +++- .../dotty/tools/dotc/transform/Staging.scala | 3 ++- docs/docs/reference/metaprogramming/macros.md | 17 +++++++++-------- tests/pos/i7405.scala | 13 +++++++++++++ tests/pos/i7405b.scala | 18 ++++++++++++++++++ 5 files changed, 45 insertions(+), 10 deletions(-) create mode 100644 tests/pos/i7405.scala create mode 100644 tests/pos/i7405b.scala diff --git a/compiler/src/dotty/tools/dotc/transform/PCPCheckAndHeal.scala b/compiler/src/dotty/tools/dotc/transform/PCPCheckAndHeal.scala index 264319746933..449821c7a5d9 100644 --- a/compiler/src/dotty/tools/dotc/transform/PCPCheckAndHeal.scala +++ b/compiler/src/dotty/tools/dotc/transform/PCPCheckAndHeal.scala @@ -214,7 +214,7 @@ class PCPCheckAndHeal(@constructorOnly ictx: Context) extends TreeMapWithStages( assert(ctx.inInlineMethod) None } - else { + else if (levelOf(sym).getOrElse(0) < level) { val reqType = defn.QuotedTypeClass.typeRef.appliedTo(tp) val tag = ctx.typer.inferImplicitArg(reqType, pos.span) tag.tpe match { @@ -232,6 +232,8 @@ class PCPCheckAndHeal(@constructorOnly ictx: Context) extends TreeMapWithStages( | | The access would be accepted with an implict $reqType""") } + } else { + None } case _ => levelError(sym, tp, pos, "") diff --git a/compiler/src/dotty/tools/dotc/transform/Staging.scala b/compiler/src/dotty/tools/dotc/transform/Staging.scala index 929a2a056fce..5702a5975fa3 100644 --- a/compiler/src/dotty/tools/dotc/transform/Staging.scala +++ b/compiler/src/dotty/tools/dotc/transform/Staging.scala @@ -44,13 +44,14 @@ class Staging extends MacroTransform { case PackageDef(pid, _) if tree.symbol.owner == defn.RootClass => val checker = new PCPCheckAndHeal(freshStagingContext) { override protected def tryHeal(sym: Symbol, tp: Type, pos: SourcePosition)(implicit ctx: Context): Option[tpd.Tree] = { + def symStr = if (!tp.isInstanceOf[ThisType]) sym.show else if (sym.is(ModuleClass)) sym.sourceModule.show else i"${sym.name}.this" val errMsg = s"\nin ${ctx.owner.fullName}" - assert(false, + assert(levelOf(sym).getOrElse(0) >= level, em"""access to $symStr from wrong staging level: | - the definition is at level ${levelOf(sym).getOrElse(0)}, | - but the access is at level $level.$errMsg""") diff --git a/docs/docs/reference/metaprogramming/macros.md b/docs/docs/reference/metaprogramming/macros.md index 984a0550c794..7d527283d05a 100644 --- a/docs/docs/reference/metaprogramming/macros.md +++ b/docs/docs/reference/metaprogramming/macros.md @@ -161,14 +161,15 @@ different directions here for `f` and `x`. The reference to `f` is legal because it is quoted, then spliced, whereas the reference to `x` is legal because it is spliced, then quoted. -### Types and the PCP +### Lifting Types -In principle, The phase consistency principle applies to types as well -as for expressions. This might seem too restrictive. Indeed, the -definition of `reflect` above is not phase correct since there is a +Types are not directly affected by the phase consistency principle. +It is possible to use types defined at any level in any other level. +But, if a type is used in a subsequent stage it will need to be lifted to a `Type`. +The resulting value of `Type` will be subject to PCP. +Indeed, the definition of `reflect` above uses `T` in the next stage, there is a quote but no splice between the parameter binding of `T` and its -usage. But the code can be made phase correct by adding a binding -of a `Type[T]` tag: +usage. But the code can be rewritten by adding a binding of a `Type[T]` tag: ```scala def reflect[T, U](f: Expr[T] => Expr[U])(given t: Type[T]): Expr[T => U] = '{ (x: $t) => ${ f('x) } } @@ -177,8 +178,8 @@ In this version of `reflect`, the type of `x` is now the result of splicing the `Type` value `t`. This operation _is_ splice correct -- there is one quote and one splice between the use of `t` and its definition. -To avoid clutter, the Scala implementation tries to convert any phase-incorrect -reference to a type `T` to a type-splice, by rewriting `T` to `${ summon[Type[T]] }`. +To avoid clutter, the Scala implementation tries to convert any type +reference to a type `T` in subsequent phases to a type-splice, by rewriting `T` to `${ summon[Type[T]] }`. For instance, the user-level definition of `reflect`: ```scala diff --git a/tests/pos/i7405.scala b/tests/pos/i7405.scala new file mode 100644 index 000000000000..2f7d94d06151 --- /dev/null +++ b/tests/pos/i7405.scala @@ -0,0 +1,13 @@ +import scala.quoted._ +class Foo { + def f(given QuoteContext): Expr[Any] = { + '{ + type X = Int // Level 1 + val x: X = ??? + ${ + val t: Type[X] = '[X] // Level 0 + '{ val y: $t = x } + } + } + } +} diff --git a/tests/pos/i7405b.scala b/tests/pos/i7405b.scala new file mode 100644 index 000000000000..c90895bce3b4 --- /dev/null +++ b/tests/pos/i7405b.scala @@ -0,0 +1,18 @@ +import scala.quoted._ + +class Foo { + def f(given QuoteContext): Expr[Any] = { + '{ + trait X { + type Y + def y: Y = ??? + } + val x: X = ??? + type Z = x.Y + ${ + val t: Type[Z] = '[Z] + '{ val y: $t = x.y } + } + } + } +} From a6b8a986dd2bcf887f366c12c994b3233f251e2a Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Thu, 5 Dec 2019 13:56:14 +0100 Subject: [PATCH 2/3] Refactor tryHeal --- .../dotc/transform/PCPCheckAndHeal.scala | 56 ++++++++----------- .../dotty/tools/dotc/transform/Staging.scala | 9 +-- 2 files changed, 27 insertions(+), 38 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/PCPCheckAndHeal.scala b/compiler/src/dotty/tools/dotc/transform/PCPCheckAndHeal.scala index 449821c7a5d9..787c83347cec 100644 --- a/compiler/src/dotty/tools/dotc/transform/PCPCheckAndHeal.scala +++ b/compiler/src/dotty/tools/dotc/transform/PCPCheckAndHeal.scala @@ -176,7 +176,12 @@ class PCPCheckAndHeal(@constructorOnly ictx: Context) extends TreeMapWithStages( if (!sym.exists || levelOK(sym) || isStaticPathOK(tp) || isStaticNew(tp)) None else if (!sym.isStaticOwner && !isClassRef) - tryHeal(sym, tp, pos) + tp match + case tp: TypeRef => + if levelOf(sym).getOrElse(0) < level then tryHeal(sym, tp, pos) + else None + case _ => + levelError(sym, tp, pos, "") else if (!sym.owner.isStaticOwner) // non-top level class reference that is phase inconsistent levelError(sym, tp, pos, "") else @@ -202,42 +207,29 @@ class PCPCheckAndHeal(@constructorOnly ictx: Context) extends TreeMapWithStages( sym.is(Package) || sym.owner.isStaticOwner || levelOK(sym.owner) } - /** Try to heal phase-inconsistent reference to type `T` using a local type definition. + /** Try to heal reference to type `T` used in a higher level than its definition. * @return None if successful * @return Some(msg) if unsuccessful where `msg` is a potentially empty error message * to be added to the "inconsistent phase" message. */ - protected def tryHeal(sym: Symbol, tp: Type, pos: SourcePosition)(implicit ctx: Context): Option[Tree] = - tp match { - case tp: TypeRef => - if (level == -1) { - assert(ctx.inInlineMethod) - None - } - else if (levelOf(sym).getOrElse(0) < level) { - val reqType = defn.QuotedTypeClass.typeRef.appliedTo(tp) - val tag = ctx.typer.inferImplicitArg(reqType, pos.span) - tag.tpe match { - case _: TermRef => - Some(tag.select(tpnme.splice)) - case _: SearchFailureType => - levelError(sym, tp, pos, - i""" - | - | The access would be accepted with the right type tag, but - | ${ctx.typer.missingArgMsg(tag, reqType, "")}""") - case _ => - levelError(sym, tp, pos, - i""" - | - | The access would be accepted with an implict $reqType""") - } - } else { - None - } + protected def tryHeal(sym: Symbol, tp: TypeRef, pos: SourcePosition)(implicit ctx: Context): Option[Tree] = { + val reqType = defn.QuotedTypeClass.typeRef.appliedTo(tp) + val tag = ctx.typer.inferImplicitArg(reqType, pos.span) + tag.tpe match + case _: TermRef => + Some(tag.select(tpnme.splice)) + case _: SearchFailureType => + levelError(sym, tp, pos, + i""" + | + | The access would be accepted with the right type tag, but + | ${ctx.typer.missingArgMsg(tag, reqType, "")}""") case _ => - levelError(sym, tp, pos, "") - } + levelError(sym, tp, pos, + i""" + | + | The access would be accepted with a given $reqType""") + } private def levelError(sym: Symbol, tp: Type, pos: SourcePosition, errMsg: String)(given Context) = { def symStr = diff --git a/compiler/src/dotty/tools/dotc/transform/Staging.scala b/compiler/src/dotty/tools/dotc/transform/Staging.scala index 5702a5975fa3..0800eca3e598 100644 --- a/compiler/src/dotty/tools/dotc/transform/Staging.scala +++ b/compiler/src/dotty/tools/dotc/transform/Staging.scala @@ -43,15 +43,12 @@ class Staging extends MacroTransform { tree match { case PackageDef(pid, _) if tree.symbol.owner == defn.RootClass => val checker = new PCPCheckAndHeal(freshStagingContext) { - override protected def tryHeal(sym: Symbol, tp: Type, pos: SourcePosition)(implicit ctx: Context): Option[tpd.Tree] = { - + override protected def tryHeal(sym: Symbol, tp: TypeRef, pos: SourcePosition)(implicit ctx: Context): Option[tpd.Tree] = { def symStr = - if (!tp.isInstanceOf[ThisType]) sym.show - else if (sym.is(ModuleClass)) sym.sourceModule.show + if (sym.is(ModuleClass)) sym.sourceModule.show else i"${sym.name}.this" - val errMsg = s"\nin ${ctx.owner.fullName}" - assert(levelOf(sym).getOrElse(0) >= level, + assert(false, em"""access to $symStr from wrong staging level: | - the definition is at level ${levelOf(sym).getOrElse(0)}, | - but the access is at level $level.$errMsg""") From 8042c00f81de876ff562240f78a68933752c2fcb Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Fri, 10 Jan 2020 13:56:28 +0100 Subject: [PATCH 3/3] Fix #7887: Add regression test --- tests/pos/i7887.scala | 7 +++++++ tests/run-macros/i7887/Macro_1.scala | 17 +++++++++++++++++ tests/run-macros/i7887/Test_2.scala | 3 +++ 3 files changed, 27 insertions(+) create mode 100644 tests/pos/i7887.scala create mode 100644 tests/run-macros/i7887/Macro_1.scala create mode 100644 tests/run-macros/i7887/Test_2.scala diff --git a/tests/pos/i7887.scala b/tests/pos/i7887.scala new file mode 100644 index 000000000000..daeac328c5c5 --- /dev/null +++ b/tests/pos/i7887.scala @@ -0,0 +1,7 @@ +def typed[A](given t: quoted.Type[A], qctx: quoted.QuoteContext): Unit = { + import qctx.tasty.{given, _} + '{ + type T = $t + ${'{???}.cast[T]} + } +} diff --git a/tests/run-macros/i7887/Macro_1.scala b/tests/run-macros/i7887/Macro_1.scala new file mode 100644 index 000000000000..99a35135b52b --- /dev/null +++ b/tests/run-macros/i7887/Macro_1.scala @@ -0,0 +1,17 @@ +def myMacroImpl(a: quoted.Expr[_])(given qctx: quoted.QuoteContext) = { + import qctx.tasty.{_, given} + def typed[A] = { + implicit val t: quoted.Type[A] = a.unseal.tpe.widen.seal.asInstanceOf[quoted.Type[A]] + '{ + type T = $t + ${a.unseal.seal.cast[T]} + } + } + + typed +} + + +inline def myMacro(a: => Any) = ${ + myMacroImpl('a) +} diff --git a/tests/run-macros/i7887/Test_2.scala b/tests/run-macros/i7887/Test_2.scala new file mode 100644 index 000000000000..565f6f019fe4 --- /dev/null +++ b/tests/run-macros/i7887/Test_2.scala @@ -0,0 +1,3 @@ +object Test extends App { + assert(myMacro(42) == 42) +}