From 5e47af2a51d1fe7e032a074a24a7843c7f561b07 Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Mon, 12 Jun 2023 08:22:06 +0200 Subject: [PATCH 1/2] Fix bounds of encoded type variables in quote patterns When we encode quote patterns into `unapply` methods, we need to create a copy of each type variable. One copy is kept within the quote(in the next stage) and the other is used in the `unapply` method to define the usual pattern type variables. When creating the latter we copied the symbols but did not update the infos. This implies that if type variables would be bounded by each other, the bounds of the copies would be the original types instead of the copies. We need to update those references. To update the info we now create all the symbols in one pass and the update all their infos in a second pass. This also implies that we cannot use the `newPatternBoundSymbol` to create the symbol as this constructor will register the info into GADT bounds. Instead we use the plain `newSymbol`. Then in the second pass, when we have updated the infos, we register the symbol into GADT bounds. Note that the code in the added test does compiles correctly, but it had the inconsistent bounds. This test is added in case we need to manually inspect the bounds latter. This test does fail to compile in https://github.com/lampepfl/dotty/pull/17935 if this fix is not applied. --- .../tools/dotc/typer/QuotesAndSplices.scala | 61 ++++++++++++++++--- .../quote-pattern-type-variable-bounds.scala | 12 ++++ 2 files changed, 64 insertions(+), 9 deletions(-) create mode 100644 tests/pos-macros/quote-pattern-type-variable-bounds.scala diff --git a/compiler/src/dotty/tools/dotc/typer/QuotesAndSplices.scala b/compiler/src/dotty/tools/dotc/typer/QuotesAndSplices.scala index d3becfd6e9b7..6a8d784c57cd 100644 --- a/compiler/src/dotty/tools/dotc/typer/QuotesAndSplices.scala +++ b/compiler/src/dotty/tools/dotc/typer/QuotesAndSplices.scala @@ -214,13 +214,7 @@ trait QuotesAndSplices { private def splitQuotePattern(quoted: Tree)(using Context): (collection.Map[Symbol, Bind], Tree, List[Tree]) = { val ctx0 = ctx - val typeBindings: mutable.Map[Symbol, Bind] = mutable.LinkedHashMap.empty - def getBinding(sym: Symbol): Bind = - typeBindings.getOrElseUpdate(sym, { - val bindingBounds = sym.info - val bsym = newPatternBoundSymbol(sym.name.toString.stripPrefix("$").toTypeName, bindingBounds, quoted.span) - Bind(bsym, untpd.Ident(nme.WILDCARD).withType(bindingBounds)).withSpan(quoted.span) - }) + val bindSymMapping: collection.Map[Symbol, Bind] = unapplyBindingsMapping(quoted) object splitter extends tpd.TreeMap { private var variance: Int = 1 @@ -300,7 +294,7 @@ trait QuotesAndSplices { report.error(IllegalVariableInPatternAlternative(tdef.symbol.name), tdef.srcPos) if variance == -1 then tdef.symbol.addAnnotation(Annotation(New(ref(defn.QuotedRuntimePatterns_fromAboveAnnot.typeRef)).withSpan(tdef.span))) - val bindingType = getBinding(tdef.symbol).symbol.typeRef + val bindingType = bindSymMapping(tdef.symbol).symbol.typeRef val bindingTypeTpe = AppliedType(defn.QuotedTypeClass.typeRef, bindingType :: Nil) val sym = newPatternBoundSymbol(nameOfSyntheticGiven, bindingTypeTpe, tdef.span, flags = ImplicitVal)(using ctx0) buff += Bind(sym, untpd.Ident(nme.WILDCARD).withType(bindingTypeTpe)).withSpan(tdef.span) @@ -337,7 +331,56 @@ trait QuotesAndSplices { new TreeTypeMap(typeMap = typeMap).transform(shape1) } - (typeBindings, shape2, patterns) + (bindSymMapping, shape2, patterns) + } + + /** For each type variable defined in the quote pattern we generate an equivalent + * binding that will be as type variable in the encoded `unapply` of the quote pattern. + * + * @return Mapping from type variable symbols defined in the quote pattern into + * type variable `Bind` definitions for the `unapply` of the quote pattern. + * This mapping retains the original type variable definition order. + */ + private def unapplyBindingsMapping(quoted: Tree)(using Context): collection.Map[Symbol, Bind] = { + // Collect all existing type variable bindings and create new symbols for them. + // The old info is used, it may contain references to the old symbols. + val (oldBindings, newBindings) = { + val seen = mutable.Set.empty[Symbol] + val oldBindingsBuffer = mutable.LinkedHashSet.empty[Symbol] + val newBindingsBuffer = mutable.ListBuffer.empty[Symbol] + + new tpd.TreeTraverser { + def traverse(tree: Tree)(using Context): Unit = tree match { + case _: SplicePattern => + case Select(pat: Bind, _) if tree.symbol.isTypeSplice => + val sym = tree.tpe.dealias.typeSymbol + if sym.exists then registerNewBindSym(sym) + case tdef: TypeDef => + if tdef.symbol.hasAnnotation(defn.QuotedRuntimePatterns_patternTypeAnnot) then + registerNewBindSym(tdef.symbol) + traverseChildren(tdef) + case _ => + traverseChildren(tree) + } + private def registerNewBindSym(sym: Symbol): Unit = + if !seen(sym) then + seen += sym + oldBindingsBuffer += sym + newBindingsBuffer += newSymbol(ctx.owner, sym.name.toString.stripPrefix("$").toTypeName, Case | sym.flags, sym.info, coord = quoted.span) + }.traverse(quoted) + (oldBindingsBuffer.toList, newBindingsBuffer.toList) + } + + // Replace symbols in `mapping` in the infos of the new symbol and register GADT bounds. + // GADT bounds need to be added after the info is updated to avoid references to the old symbols. + val newBindingsRefs = newBindings.map(_.typeRef) + for newBindings <- newBindings do + newBindings.info = newBindings.info.subst(oldBindings.toList, newBindingsRefs) + ctx.gadtState.addToConstraint(newBindings) // This must be performed after the info has been updated + + // Map into Bind nodes retaining the original order + val newBindingBinds = newBindings.map(newSym => Bind(newSym, untpd.Ident(nme.WILDCARD).withType(newSym.info)).withSpan(quoted.span)) + mutable.LinkedHashMap.from(oldBindings.lazyZip(newBindingBinds)) } /** Type a quote pattern `case '{ } =>` qiven the a current prototype. Typing the pattern diff --git a/tests/pos-macros/quote-pattern-type-variable-bounds.scala b/tests/pos-macros/quote-pattern-type-variable-bounds.scala new file mode 100644 index 000000000000..c342783664d5 --- /dev/null +++ b/tests/pos-macros/quote-pattern-type-variable-bounds.scala @@ -0,0 +1,12 @@ +import quoted.* + +def foo(using Quotes)(x: Expr[Int]) = + x match + case '{ type t; type u <: `t`; f[`t`, `u`] } => + case '{ type u <: `t`; type t; f[`t`, `u`] } => + case '{ type t; type u <: `t`; g[F[`t`, `u`]] } => + case '{ type u <: `t`; type t; g[F[`t`, `u`]] } => + +def f[T, U <: T] = ??? +def g[T] = ??? +type F[T, U <: T] From 1f4d46c0be0c978a5eb4f3ca5e171b07835d2a2d Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Fri, 23 Jun 2023 16:39:38 +0200 Subject: [PATCH 2/2] Refactor split quote symbol mapping Now, the mapping is a `SeqMap[Symbol, Symbol]` to convey its ordering and the `Bind` is created when we are creating the tree of the quote pattern. --- .../tools/dotc/typer/QuotesAndSplices.scala | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/QuotesAndSplices.scala b/compiler/src/dotty/tools/dotc/typer/QuotesAndSplices.scala index 6a8d784c57cd..24320d8b445e 100644 --- a/compiler/src/dotty/tools/dotc/typer/QuotesAndSplices.scala +++ b/compiler/src/dotty/tools/dotc/typer/QuotesAndSplices.scala @@ -25,6 +25,7 @@ import dotty.tools.dotc.util.Spans._ import dotty.tools.dotc.util.Stats.record import dotty.tools.dotc.reporting.IllegalVariableInPatternAlternative import scala.collection.mutable +import scala.collection.SeqMap /** Type quotes `'{ ... }` and splices `${ ... }` */ trait QuotesAndSplices { @@ -202,7 +203,7 @@ trait QuotesAndSplices { * will return * ``` * ( - * Map(: Symbol -> : Bind), + * Map(: Symbol -> : Symbol), * <'{ * @scala.internal.Quoted.patternType type t * scala.internal.Quoted.patternHole[List[t]] @@ -211,10 +212,10 @@ trait QuotesAndSplices { * ) * ``` */ - private def splitQuotePattern(quoted: Tree)(using Context): (collection.Map[Symbol, Bind], Tree, List[Tree]) = { + private def splitQuotePattern(quoted: Tree)(using Context): (SeqMap[Symbol, Symbol], Tree, List[Tree]) = { val ctx0 = ctx - val bindSymMapping: collection.Map[Symbol, Bind] = unapplyBindingsMapping(quoted) + val bindSymMapping: SeqMap[Symbol, Symbol] = unapplyBindingsMapping(quoted) object splitter extends tpd.TreeMap { private var variance: Int = 1 @@ -294,7 +295,7 @@ trait QuotesAndSplices { report.error(IllegalVariableInPatternAlternative(tdef.symbol.name), tdef.srcPos) if variance == -1 then tdef.symbol.addAnnotation(Annotation(New(ref(defn.QuotedRuntimePatterns_fromAboveAnnot.typeRef)).withSpan(tdef.span))) - val bindingType = bindSymMapping(tdef.symbol).symbol.typeRef + val bindingType = bindSymMapping(tdef.symbol).typeRef val bindingTypeTpe = AppliedType(defn.QuotedTypeClass.typeRef, bindingType :: Nil) val sym = newPatternBoundSymbol(nameOfSyntheticGiven, bindingTypeTpe, tdef.span, flags = ImplicitVal)(using ctx0) buff += Bind(sym, untpd.Ident(nme.WILDCARD).withType(bindingTypeTpe)).withSpan(tdef.span) @@ -338,10 +339,10 @@ trait QuotesAndSplices { * binding that will be as type variable in the encoded `unapply` of the quote pattern. * * @return Mapping from type variable symbols defined in the quote pattern into - * type variable `Bind` definitions for the `unapply` of the quote pattern. + * type variable definitions for the `unapply` of the quote pattern. * This mapping retains the original type variable definition order. */ - private def unapplyBindingsMapping(quoted: Tree)(using Context): collection.Map[Symbol, Bind] = { + private def unapplyBindingsMapping(quoted: Tree)(using Context): SeqMap[Symbol, Symbol] = { // Collect all existing type variable bindings and create new symbols for them. // The old info is used, it may contain references to the old symbols. val (oldBindings, newBindings) = { @@ -379,8 +380,7 @@ trait QuotesAndSplices { ctx.gadtState.addToConstraint(newBindings) // This must be performed after the info has been updated // Map into Bind nodes retaining the original order - val newBindingBinds = newBindings.map(newSym => Bind(newSym, untpd.Ident(nme.WILDCARD).withType(newSym.info)).withSpan(quoted.span)) - mutable.LinkedHashMap.from(oldBindings.lazyZip(newBindingBinds)) + mutable.LinkedHashMap.from(oldBindings.lazyZip(newBindings)) } /** Type a quote pattern `case '{ } =>` qiven the a current prototype. Typing the pattern @@ -470,20 +470,23 @@ trait QuotesAndSplices { else tpd.Block(typeTypeVariables, pattern) } - val (typeBindings, shape, splices) = splitQuotePattern(quoted1) + val (bindSymMapping, shape, splices) = splitQuotePattern(quoted1) class ReplaceBindings extends TypeMap() { override def apply(tp: Type): Type = tp match { case tp: TypeRef => val tp1 = if (tp.symbol.isTypeSplice) tp.dealias else tp - mapOver(typeBindings.get(tp1.typeSymbol).fold(tp)(_.symbol.typeRef)) + mapOver(bindSymMapping.get(tp1.typeSymbol).fold(tp)(_.typeRef)) case tp => mapOver(tp) } } val replaceBindings = new ReplaceBindings val patType = defn.tupleType(splices.tpes.map(tpe => replaceBindings(tpe.widen))) - val typeBindingsTuple = tpd.hkNestedPairsTypeTree(typeBindings.values.toList) + val typeBinds = bindSymMapping.values.toList.map(sym => + Bind(sym, untpd.Ident(nme.WILDCARD).withType(sym.info)).withSpan(quoted.span) + ) + val typeBindingsTuple = tpd.hkNestedPairsTypeTree(typeBinds) val replaceBindingsInTree = new TreeMap { private var bindMap = Map.empty[Symbol, Symbol]