From 8ab9133db9069e031d2a2cb3f1b7629308503f70 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 26 Mar 2021 17:41:53 +0100 Subject: [PATCH 1/3] Don't drop Inlined nodes in beta reduce Partial fix for #11894 --- .../src/dotty/tools/dotc/typer/Inliner.scala | 43 +++++++++++-------- tests/pos/i11894a.scala | 24 +++++++++++ 2 files changed, 48 insertions(+), 19 deletions(-) create mode 100644 tests/pos/i11894a.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Inliner.scala b/compiler/src/dotty/tools/dotc/typer/Inliner.scala index 08ff58e918a2..4f4b3f942b10 100644 --- a/compiler/src/dotty/tools/dotc/typer/Inliner.scala +++ b/compiler/src/dotty/tools/dotc/typer/Inliner.scala @@ -995,26 +995,31 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(using Context) { */ def betaReduce(tree: Tree)(using Context): Tree = tree match { case Apply(Select(cl @ closureDef(ddef), nme.apply), args) if defn.isFunctionType(cl.tpe) => - ddef.tpe.widen match { - case mt: MethodType if ddef.paramss.head.length == args.length => - val bindingsBuf = new mutable.ListBuffer[ValOrDefDef] - val argSyms = mt.paramNames.lazyZip(mt.paramInfos).lazyZip(args).map { (name, paramtp, arg) => - arg.tpe.dealias match { - case ref @ TermRef(NoPrefix, _) => ref.symbol - case _ => - paramBindingDef(name, paramtp, arg, bindingsBuf)( - using ctx.withSource(cl.source) - ).symbol + // closureDef also returns a result for closures wrapped in Inlined nodes. + // These need to be preserved. + def recur(cl: Tree): Tree = cl match + case Inlined(call, bindings, expr) => + cpy.Inlined(cl)(call, bindings, recur(expr)) + case _ => ddef.tpe.widen match + case mt: MethodType if ddef.paramss.head.length == args.length => + val bindingsBuf = new mutable.ListBuffer[ValOrDefDef] + val argSyms = mt.paramNames.lazyZip(mt.paramInfos).lazyZip(args).map { (name, paramtp, arg) => + arg.tpe.dealias match { + case ref @ TermRef(NoPrefix, _) => ref.symbol + case _ => + paramBindingDef(name, paramtp, arg, bindingsBuf)( + using ctx.withSource(cl.source) + ).symbol + } } - } - val expander = new TreeTypeMap( - oldOwners = ddef.symbol :: Nil, - newOwners = ctx.owner :: Nil, - substFrom = ddef.paramss.head.map(_.symbol), - substTo = argSyms) - Block(bindingsBuf.toList, expander.transform(ddef.rhs)) - case _ => tree - } + val expander = new TreeTypeMap( + oldOwners = ddef.symbol :: Nil, + newOwners = ctx.owner :: Nil, + substFrom = ddef.paramss.head.map(_.symbol), + substTo = argSyms) + Block(bindingsBuf.toList, expander.transform(ddef.rhs)).withSpan(tree.span) + case _ => tree + recur(cl) case _ => tree } diff --git a/tests/pos/i11894a.scala b/tests/pos/i11894a.scala new file mode 100644 index 000000000000..f653f979ddde --- /dev/null +++ b/tests/pos/i11894a.scala @@ -0,0 +1,24 @@ +object CallbackTo { + extension [A](self: CallbackTo[Option[A]]) { + transparent inline def asOption: Option[A] = + self.toScalaFn() + } +} + +final class CallbackTo[A] (val x: List[A]) { + + transparent inline def runNow(): A = + x.head + + transparent inline def toScalaFn: () => A = + () => runNow() + + def map[B](f: A => B): CallbackTo[B] = + ??? + + def toOption: Option[A] = { + val x = map[Option[A]](Some(_)) + val y = x: CallbackTo[Option[A]] // ok: type is what we expect + y.asOption // error + } +} \ No newline at end of file From b3baab2a131cb7957c173cb8c57c74984aba0918 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 26 Mar 2021 18:56:19 +0100 Subject: [PATCH 2/3] Don't recurse into type arguments in inliner map Inlined type arguments should be left alone in the inliner TreeTypeMap, analogous to inlined term arguments. --- .../dotty/tools/dotc/ast/TreeTypeMap.scala | 25 ++++++++++++++----- .../src/dotty/tools/dotc/typer/Inliner.scala | 9 +++---- tests/pos/i11350.scala | 5 ++++ tests/pos/i11894.scala | 7 ++++++ tests/pos/i11894b.scala | 24 ++++++++++++++++++ 5 files changed, 59 insertions(+), 11 deletions(-) create mode 100644 tests/pos/i11350.scala create mode 100644 tests/pos/i11894.scala create mode 100644 tests/pos/i11894b.scala diff --git a/compiler/src/dotty/tools/dotc/ast/TreeTypeMap.scala b/compiler/src/dotty/tools/dotc/ast/TreeTypeMap.scala index dcfc6e1447c3..9203d29bc81d 100644 --- a/compiler/src/dotty/tools/dotc/ast/TreeTypeMap.scala +++ b/compiler/src/dotty/tools/dotc/ast/TreeTypeMap.scala @@ -22,6 +22,8 @@ import core.tasty.TreePickler.Hole * @param newOwners New owners, replacing previous owners. * @param substFrom The symbols that need to be substituted. * @param substTo The substitution targets. + * @param stopAtInlinedArgument Whether one should stop at an inlined argument, + * i.e. a node of form Inlined(EmptyTree, _, _). * * The reason the substitution is broken out from the rest of the type map is * that all symbols have to be substituted at the same time. If we do not do this, @@ -31,14 +33,19 @@ import core.tasty.TreePickler.Hole * do S2 we get outer#2.inner#4. But that means that the named type outer#2.inner * gets two different denotations in the same period. Hence, if -Yno-double-bindings is * set, we would get a data race assertion error. + * + * Note: TreeTypeMap is final, since derived maps are created dynamically for + * nested scopes. Any subclass of TreeTypeMap would revert to the standard + * TreeTypeMap in these recursive invocations. */ -class TreeTypeMap( +final class TreeTypeMap( val typeMap: Type => Type = IdentityTypeMap, val treeMap: tpd.Tree => tpd.Tree = identity _, val oldOwners: List[Symbol] = Nil, val newOwners: List[Symbol] = Nil, val substFrom: List[Symbol] = Nil, - val substTo: List[Symbol] = Nil)(using Context) extends tpd.TreeMap { + val substTo: List[Symbol] = Nil, + stopAtInlinedArgument: Boolean = false)(using Context) extends tpd.TreeMap { import tpd._ /** If `sym` is one of `oldOwners`, replace by corresponding symbol in `newOwners` */ @@ -76,8 +83,11 @@ class TreeTypeMap( updateDecls(prevStats.tail, newStats.tail) } - /** If true, stop return Inlined(Empty, _, _) nodes unchanged */ - def stopAtInlinedArgument: Boolean = false + def transformInlined(tree: tpd.Inlined)(using Context): tpd.Tree = + val Inlined(call, bindings, expanded) = tree + val (tmap1, bindings1) = transformDefs(bindings) + val expanded1 = tmap1.transform(expanded) + cpy.Inlined(tree)(call, bindings1, expanded1) override def transform(tree: tpd.Tree)(using Context): tpd.Tree = treeMap(tree) match { case impl @ Template(constr, parents, self, _) => @@ -111,7 +121,9 @@ class TreeTypeMap( cpy.Block(blk)(stats1, expr1) case inlined @ Inlined(call, bindings, expanded) => if stopAtInlinedArgument && call.isEmpty then - inlined + expanded match + case expanded: TypeTree => assert(bindings.isEmpty); expanded + case _ => inlined else val (tmap1, bindings1) = transformDefs(bindings) val expanded1 = tmap1.transform(expanded) @@ -177,7 +189,8 @@ class TreeTypeMap( from ++ oldOwners, to ++ newOwners, from ++ substFrom, - to ++ substTo) + to ++ substTo, + stopAtInlinedArgument) } /** Apply `typeMap` and `ownerMap` to given symbols `syms` diff --git a/compiler/src/dotty/tools/dotc/typer/Inliner.scala b/compiler/src/dotty/tools/dotc/typer/Inliner.scala index 4f4b3f942b10..5b0d4ad50dd4 100644 --- a/compiler/src/dotty/tools/dotc/typer/Inliner.scala +++ b/compiler/src/dotty/tools/dotc/typer/Inliner.scala @@ -746,16 +746,15 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(using Context) { val inlinedSingleton = singleton(t).withSpan(argSpan) inlinedFromOutside(inlinedSingleton)(tree.span) case Some(t) if tree.isType => - TypeTree(t).withSpan(argSpan) + inlinedFromOutside(TypeTree(t).withSpan(argSpan))(tree.span) case _ => tree } case tree => tree }, oldOwners = inlinedMethod :: Nil, - newOwners = ctx.owner :: Nil - )(using inlineCtx) { - override def stopAtInlinedArgument: Boolean = true - } + newOwners = ctx.owner :: Nil, + stopAtInlinedArgument = true + )(using inlineCtx) // Apply inliner to `rhsToInline`, split off any implicit bindings from result, and // make them part of `bindingsBuf`. The expansion is then the tree that remains. diff --git a/tests/pos/i11350.scala b/tests/pos/i11350.scala new file mode 100644 index 000000000000..63b22d70df41 --- /dev/null +++ b/tests/pos/i11350.scala @@ -0,0 +1,5 @@ +//case class A[T](action: A[T] ?=> String) // now disallowed + +class A1[T](action: A1[T] ?=> String = (_: A1[T]) ?=> "") // works +//case class A2[T](action: A2[?] ?=> String) // now disallowed +//case class A3[T](action: A3[T] => String) // now disallowed diff --git a/tests/pos/i11894.scala b/tests/pos/i11894.scala new file mode 100644 index 000000000000..c942b7168f59 --- /dev/null +++ b/tests/pos/i11894.scala @@ -0,0 +1,7 @@ +class CallbackTo[A](val x: A): + inline def runNow(): A = x + inline def toScalaFn: () => A = () => runNow() + def toOption(): Option[A] = + val y: CallbackTo[Option[A]] = ??? + val f: () => Option[A] = y.toScalaFn // error + f() \ No newline at end of file diff --git a/tests/pos/i11894b.scala b/tests/pos/i11894b.scala new file mode 100644 index 000000000000..828e8b3d1989 --- /dev/null +++ b/tests/pos/i11894b.scala @@ -0,0 +1,24 @@ +object CallbackTo { + extension [A](self: CallbackTo[Option[A]]) { + inline def asOption: Option[A] = + self.toScalaFn() + } +} + +final class CallbackTo[A] (val x: List[A]) { + + inline def runNow(): A = + x.head + + inline def toScalaFn: () => A = + () => runNow() + + def map[B](f: A => B): CallbackTo[B] = + ??? + + def toOption: Option[A] = { + val x = map[Option[A]](Some(_)) + val y = x: CallbackTo[Option[A]] // ok: type is what we expect + y.asOption // error + } +} \ No newline at end of file From c48fb9bffcdade52af1b586f603d37eef765f892 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 27 Mar 2021 10:39:46 +0100 Subject: [PATCH 3/3] Refactor TreeTypeMap Break out an `InlinerMap` that contains inliner-specific overrides. This avoids a global mode in TreeTypeMap that had to deal with inliner-specific behavior. --- .../dotty/tools/dotc/ast/TreeTypeMap.scala | 36 ++++++++----------- .../src/dotty/tools/dotc/typer/Inliner.scala | 34 ++++++++++++++++-- 2 files changed, 47 insertions(+), 23 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/TreeTypeMap.scala b/compiler/src/dotty/tools/dotc/ast/TreeTypeMap.scala index 9203d29bc81d..f80d126bc471 100644 --- a/compiler/src/dotty/tools/dotc/ast/TreeTypeMap.scala +++ b/compiler/src/dotty/tools/dotc/ast/TreeTypeMap.scala @@ -22,8 +22,6 @@ import core.tasty.TreePickler.Hole * @param newOwners New owners, replacing previous owners. * @param substFrom The symbols that need to be substituted. * @param substTo The substitution targets. - * @param stopAtInlinedArgument Whether one should stop at an inlined argument, - * i.e. a node of form Inlined(EmptyTree, _, _). * * The reason the substitution is broken out from the rest of the type map is * that all symbols have to be substituted at the same time. If we do not do this, @@ -33,21 +31,25 @@ import core.tasty.TreePickler.Hole * do S2 we get outer#2.inner#4. But that means that the named type outer#2.inner * gets two different denotations in the same period. Hence, if -Yno-double-bindings is * set, we would get a data race assertion error. - * - * Note: TreeTypeMap is final, since derived maps are created dynamically for - * nested scopes. Any subclass of TreeTypeMap would revert to the standard - * TreeTypeMap in these recursive invocations. */ -final class TreeTypeMap( +class TreeTypeMap( val typeMap: Type => Type = IdentityTypeMap, val treeMap: tpd.Tree => tpd.Tree = identity _, val oldOwners: List[Symbol] = Nil, val newOwners: List[Symbol] = Nil, val substFrom: List[Symbol] = Nil, - val substTo: List[Symbol] = Nil, - stopAtInlinedArgument: Boolean = false)(using Context) extends tpd.TreeMap { + val substTo: List[Symbol] = Nil)(using Context) extends tpd.TreeMap { import tpd._ + def copy( + typeMap: Type => Type, + treeMap: tpd.Tree => tpd.Tree, + oldOwners: List[Symbol], + newOwners: List[Symbol], + substFrom: List[Symbol], + substTo: List[Symbol])(using Context): TreeTypeMap = + new TreeTypeMap(typeMap, treeMap, oldOwners, newOwners, substFrom, substTo) + /** If `sym` is one of `oldOwners`, replace by corresponding symbol in `newOwners` */ def mapOwner(sym: Symbol): Symbol = sym.subst(oldOwners, newOwners) @@ -119,15 +121,8 @@ final class TreeTypeMap( val (tmap1, stats1) = transformDefs(stats) val expr1 = tmap1.transform(expr) cpy.Block(blk)(stats1, expr1) - case inlined @ Inlined(call, bindings, expanded) => - if stopAtInlinedArgument && call.isEmpty then - expanded match - case expanded: TypeTree => assert(bindings.isEmpty); expanded - case _ => inlined - else - val (tmap1, bindings1) = transformDefs(bindings) - val expanded1 = tmap1.transform(expanded) - cpy.Inlined(inlined)(call, bindings1, expanded1) + case inlined: Inlined => + transformInlined(inlined) case cdef @ CaseDef(pat, guard, rhs) => val tmap = withMappedSyms(patVars(pat)) val pat1 = tmap.transform(pat) @@ -183,14 +178,13 @@ final class TreeTypeMap( assert(!to.exists(substFrom contains _)) assert(!from.exists(newOwners contains _)) assert(!to.exists(oldOwners contains _)) - new TreeTypeMap( + copy( typeMap, treeMap, from ++ oldOwners, to ++ newOwners, from ++ substFrom, - to ++ substTo, - stopAtInlinedArgument) + to ++ substTo) } /** Apply `typeMap` and `ownerMap` to given symbols `syms` diff --git a/compiler/src/dotty/tools/dotc/typer/Inliner.scala b/compiler/src/dotty/tools/dotc/typer/Inliner.scala index 5b0d4ad50dd4..97afc587fcd6 100644 --- a/compiler/src/dotty/tools/dotc/typer/Inliner.scala +++ b/compiler/src/dotty/tools/dotc/typer/Inliner.scala @@ -706,11 +706,40 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(using Context) { def inlinedFromOutside(tree: Tree)(span: Span): Tree = Inlined(EmptyTree, Nil, tree)(using ctx.withSource(inlinedMethod.topLevelClass.source)).withSpan(span) + // InlinerMap is a TreeTypeMap with special treatment for inlined arguments: + // They are generally left alone (not mapped further, and if they wrap a type + // the type Inlined wrapper gets dropped + class InlinerMap( + typeMap: Type => Type, + treeMap: Tree => Tree, + oldOwners: List[Symbol], + newOwners: List[Symbol], + substFrom: List[Symbol], + substTo: List[Symbol])(using Context) + extends TreeTypeMap(typeMap, treeMap, oldOwners, newOwners, substFrom, substTo): + + override def copy( + typeMap: Type => Type, + treeMap: Tree => Tree, + oldOwners: List[Symbol], + newOwners: List[Symbol], + substFrom: List[Symbol], + substTo: List[Symbol])(using Context) = + new InlinerMap(typeMap, treeMap, oldOwners, newOwners, substFrom, substTo) + + override def transformInlined(tree: Inlined)(using Context) = + if tree.call.isEmpty then + tree.expansion match + case expansion: TypeTree => expansion + case _ => tree + else super.transformInlined(tree) + end InlinerMap + // A tree type map to prepare the inlined body for typechecked. // The translation maps references to `this` and parameters to // corresponding arguments or proxies on the type and term level. It also changes // the owner from the inlined method to the current owner. - val inliner = new TreeTypeMap( + val inliner = new InlinerMap( typeMap = new DeepTypeMap { def apply(t: Type) = t match { @@ -753,7 +782,8 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(using Context) { }, oldOwners = inlinedMethod :: Nil, newOwners = ctx.owner :: Nil, - stopAtInlinedArgument = true + substFrom = Nil, + substTo = Nil )(using inlineCtx) // Apply inliner to `rhsToInline`, split off any implicit bindings from result, and