Skip to content

Add missing owner for ValDef.let #10402

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.

Sign up for GitHub

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 1 commit into from
Nov 20, 2020
Merged

Add missing owner for ValDef.let #10402

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion community-build/community-projects/scalatest
Open in desktop
Submodule scalatest updated 2 files
+23 −23 scalactic.dotty/src/main/scala/org/scalactic/BooleanMacro.scala
+10 −10 scalatest.dotty/src/main/scala/org/scalatest/diagrams/DiagramsMacro.scala
10 changes: 6 additions & 4 deletions compiler/src/scala/quoted/internal/impl/QuoteContextImpl.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,13 +278,14 @@ class QuoteContextImpl private (ctx: Context) extends QuoteContext, QuoteUnpickl
def unapply(vdef: ValDef): Option[(String, TypeTree, Option[Term])] =
Some((vdef.name.toString, vdef.tpt, optional(vdef.rhs)))

def let(name: String, rhs: Term)(body: Ident => Term): Term =
val vdef = tpd.SyntheticValDef(name.toTermName, rhs)
def let(owner: Symbol, name: String, rhs: Term)(body: Ident => Term): Term =
val vdef = tpd.SyntheticValDef(name.toTermName, rhs)(using ctx.withOwner(owner))
val ref = tpd.ref(vdef.symbol).asInstanceOf[Ident]
Block(List(vdef), body(ref))

def let(terms: List[Term])(body: List[Ident] => Term): Term =
val vdefs = terms.map(term => tpd.SyntheticValDef("x".toTermName, term))
def let(owner: Symbol, terms: List[Term])(body: List[Ident] => Term): Term =
val ctx1 = ctx.withOwner(owner)
val vdefs = terms.map(term => tpd.SyntheticValDef("x".toTermName, term)(using ctx1))
val refs = vdefs.map(vdef => tpd.ref(vdef.symbol).asInstanceOf[Ident])
Block(vdefs, body(refs))
end ValDef
Expand Down Expand Up @@ -2203,6 +2204,7 @@ class QuoteContextImpl private (ctx: Context) extends QuoteContext, QuoteUnpickl
type Symbol = dotc.core.Symbols.Symbol

object Symbol extends SymbolModule:
def spliceOwner: Symbol = ctx.owner
def currentOwner(using ctx: Context): Symbol = ctx.owner
def requiredPackage(path: String): Symbol = dotc.core.Symbols.requiredPackage(path)
def requiredClass(path: String): Symbol = dotc.core.Symbols.requiredClass(path)
Expand Down
18 changes: 15 additions & 3 deletions library/src/scala/quoted/QuoteContext.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,13 +386,14 @@ trait QuoteContext { self: internal.QuoteUnpickler & internal.QuoteMatching =>
def unapply(vdef: ValDef): Option[(String, TypeTree, Option[Term])]

/** Creates a block `{ val <name> = <rhs: Term>; <body(x): Term> }` */
def let(name: String, rhs: Term)(body: Ident => Term): Term
def let(owner: Symbol, name: String, rhs: Term)(body: Ident => Term): Term

/** Creates a block `{ val x = <rhs: Term>; <body(x): Term> }` */
def let(rhs: Term)(body: Ident => Term): Term = let("x", rhs)(body)
def let(owner: Symbol, rhs: Term)(body: Ident => Term): Term =
let(owner, "x", rhs)(body)

/** Creates a block `{ val x1 = <terms(0): Term>; ...; val xn = <terms(n-1): Term>; <body(List(x1, ..., xn)): Term> }` */
def let(terms: List[Term])(body: List[Ident] => Term): Term
def let(owner: Symbol, terms: List[Term])(body: List[Ident] => Term): Term
}

