From 74297159f5c4df42dcf6289f4daea79e7d4f7bb4 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Sun, 28 Oct 2012 13:44:10 +0100 Subject: SI-6575 Plug inference leak of AbstractPartialFun Usually this isn't needed, as partial functions can only be defined with an expected type. But if that expected type is and inherited method return type, the actual type of the partial function literal is used, and the implementation detail of `AbstractPartialFunction[A, B] with Serializable` leaks out. After this change, the inferred types match those from Scala 2.9.2. ticket/6575 ~/code/scala scalac29 -Xprint:typer test/files/pos/t6575a.scala | grep def > 29.txt ticket/6575 ~/code/scala squalac -Xprint:typer test/files/pos/t6575a.scala | grep def > 210.txt ticket/6575 ~/code/scala diff -u 29.txt 210.txt --- 29.txt 2012-10-28 13:51:07.000000000 +0100 +++ 210.txt 2012-10-28 13:51:20.000000000 +0100 @@ -1,7 +1,16 @@ def foo: PartialFunction[Int,Int] def /*Y*/$init$(): Unit = { - absoverride def foo: PartialFunction[Int,Int] = ((x0$1: Int) => x0$1 match { + absoverride def foo: PartialFunction[Int,Int] = { + def (): anonymous class $anonfun = { + final override def applyOrElse[A1 >: Nothing <: Int, B1 >: Int <: Any](x$1: A1, default: A1 => B1): B1 = (x$1: A1 @unchecked) match { + final def isDefinedAt(x$1: Int): Boolean = (x$1: Int @unchecked) match { def /*Z*/$init$(): Unit = { - absoverride def foo: PartialFunction[Int,Int] = ((x0$2: Int) => x0$2 match { + absoverride def foo: PartialFunction[Int,Int] = { + def (): anonymous class $anonfun = { + final override def applyOrElse[A1 >: Nothing <: Int, B1 >: Int <: Any](x$1: A1, default: A1 => B1): B1 = (x$1: A1 @unchecked) match { + final def isDefinedAt(x$1: Int): Boolean = (x$1: Int @unchecked) match { def /*Comb*/$init$(): Unit = { - absoverride def foo: PartialFunction[Int,Int] = ((x0$3: Int) => x0$3 match { + absoverride def foo: PartialFunction[Int,Int] = { + def (): anonymous class $anonfun = { + final override def applyOrElse[A1 >: Nothing <: Int, B1 >: Int <: Any](x$1: A1, default: A1 => B1): B1 = (x$1: A1 @unchecked) match { + final def isDefinedAt(x$1: Int): Boolean = (x$1: Int @unchecked) match { --- src/compiler/scala/tools/nsc/typechecker/Typers.scala | 10 +++++++++- test/files/pos/t6575a.scala | 15 +++++++++++++++ test/files/pos/t6575b.scala | 17 +++++++++++++++++ 3 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 test/files/pos/t6575a.scala create mode 100644 test/files/pos/t6575b.scala diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index f82786da35..8e06588ed8 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -2590,7 +2590,15 @@ trait Typers extends Modes with Adaptations with Tags { def translated = if (members.head eq EmptyTree) setError(tree) - else typed(atPos(tree.pos)(Block(List(ClassDef(anonClass, NoMods, ListOfNil, ListOfNil, members, tree.pos.focus)), atPos(tree.pos.focus)(New(anonClass.tpe)))), mode, pt) + else { + val typedBlock = typed(atPos(tree.pos)( + Block(ClassDef(anonClass, NoMods, ListOfNil, ListOfNil, members, tree.pos.focus), atPos(tree.pos.focus)(New(anonClass.tpe))) + ), mode, pt) + // Don't leak implementation details into the type, see SI-6575 + if (isPartial && !typedBlock.isErrorTyped) + typedBlock modifyType (_ baseType PartialFunctionClass) + else typedBlock + } } // Function(params, Match(sel, cases)) ==> new Function { def apply(params) = `translateMatch('sel match { cases }')` } diff --git a/test/files/pos/t6575a.scala b/test/files/pos/t6575a.scala new file mode 100644 index 0000000000..f128714dab --- /dev/null +++ b/test/files/pos/t6575a.scala @@ -0,0 +1,15 @@ +trait X { def foo: PartialFunction[Int, Int] } + +trait Y extends X { + // Inferred type was AbstractPartialFunction[Int, Int] with Serializable + abstract override def foo = { case i => super.foo(i) * 2 } +} +trait Z extends X { + // ditto + abstract override def foo = { case i => super.foo(i) + 3 } +} + +trait Comb extends Y with Z { + // ... which led to a type error here. + abstract override def foo: PartialFunction[Int, Int] = { case i => super.foo(i) - 2 } +} diff --git a/test/files/pos/t6575b.scala b/test/files/pos/t6575b.scala new file mode 100644 index 0000000000..d3e58b2a16 --- /dev/null +++ b/test/files/pos/t6575b.scala @@ -0,0 +1,17 @@ +// inferred types were okay here as Function nodes aren't +// translated into anoymous subclasses of AbstractFunctionN +// until after the typer. +// +// So this test is just confirmation. +trait X { def foo: Function1[Int, Int] } + +trait Y extends X { + abstract override def foo = { case i => super.foo(i) * 2 } +} +trait Z extends X { + abstract override def foo = { case i => super.foo(i) + 3 } +} + +trait Comb extends Y with Z { + abstract override def foo: Function1[Int, Int] = { case i => super.foo(i) - 2 } +} -- cgit v1.2.3 From 2d9b65b8fce13166f7fd521e433e18039356c4db Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Sun, 28 Oct 2012 23:10:12 +0100 Subject: Wider use and a new variant of typedPos. It's safe to replace `localTyper.typed(atPos(pos)(tree))` with `localTyper.typedPos(pos)(tree)` given that we're all in the same cake and we'll get to the same `atPos`. --- src/compiler/scala/tools/nsc/transform/Mixin.scala | 2 +- .../scala/tools/nsc/typechecker/Duplicators.scala | 8 +++++--- .../scala/tools/nsc/typechecker/Implicits.scala | 2 +- .../scala/tools/nsc/typechecker/Typers.scala | 17 ++++++++++++----- .../tools/selectivecps/SelectiveCPSTransform.scala | 22 +++++++++++----------- 5 files changed, 30 insertions(+), 21 deletions(-) diff --git a/src/compiler/scala/tools/nsc/transform/Mixin.scala b/src/compiler/scala/tools/nsc/transform/Mixin.scala index 2b0520592b..80900a1a0a 100644 --- a/src/compiler/scala/tools/nsc/transform/Mixin.scala +++ b/src/compiler/scala/tools/nsc/transform/Mixin.scala @@ -481,7 +481,7 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL { /** The typer */ private var localTyper: erasure.Typer = _ - private def typedPos(pos: Position)(tree: Tree) = localTyper typed { atPos(pos)(tree) } + private def typedPos(pos: Position)(tree: Tree): Tree = localTyper.typedPos(pos)(tree) private def localTyped(pos: Position, tree: Tree, pt: Type) = localTyper.typed(atPos(pos)(tree), pt) /** Map lazy values to the fields they should null after initialization. */ diff --git a/src/compiler/scala/tools/nsc/typechecker/Duplicators.scala b/src/compiler/scala/tools/nsc/typechecker/Duplicators.scala index 97e86d183e..aa507efe5a 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Duplicators.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Duplicators.scala @@ -321,7 +321,9 @@ abstract class Duplicators extends Analyzer { // we use the symbol name instead of the tree name because the symbol may have been // name mangled, rendering the tree name obsolete // log(tree) - val t = super.typed(atPos(tree.pos)(Select(This(newClassOwner), tree.symbol.name)), mode, pt) + val t = super.typedPos(tree.pos, mode, pt) { + Select(This(newClassOwner), tree.symbol.name) + } // log("typed to: " + t + "; tpe = " + t.tpe + "; " + inspectTpe(t.tpe)) t @@ -331,7 +333,7 @@ abstract class Duplicators extends Analyzer { val tree1 = This(newClassOwner) // log("tree1: " + tree1) debuglog("mapped " + tree + " to " + tree1) - super.typed(atPos(tree.pos)(tree1), mode, pt) + super.typedPos(tree.pos, mode, pt)(tree1) case This(_) => debuglog("selection on this, plain: " + tree) @@ -368,7 +370,7 @@ abstract class Duplicators extends Analyzer { cases } - super.typed(atPos(tree.pos)(Match(scrut, cases1)), mode, pt) + super.typedPos(tree.pos, mode, pt)(Match(scrut, cases1)) case EmptyTree => // no need to do anything, in particular, don't set the type to null, EmptyTree.tpe_= asserts diff --git a/src/compiler/scala/tools/nsc/typechecker/Implicits.scala b/src/compiler/scala/tools/nsc/typechecker/Implicits.scala index 7852ff49e1..99301cebcf 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Implicits.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Implicits.scala @@ -1172,7 +1172,7 @@ trait Implicits { } try { - val tree1 = typed(atPos(pos.focus)(arg)) + val tree1 = typedPos(pos.focus)(arg) if (context.hasErrors) processMacroExpansionError(context.errBuffer.head.errPos, context.errBuffer.head.errMsg) else new SearchResult(tree1, EmptyTreeTypeSubstituter) } catch { diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index 8e06588ed8..6a55b16275 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -1049,7 +1049,9 @@ trait Typers extends Modes with Adaptations with Tags { case other => other } - typed(atPos(tree.pos)(Select(qual setPos tree.pos.makeTransparent, nme.apply)), mode, pt) + typedPos(tree.pos, mode, pt) { + Select(qual setPos tree.pos.makeTransparent, nme.apply) + } } // begin adapt @@ -1147,11 +1149,15 @@ trait Typers extends Modes with Adaptations with Tags { if (sym == UnitClass && tree.tpe <:< AnyClass.tpe) { // (12) if (settings.warnValueDiscard.value) context.unit.warning(tree.pos, "discarded non-Unit value") - return typed(atPos(tree.pos)(Block(List(tree), Literal(Constant()))), mode, pt) + return typedPos(tree.pos, mode, pt) { + Block(List(tree), Literal(Constant())) + } } else if (isNumericValueClass(sym) && isNumericSubType(tree.tpe, pt)) { if (settings.warnNumericWiden.value) context.unit.warning(tree.pos, "implicit numeric widening") - return typed(atPos(tree.pos)(Select(tree, "to" + sym.name)), mode, pt) + return typedPos(tree.pos, mode, pt) { + Select(tree, "to" + sym.name) + } } case AnnotatedType(_, _, _) if canAdaptAnnotations(tree, mode, pt) => // (13) return typed(adaptAnnotations(tree, mode, pt), mode, pt) @@ -2591,9 +2597,9 @@ trait Typers extends Modes with Adaptations with Tags { def translated = if (members.head eq EmptyTree) setError(tree) else { - val typedBlock = typed(atPos(tree.pos)( + val typedBlock = typedPos(tree.pos, mode, pt) { Block(ClassDef(anonClass, NoMods, ListOfNil, ListOfNil, members, tree.pos.focus), atPos(tree.pos.focus)(New(anonClass.tpe))) - ), mode, pt) + } // Don't leak implementation details into the type, see SI-6575 if (isPartial && !typedBlock.isErrorTyped) typedBlock modifyType (_ baseType PartialFunctionClass) @@ -5502,6 +5508,7 @@ trait Typers extends Modes with Adaptations with Tags { ret } + def typedPos(pos: Position, mode: Int, pt: Type)(tree: Tree) = typed(atPos(pos)(tree), mode, pt) def typedPos(pos: Position)(tree: Tree) = typed(atPos(pos)(tree)) // TODO: see if this formulation would impose any penalty, since // it makes for a lot less casting. diff --git a/src/continuations/plugin/scala/tools/selectivecps/SelectiveCPSTransform.scala b/src/continuations/plugin/scala/tools/selectivecps/SelectiveCPSTransform.scala index 54a0079f40..4482bf2b7c 100644 --- a/src/continuations/plugin/scala/tools/selectivecps/SelectiveCPSTransform.scala +++ b/src/continuations/plugin/scala/tools/selectivecps/SelectiveCPSTransform.scala @@ -193,12 +193,12 @@ abstract class SelectiveCPSTransform extends PluginComponent with val pos = catches.head.pos val funSym = currentOwner.newValueParameter(cpsNames.catches, pos).setInfo(appliedType(PartialFunctionClass.tpe, List(ThrowableClass.tpe, targettp))) - val funDef = localTyper.typed(atPos(pos) { + val funDef = localTyper.typedPos(pos) { ValDef(funSym, Match(EmptyTree, catches1)) - }) - val expr2 = localTyper.typed(atPos(pos) { + } + val expr2 = localTyper.typedPos(pos) { Apply(Select(expr1, expr1.tpe.member(cpsNames.flatMapCatch)), List(Ident(funSym))) - }) + } val exSym = currentOwner.newValueParameter(cpsNames.ex, pos).setInfo(ThrowableClass.tpe) @@ -223,7 +223,7 @@ abstract class SelectiveCPSTransform extends PluginComponent with val pos = finalizer.pos val finalizer2 = duplicateTree(finalizer1) val fun = Function(List(), finalizer2) - val expr3 = localTyper.typed(atPos(pos) { Apply(Select(expr2, expr2.tpe.member("mapFinally")), List(fun)) }) + val expr3 = localTyper.typedPos(pos) { Apply(Select(expr2, expr2.tpe.member("mapFinally")), List(fun)) } val chown = new ChangeOwnerTraverser(currentOwner, fun.symbol) chown.traverse(finalizer2) @@ -290,7 +290,7 @@ abstract class SelectiveCPSTransform extends PluginComponent with val body1 = (new TreeSymSubstituter(List(vd.symbol), List(ctxValSym)))(body) - val body2 = localTyper.typed(atPos(vd.symbol.pos) { body1 }) + val body2 = localTyper.typedPos(vd.symbol.pos) { body1 } // in theory it would be nicer to look for an @cps annotation instead // of testing for Context @@ -304,7 +304,7 @@ abstract class SelectiveCPSTransform extends PluginComponent with def applyCombinatorFun(ctxR: Tree, body: Tree) = { val arg = currentOwner.newValueParameter(name, ctxR.pos).setInfo(tpe) val body1 = (new TreeSymSubstituter(List(vd.symbol), List(arg)))(body) - val fun = localTyper.typed(atPos(vd.symbol.pos) { Function(List(ValDef(arg)), body1) }) // types body as well + val fun = localTyper.typedPos(vd.symbol.pos) { Function(List(ValDef(arg)), body1) } // types body as well arg.owner = fun.symbol body1.changeOwner(currentOwner -> fun.symbol) @@ -328,9 +328,9 @@ abstract class SelectiveCPSTransform extends PluginComponent with debuglog("will use method:"+methodName) - localTyper.typed(atPos(vd.symbol.pos) { + localTyper.typedPos(vd.symbol.pos) { Apply(Select(ctxR, ctxR.tpe.member(methodName)), List(fun)) - }) + } } def mkBlock(stms: List[Tree], expr: Tree) = if (stms.nonEmpty) Block(stms, expr) else expr @@ -352,12 +352,12 @@ abstract class SelectiveCPSTransform extends PluginComponent with def ctxRef = localTyper.typed(Ident(ctxSym)) val argSym = currentOwner.newValue(vd.symbol.name).setInfo(tpe) val argDef = localTyper.typed(ValDef(argSym, Select(ctxRef, ctxRef.tpe.member(cpsNames.getTrivialValue)))) - val switchExpr = localTyper.typed(atPos(vd.symbol.pos) { + val switchExpr = localTyper.typedPos(vd.symbol.pos) { val body2 = mkBlock(bodyStms, bodyExpr).duplicate // dup before typing! If(Select(ctxRef, ctxSym.tpe.member(cpsNames.isTrivial)), applyTrivial(argSym, mkBlock(argDef::bodyStms, bodyExpr)), applyCombinatorFun(ctxRef, body2)) - }) + } (List(ctxDef), switchExpr) } else { // ctx.flatMap { => ... } -- cgit v1.2.3 From f30f97273aa40600f0f8d7c3e96d0ab3e0eed318 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Mon, 29 Oct 2012 07:48:56 +0100 Subject: Use Typed rather than .setType In response to pull request feedback. --- src/compiler/scala/tools/nsc/typechecker/Typers.scala | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index 6a55b16275..c5bd92a943 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -2602,7 +2602,9 @@ trait Typers extends Modes with Adaptations with Tags { } // Don't leak implementation details into the type, see SI-6575 if (isPartial && !typedBlock.isErrorTyped) - typedBlock modifyType (_ baseType PartialFunctionClass) + typedPos(tree.pos, mode, pt) { + Typed(typedBlock, TypeTree(typedBlock.tpe baseType PartialFunctionClass)) + } else typedBlock } } -- cgit v1.2.3