-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Properly encode splice hole using PolyFunction #17072
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
Changes from 1 commit
92ff00a
134152a
b931dd2
7c2d5e9
cf86861
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,6 +24,7 @@ import dotty.tools.dotc.config.ScalaRelease.* | |
import dotty.tools.dotc.staging.QuoteContext.* | ||
import dotty.tools.dotc.staging.StagingLevel.* | ||
import dotty.tools.dotc.staging.QuoteTypeTags | ||
import dotty.tools.dotc.staging.DirectTypeOf | ||
|
||
import scala.annotation.constructorOnly | ||
|
||
|
@@ -44,12 +45,13 @@ object Splicing: | |
* contains the quotes with references to all cross-quote references. There are some special rules | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the description a couple of lines above |
||
* for references in the LHS of assignments and cross-quote method references. | ||
* | ||
* In the following code example `x1` and `x2` are cross-quote references. | ||
* In the following code example `x1`, `x2` and `U` are cross-quote references. | ||
* ``` | ||
* '{ ... | ||
* val x1: T1 = ??? | ||
* val x2: T2 = ??? | ||
* ${ (q: Quotes) ?=> f('{ g(x1, x2) }) }: T3 | ||
* type U | ||
* val x1: T = ??? | ||
* val x2: U = ??? | ||
* ${ (q: Quotes) ?=> f('{ g[U](x1, x2) }) }: T3 | ||
* } | ||
* ``` | ||
* | ||
|
@@ -60,15 +62,15 @@ object Splicing: | |
* '{ ... | ||
* val x1: T1 = ??? | ||
* val x2: T2 = ??? | ||
* {{{ 0 | T3 | x1, x2 | | ||
* (x1$: Expr[T1], x2$: Expr[T2]) => // body of this lambda does not contain references to x1 or x2 | ||
* (q: Quotes) ?=> f('{ g(${x1$}, ${x2$}) }) | ||
* {{{ 0 | T3 | U, x1, x2 | | ||
* [U$1] => (U$2: Type[U$1], x1$: Expr[T], x2$: Expr[U$1]) => // body of this lambda does not contain references to U, x1 or x2 | ||
* (q: Quotes) ?=> f('{ @SplicedType type U$3 = [[[ 0 | U$2 | | U$1 ]]]; g[U$3](${x1$}, ${x2$}) }) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the [[[ ... ]]] syntax used here documented somewhere? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not really. It is the syntax defined in the |
||
* | ||
* }}} | ||
* } | ||
* ``` | ||
* | ||
* and then performs the same transformation on `'{ g(${x1$}, ${x2$}) }`. | ||
* and then performs the same transformation on `'{ @SplicedType type U$3 = [[[ 0 | U$2 | | U$1 ]]]; g[U$3](${x1$}, ${x2$}) }`. | ||
* | ||
*/ | ||
class Splicing extends MacroTransform: | ||
|
@@ -132,7 +134,7 @@ class Splicing extends MacroTransform: | |
case None => | ||
val holeIdx = numHoles | ||
numHoles += 1 | ||
val hole = tpd.Hole(false, holeIdx, Nil, ref(qual), TypeTree(tp)) | ||
val hole = tpd.Hole(false, holeIdx, Nil, ref(qual), TypeTree(tp.dealias)) | ||
typeHoles.put(qual.symbol, hole) | ||
hole | ||
cpy.TypeDef(tree)(rhs = hole) | ||
|
@@ -154,7 +156,7 @@ class Splicing extends MacroTransform: | |
|
||
private def transformAnnotations(tree: DefTree)(using Context): Unit = | ||
tree.symbol.annotations = tree.symbol.annotations.mapconserve { annot => | ||
val newAnnotTree = transform(annot.tree)(using ctx.withOwner(tree.symbol)) | ||
val newAnnotTree = transform(annot.tree) | ||
if (annot.tree == newAnnotTree) annot | ||
else ConcreteAnnotation(newAnnotTree) | ||
} | ||
|
@@ -198,10 +200,57 @@ class Splicing extends MacroTransform: | |
val newTree = transform(tree) | ||
val (refs, bindings) = refBindingMap.values.toList.unzip | ||
val bindingsTypes = bindings.map(_.termRef.widenTermRefExpr) | ||
val methType = MethodType(bindingsTypes, newTree.tpe) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The code example in the doc comment above SpliceTransformer also needs to be updated to use a polymorphic function. |
||
val types = bindingsTypes.collect { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's a lot of types involved in this code :), maybe we can use a more precise name like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Renamed to |
||
case AppliedType(tycon, List(arg: TypeRef)) if tycon.derivesFrom(defn.QuotedTypeClass) => arg | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think a dealias might be needed in case the user defines an alias such as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These types are created in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Even if they have the same shape, they might not be referentially equal (they are only referentially equal if they are cached by Uniques.scala, but not all types are cacheable). |
||
} | ||
val newTypeParams = types.map { tpe => | ||
newSymbol( | ||
spliceOwner, | ||
UniqueName.fresh(tpe.symbol.name.toTypeName), | ||
Param, | ||
TypeBounds.empty | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't the type bounds of the type params match the type bounds of the original type for the encoding to always typecheck? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems that I am missing something. I will add some test cases and update these bounds. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Speaking of bounds, if types can refer to terms in their bounds ( |
||
) | ||
} | ||
val methType = | ||
if types.nonEmpty then | ||
PolyType(types.map(tp => UniqueName.fresh(tp.symbol.name.toTypeName)))( | ||
pt => types.map(_ => TypeBounds.empty), | ||
pt => { | ||
val tpParamMap = new TypeMap { | ||
private val mapping = types.map(_.typeSymbol).zip(pt.paramRefs).toMap | ||
def apply(tp: Type): Type = tp match | ||
case tp: TypeRef => mapping.getOrElse(tp.typeSymbol, tp) | ||
case tp => mapOver(tp) | ||
} | ||
MethodType(bindingsTypes.map(tpParamMap), tpParamMap(newTree.tpe)) | ||
} | ||
) | ||
else MethodType(bindingsTypes, newTree.tpe) | ||
val meth = newSymbol(spliceOwner, nme.ANON_FUN, Synthetic | Method, methType) | ||
val ddef = DefDef(meth, List(bindings), newTree.tpe, newTree.changeOwner(ctx.owner, meth)) | ||
val fnType = defn.FunctionType(bindings.size, isContextual = false).appliedTo(bindingsTypes :+ newTree.tpe) | ||
|
||
def substituteTypes(tree: Tree): Tree = | ||
if types.nonEmpty then | ||
val typeIndex = types.zipWithIndex.toMap | ||
TreeTypeMap( | ||
typeMap = new TypeMap { | ||
def apply(tp: Type): Type = tp match | ||
case tp @ TypeRef(x: TermRef, _) if tp.symbol == defn.QuotedType_splice => tp | ||
case tp: TypeRef if tp.typeSymbol.hasAnnotation(defn.QuotedRuntime_SplicedTypeAnnot) => tp | ||
case tp: TypeRef => | ||
typeIndex.get(tp) match | ||
case Some(idx) => newTypeParams(idx).typeRef | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Having a Map where the keys are Types might be a little bit fragile since we could have two equivalent Types which are not referentially equal, is this not a problem here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These should only contain There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the prefix of a TypeRef could be an uncacheable type, for example if it contains a LazyRef |
||
case None => mapOver(tp) | ||
case _ => mapOver(tp) | ||
} | ||
).transform(tree) | ||
else tree | ||
val paramss = | ||
if types.nonEmpty then List(newTypeParams, bindings) | ||
else List(bindings) | ||
val ddef = substituteTypes(DefDef(meth, paramss, newTree.tpe, newTree.changeOwner(ctx.owner, meth))) | ||
val fnType = | ||
if types.isEmpty then defn.FunctionType(bindings.size, isContextual = false).appliedTo(bindingsTypes :+ newTree.tpe) | ||
else RefinedType(defn.PolyFunctionType, nme.apply, methType) | ||
val closure = Block(ddef :: Nil, Closure(Nil, ref(meth), TypeTree(fnType))) | ||
tpd.Hole(true, holeIdx, refs, closure, TypeTree(tpe)) | ||
|
||
|
@@ -255,6 +304,9 @@ class Splicing extends MacroTransform: | |
if tree.symbol == defn.QuotedTypeModule_of && containsCapturedType(tpt.tpe) => | ||
val newContent = capturedPartTypes(tpt) | ||
newContent match | ||
case DirectTypeOf.Healed(termRef) => | ||
// Optimization: `quoted.Type.of[@SplicedType type T = x.Underlying; T](quotes)` --> `x` | ||
tpd.ref(termRef).withSpan(tpt.span) | ||
case block: Block => | ||
inContext(ctx.withSource(tree.source)) { | ||
Apply(TypeApply(typeof, List(newContent)), List(quotes)).withSpan(tree.span) | ||
|
@@ -354,7 +406,7 @@ class Splicing extends MacroTransform: | |
private def newQuotedTypeClassBinding(tpe: Type)(using Context) = | ||
newSymbol( | ||
spliceOwner, | ||
UniqueName.fresh(nme.Type).toTermName, | ||
UniqueName.fresh(tpe.typeSymbol.name.toTermName), | ||
Param, | ||
defn.QuotedTypeClass.typeRef.appliedTo(tpe), | ||
) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -670,6 +670,9 @@ object TreeChecker { | |
else assert(tpt.typeOpt =:= pt) | ||
|
||
// Check that the types of the args conform to the types of the contents of the hole | ||
val typeArgsTypes = args.collect { case arg if arg.isType => | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will the new checker still work with tasty files generated by previous compiler versions? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TASTy files will not contain Holes so it will work. Pickled quotes will not contain |
||
arg.typeOpt | ||
} | ||
val argQuotedTypes = args.map { arg => | ||
if arg.isTerm then | ||
val tpe = arg.typeOpt.widenTermRefExpr match | ||
|
@@ -687,7 +690,29 @@ object TreeChecker { | |
val contextualResult = | ||
defn.FunctionOf(List(defn.QuotesClass.typeRef), expectedResultType, isContextual = true) | ||
val expectedContentType = | ||
defn.FunctionOf(argQuotedTypes, contextualResult) | ||
if typeArgsTypes.isEmpty then defn.FunctionOf(argQuotedTypes, contextualResult) | ||
else RefinedType(defn.PolyFunctionType, nme.apply, PolyType(typeArgsTypes.map(_ => TypeBounds.empty))(pt => | ||
val tpParamMap = new TypeMap { | ||
private val mapping = typeArgsTypes.map(_.typeSymbol).zip(pt.paramRefs).toMap | ||
def apply(tp: Type): Type = tp match | ||
case tp: TypeRef => mapping.getOrElse(tp.typeSymbol, tp) | ||
case tp => mapOver(tp) | ||
} | ||
MethodType( | ||
args.zipWithIndex.map { case (arg, idx) => | ||
if arg.isTerm then | ||
val tpe = arg.typeOpt.widenTermRefExpr match | ||
case _: MethodicType => | ||
// Special erasure for captured function references | ||
// See `SpliceTransformer.transformCapturedApplication` | ||
defn.AnyType | ||
case tpe => tpe | ||
defn.QuotedExprClass.typeRef.appliedTo(tpParamMap(tpe)) | ||
else defn.QuotedTypeClass.typeRef.appliedTo(tpParamMap(arg.typeOpt)) | ||
}, | ||
tpParamMap(contextualResult)) | ||
) | ||
) | ||
assert(content.typeOpt =:= expectedContentType, i"unexpected content of hole\nexpected: ${expectedContentType}\nwas: ${content.typeOpt}") | ||
|
||
tree1 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
import scala.quoted.* | ||
|
||
object Foo: | ||
def baz(using Quotes): Unit = '{ | ||
def f[T](x: T): T = ${ identity('{ x: T }) } | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A comment with an example showing the shape of the generated tree would make this code easier to follow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added an example and refactored a bit to make it more readable.