given ValDefMethods as ValDefMethods = ValDefMethodsImpl
Expand Down Expand Up @@ -2673,6 +2674,17 @@ trait QuoteContext { self: internal.QuoteUnpickler & internal.QuoteMatching =>

trait SymbolModule { this: Symbol.type =>

/** Symbol of the definition that encloses the current splicing context.
*
* For example, the following call to `spliceOwner` would return the symbol `x`.
* ```
* val x = ${ ... Symbol.spliceOwner ... }
* ```
*
* For a macro splice, it is the symbol of the definition where the macro expansion happens.
*/
def spliceOwner: Symbol

Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: what about currentOwner? I find spliceOwner is not so easy to understand.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The current owner will eventually need to disappear if we use an explicit owner scheme. spliceOwner is the ower of the current splice (i.e. x in val x = ${ ... }; ...). This is the same as the owner of the current QuoteContext. But currentOwner would be a confusing name to use for this one as in the middle of a TreeMap the current owner is not the same as the owner of the QuoteContext.

/** Returns the symbol of the current enclosing definition */
def currentOwner(using ctx: Context): Symbol

Expand Down
6 changes: 3 additions & 3 deletions tests/pos-macros/i6535/Macro_1.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ object scalatest {

Term.of(cond).underlyingArgument match {
case t @ Apply(Select(lhs, op), rhs :: Nil) =>
let(lhs) { left =>
let(rhs) { right =>
let(Symbol.currentOwner, lhs) { left =>
let(Symbol.currentOwner, rhs) { right =>
val app = Select.overloaded(left, op, Nil, right :: Nil)
let(app) { result =>
let(Symbol.currentOwner, app) { result =>
val l = left.asExpr
val r = right.asExpr
val b = result.asExprOf[Boolean]
Expand Down
1 change: 1 addition & 0 deletions tests/pos-macros/i8866/Macro_1.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ object Macro {
import qctx.reflect._

ValDef.let(
Symbol.currentOwner,
Select.unique(
Term.of('{ OtherMacro }),
"apply"
Expand Down
1 change: 1 addition & 0 deletions tests/pos-macros/i8866b/Macro_1.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ object Macro {
import qctx.reflect._

ValDef.let(
Symbol.spliceOwner,
Select.unique(
Term.of('{ Other }),
"apply"
Expand Down
12 changes: 6 additions & 6 deletions tests/run-macros/i6171/Macro_1.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ object scalatest {

Term.of(cond).underlyingArgument match {
case t @ Apply(Select(lhs, op), rhs :: Nil) =>
ValDef.let(lhs) { left =>
ValDef.let(rhs) { right =>
ValDef.let(Symbol.spliceOwner, lhs) { left =>
ValDef.let(Symbol.spliceOwner, rhs) { right =>
val app = Select.overloaded(left, op, Nil, right :: Nil)
ValDef.let(app) { result =>
ValDef.let(Symbol.spliceOwner, app) { result =>
val l = left.asExpr
val r = right.asExpr
val b = result.asExprOf[Boolean]
Expand All @@ -28,10 +28,10 @@ object scalatest {
}.asExprOf[Unit]
case Apply(f @ Apply(Select(Apply(qual, lhs :: Nil), op), rhs :: Nil), implicits)
if isImplicitMethodType(f.tpe) =>
ValDef.let(lhs) { left =>
ValDef.let(rhs) { right =>
ValDef.let(Symbol.spliceOwner, lhs) { left =>
ValDef.let(Symbol.spliceOwner, rhs) { right =>
val app = Select.overloaded(Apply(qual, left :: Nil), op, Nil, right :: Nil)
ValDef.let(Apply(app, implicits)) { result =>
ValDef.let(Symbol.spliceOwner, Apply(app, implicits)) { result =>
val l = left.asExpr
val r = right.asExpr
val b = result.asExprOf[Boolean]
Expand Down
12 changes: 6 additions & 6 deletions tests/run-macros/reflect-dsl/assert_1.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ object scalatest {

Term.of(cond).underlyingArgument match {
case t @ Apply(sel @ Select(lhs, op), rhs :: Nil) =>
ValDef.let(lhs) { left =>
ValDef.let(rhs) { right =>
ValDef.let(Symbol.spliceOwner, lhs) { left =>
ValDef.let(Symbol.spliceOwner, rhs) { right =>
val app = left.select(sel.symbol).appliedTo(right)
ValDef.let(app) { result =>
ValDef.let(Symbol.spliceOwner, app) { result =>
val l = left.asExpr
val r = right.asExpr
val b = result.asExprOf[Boolean]
Expand All @@ -28,10 +28,10 @@ object scalatest {
}.asExprOf[Unit]
case Apply(f @ Apply(sel @ Select(Apply(qual, lhs :: Nil), op), rhs :: Nil), implicits)
if isImplicitMethodType(f.tpe) =>
ValDef.let(lhs) { left =>
ValDef.let(rhs) { right =>
ValDef.let(Symbol.spliceOwner, lhs) { left =>
ValDef.let(Symbol.spliceOwner, rhs) { right =>
val app = qual.appliedTo(left).select(sel.symbol).appliedTo(right)
ValDef.let(Apply(app, implicits)) { result =>
ValDef.let(Symbol.spliceOwner, Apply(app, implicits)) { result =>
val l = left.asExpr
val r = right.asExpr
val b = result.asExprOf[Boolean]
Expand Down
4 changes: 2 additions & 2 deletions tests/run-macros/reflect-pos-fun/assert_1.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ object scalatest {

Term.of(cond).underlyingArgument match {
case t @ Apply(TypeApply(Select(lhs, op), targs), rhs) =>
ValDef.let(lhs) { left =>
ValDef.let(rhs) { rs =>
ValDef.let(Symbol.spliceOwner, lhs) { left =>
ValDef.let(Symbol.spliceOwner, rhs) { rs =>
val app = Select.overloaded(left, op, targs.map(_.tpe), rs)
val b = app.asExprOf[Boolean]
Term.of('{ scala.Predef.assert($b) })
Expand Down
12 changes: 6 additions & 6 deletions tests/run-macros/reflect-select-constructor/assert_1.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ object scalatest {

Term.of(cond).underlyingArgument match {
case t @ Apply(Select(lhs, op), rhs :: Nil) =>
ValDef.let(lhs) { left =>
ValDef.let(rhs) { right =>
ValDef.let(Symbol.spliceOwner, lhs) { left =>
ValDef.let(Symbol.spliceOwner, rhs) { right =>
val app = Select.overloaded(left, op, Nil, right :: Nil)
ValDef.let(app) { result =>
ValDef.let(Symbol.spliceOwner, app) { result =>
val l = left.asExpr
val r = right.asExpr
val b = result.asExprOf[Boolean]
Expand All @@ -28,10 +28,10 @@ object scalatest {
}.asExprOf[Unit]
case Apply(f @ Apply(Select(Apply(qual, lhs :: Nil), op), rhs :: Nil), implicits)
if isImplicitMethodType(f.tpe) =>
ValDef.let(lhs) { left =>
ValDef.let(rhs) { right =>
ValDef.let(Symbol.spliceOwner, lhs) { left =>
ValDef.let(Symbol.spliceOwner, rhs) { right =>
val app = Select.overloaded(Apply(qual, left :: Nil), op, Nil, right :: Nil)
ValDef.let(Apply(app, implicits)) { result =>
ValDef.let(Symbol.spliceOwner, Apply(app, implicits)) { result =>
val l = left.asExpr
val r = right.asExpr
val b = result.asExprOf[Boolean]
Expand Down
12 changes: 6 additions & 6 deletions tests/run-macros/reflect-select-copy-2/assert_1.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ object scalatest {

Term.of(cond).underlyingArgument match {
case Apply(sel @ Select(lhs, op), rhs :: Nil) =>
ValDef.let(lhs) { left =>
ValDef.let(rhs) { right =>
ValDef.let(Apply(Select.copy(sel)(left, op), right :: Nil)) { result =>
ValDef.let(Symbol.spliceOwner, lhs) { left =>
ValDef.let(Symbol.spliceOwner, rhs) { right =>
ValDef.let(Symbol.spliceOwner, Apply(Select.copy(sel)(left, op), right :: Nil)) { result =>
val l = left.asExpr
val r = right.asExpr
val b = result.asExprOf[Boolean]
Expand All @@ -27,9 +27,9 @@ object scalatest {
}.asExprOf[Unit]
case Apply(f @ Apply(sel @ Select(Apply(qual, lhs :: Nil), op), rhs :: Nil), implicits)
if isImplicitMethodType(f.tpe) =>
ValDef.let(lhs) { left =>
ValDef.let(rhs) { right =>
ValDef.let(Apply(Apply(Select.copy(sel)(Apply(qual, left :: Nil), op), right :: Nil), implicits)) { result =>
ValDef.let(Symbol.spliceOwner, lhs) { left =>
ValDef.let(Symbol.spliceOwner, rhs) { right =>
ValDef.let(Symbol.spliceOwner, Apply(Apply(Select.copy(sel)(Apply(qual, left :: Nil), op), right :: Nil), implicits)) { result =>
val l = left.asExpr
val r = right.asExpr
val b = result.asExprOf[Boolean]
Expand Down
12 changes: 6 additions & 6 deletions tests/run-macros/reflect-select-symbol-constructor/assert_1.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ object scalatest {

Term.of(cond).underlyingArgument match {
case t @ Apply(sel @ Select(lhs, op), rhs :: Nil) =>
ValDef.let(lhs) { left =>
ValDef.let(rhs) { right =>
ValDef.let(Symbol.spliceOwner, lhs) { left =>
ValDef.let(Symbol.spliceOwner, rhs) { right =>
val app = Apply(Select(left, sel.symbol), right :: Nil)
ValDef.let(app) { result =>
ValDef.let(Symbol.spliceOwner, app) { result =>
val l = left.asExpr
val r = right.asExpr
val b = result.asExprOf[Boolean]
Expand All @@ -28,10 +28,10 @@ object scalatest {
}.asExprOf[Unit]
case Apply(f @ Apply(sel @ Select(Apply(qual, lhs :: Nil), op), rhs :: Nil), implicits)
if isImplicitMethodType(f.tpe) =>
ValDef.let(lhs) { left =>
ValDef.let(rhs) { right =>
ValDef.let(Symbol.spliceOwner, lhs) { left =>
ValDef.let(Symbol.spliceOwner, rhs) { right =>
val app = Apply(Select(Apply(qual, left :: Nil), sel.symbol), right :: Nil)
ValDef.let(Apply(app, implicits)) { result =>
ValDef.let(Symbol.spliceOwner, Apply(app, implicits)) { result =>
val l = left.asExpr
val r = right.asExpr
val b = result.asExprOf[Boolean]
Expand Down
12 changes: 6 additions & 6 deletions tests/run-macros/reflect-select-value-class/assert_1.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ object scalatest {

Term.of(cond).underlyingArgument match {
case t @ Apply(Select(lhs, op), rhs :: Nil) =>
ValDef.let(lhs) { left =>
ValDef.let(rhs) { right =>
ValDef.let(Symbol.spliceOwner, lhs) { left =>
ValDef.let(Symbol.spliceOwner, rhs) { right =>
val app = Select.overloaded(left, op, Nil, right :: Nil)
ValDef.let(app) { result =>
ValDef.let(Symbol.spliceOwner, app) { result =>
val l = left.asExpr
val r = right.asExpr
val b = result.asExprOf[Boolean]
Expand All @@ -28,10 +28,10 @@ object scalatest {
}.asExprOf[Unit]
case Apply(f @ Apply(Select(Apply(qual, lhs :: Nil), op), rhs :: Nil), implicits)
if isImplicitMethodType(f.tpe) =>
ValDef.let(lhs) { left =>
ValDef.let(rhs) { right =>
ValDef.let(Symbol.spliceOwner, lhs) { left =>
ValDef.let(Symbol.spliceOwner, rhs) { right =>
val app = Select.overloaded(Apply(qual, left :: Nil), op, Nil, right :: Nil)
ValDef.let(Apply(app, implicits)) { result =>
ValDef.let(Symbol.spliceOwner, Apply(app, implicits)) { result =>
val l = left.asExpr
val r = right.asExpr
val b = result.asExprOf[Boolean]
Expand Down
2 changes: 1 addition & 1 deletion tests/run-macros/tasty-unsafe-let/quoted_1.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ object Macros {
val rhsTerm = Term.of(rhs)

import qctx.reflect._
ValDef.let(rhsTerm) { rhsId =>
ValDef.let(Symbol.spliceOwner, rhsTerm) { rhsId =>
Term.of(Expr.betaReduce('{$body(${rhsId.asExpr.asInstanceOf[Expr[T]]})})) // Dangerous uncheked cast!
}.asExprOf[Unit]
}
Expand Down