From 8279a730f02161b2cb078f0c7aa9856a891ca86a Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Wed, 21 Mar 2012 09:47:27 +0100 Subject: [vpm] typeMatchAnonFun improvements need to set up symbols with approximated info for the method and its owner before typing the method's body; when the body has been typed, we know what its result type will be, so update info of class and method to reflect that better detection of synthesized matches: annotate selector rather than relying on symbol info encapsulate CASE | SYNTHETIC flags (setting and querying) --- src/compiler/scala/reflect/internal/StdNames.scala | 2 + src/compiler/scala/tools/nsc/ast/TreeGen.scala | 8 + .../scala/tools/nsc/transform/TailCalls.scala | 2 +- .../tools/nsc/typechecker/PatMatVirtualiser.scala | 10 +- .../scala/tools/nsc/typechecker/Typers.scala | 193 +++++++++------------ 5 files changed, 101 insertions(+), 114 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/reflect/internal/StdNames.scala b/src/compiler/scala/reflect/internal/StdNames.scala index 84007425ed..9fb7b5b747 100644 --- a/src/compiler/scala/reflect/internal/StdNames.scala +++ b/src/compiler/scala/reflect/internal/StdNames.scala @@ -405,6 +405,8 @@ trait StdNames extends NameManglers { self: SymbolTable => val withFilter: NameType = "withFilter" val zip: NameType = "zip" + val synthSwitch: NameType = "$synthSwitch" + // unencoded operators object raw { final val AMP : NameType = "&" diff --git a/src/compiler/scala/tools/nsc/ast/TreeGen.scala b/src/compiler/scala/tools/nsc/ast/TreeGen.scala index 6d95b6ffdd..ad26ccad5e 100644 --- a/src/compiler/scala/tools/nsc/ast/TreeGen.scala +++ b/src/compiler/scala/tools/nsc/ast/TreeGen.scala @@ -65,6 +65,12 @@ abstract class TreeGen extends reflect.internal.TreeGen with TreeDSL { case _ => tree } + def mkSynthSwitchSelector(expr: Tree): Tree = atPos(expr.pos) { + // This can't be "Annotated(New(SwitchClass), expr)" because annotations + // are very picky about things and it crashes the compiler with "unexpected new". + Annotated(Ident(nme.synthSwitch), expr) + } + // must be kept in synch with the codegen in PatMatVirtualiser object VirtualCaseDef { def unapply(b: Block): Option[(Assign, Tree, Tree)] = b match { @@ -73,6 +79,8 @@ abstract class TreeGen extends reflect.internal.TreeGen with TreeDSL { } } + def hasSynthCaseSymbol(t: Tree) = (t.symbol ne null) && (t.symbol hasFlag (CASE | SYNTHETIC)) + // TODO: would be so much nicer if we would know during match-translation (i.e., type checking) // whether we should emit missingCase-style apply (and isDefinedAt), instead of transforming trees post-factum class MatchMatcher { diff --git a/src/compiler/scala/tools/nsc/transform/TailCalls.scala b/src/compiler/scala/tools/nsc/transform/TailCalls.scala index 6ebecb02c6..ef76fe1b1c 100644 --- a/src/compiler/scala/tools/nsc/transform/TailCalls.scala +++ b/src/compiler/scala/tools/nsc/transform/TailCalls.scala @@ -36,7 +36,7 @@ abstract class TailCalls extends Transform { } } - private def hasSynthCaseSymbol(t: Tree) = (t.symbol ne null) && (t.symbol hasFlag (Flags.CASE | Flags.SYNTHETIC)) + import gen.hasSynthCaseSymbol /** * A Tail Call Transformer diff --git a/src/compiler/scala/tools/nsc/typechecker/PatMatVirtualiser.scala b/src/compiler/scala/tools/nsc/typechecker/PatMatVirtualiser.scala index de7f03dc62..2b67f7f44b 100644 --- a/src/compiler/scala/tools/nsc/typechecker/PatMatVirtualiser.scala +++ b/src/compiler/scala/tools/nsc/typechecker/PatMatVirtualiser.scala @@ -32,6 +32,8 @@ trait PatMatVirtualiser extends ast.TreeDSL { self: Analyzer => import global._ import definitions._ + val SYNTH_CASE = Flags.CASE | SYNTHETIC + object vpmName { val one = newTermName("one") val drop = newTermName("drop") @@ -133,7 +135,7 @@ trait PatMatVirtualiser extends ast.TreeDSL { self: Analyzer => // and the only place that emits Matches after typers is for exception handling anyway) assert(phase.id <= currentRun.typerPhase.id, phase) - val scrutSym = freshSym(scrut.pos, pureType(scrutType)) setFlag (Flags.CASE | SYNTHETIC) // the flags allow us to detect generated matches by looking at the scrutinee's symbol (needed to avoid recursing endlessly on generated switches) + val scrutSym = freshSym(scrut.pos, pureType(scrutType)) setFlag SYNTH_CASE // pt = Any* occurs when compiling test/files/pos/annotDepMethType.scala with -Xexperimental combineCases(scrut, scrutSym, cases map translateCase(scrutSym, pt), pt, matchOwner, matchFailGenOverride) } @@ -1557,7 +1559,7 @@ class Foo(x: Other) { x._1 } // no error in this order else (REF(scrutSym) DOT (nme.toInt)) Some(BLOCK( VAL(scrutSym) === scrut, - Match(scrutToInt, caseDefsWithDefault) + Match(gen.mkSynthSwitchSelector(scrutToInt), caseDefsWithDefault) // add switch annotation )) } } else None @@ -1630,11 +1632,11 @@ class Foo(x: Other) { x._1 } // no error in this order * if keepGoing is false, the result Some(x) of the naive translation is encoded as matchRes == x */ def matcher(scrut: Tree, scrutSym: Symbol, restpe: Type)(cases: List[Casegen => Tree], matchFailGen: Option[Tree => Tree]): Tree = { - val matchEnd = NoSymbol.newLabel(freshName("matchEnd"), NoPosition) setFlag (SYNTHETIC | Flags.CASE) + val matchEnd = NoSymbol.newLabel(freshName("matchEnd"), NoPosition) setFlag SYNTH_CASE val matchRes = NoSymbol.newValueParameter(newTermName("x"), NoPosition, SYNTHETIC) setInfo restpe matchEnd setInfo MethodType(List(matchRes), restpe) - def newCaseSym = NoSymbol.newLabel(freshName("case"), NoPosition) setInfo MethodType(Nil, restpe) setFlag (SYNTHETIC | Flags.CASE) + def newCaseSym = NoSymbol.newLabel(freshName("case"), NoPosition) setInfo MethodType(Nil, restpe) setFlag SYNTH_CASE var nextCase = newCaseSym def caseDef(mkCase: Casegen => Tree): Tree = { val currCase = nextCase diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index 49ce9712df..eecf36487c 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -2143,65 +2143,63 @@ trait Typers extends Modes with Adaptations with PatMatVirtualiser { def adaptCase(cdef: CaseDef, mode: Int, tpe: Type): CaseDef = deriveCaseDef(cdef)(adapt(_, mode, tpe)) - def translateMatch(selector: Tree, cases: List[CaseDef], mode: Int, resTp: Type, scrutTp: Type = NoType, matchFailGen: Option[Tree => Tree] = None) = { - val selector1 = if(scrutTp eq NoType) checkDead(typed(selector, EXPRmode | BYVALmode, WildcardType)) else selector - val selectorTp = if(scrutTp eq NoType) packCaptured(selector1.tpe.widen) else scrutTp + def prepareTranslateMatch(selector0: Tree, cases: List[CaseDef], mode: Int, resTp: Type) = { + val (selector, doTranslation) = selector0 match { + case Annotated(Ident(nme.synthSwitch), selector) => (selector, false) + case s => (s, true) + } + val selector1 = checkDead(typed(selector, EXPRmode | BYVALmode, WildcardType)) + val selectorTp = packCaptured(selector1.tpe.widen) val casesTyped = typedCases(cases, selectorTp, resTp) val (ownType, needAdapt) = if (isFullyDefined(resTp)) (resTp, false) else weakLub(casesTyped map (_.tpe.deconst)) val casesAdapted = if (!needAdapt) casesTyped else casesTyped map (adaptCase(_, mode, ownType)) // val (owntype0, needAdapt) = ptOrLub(casesTyped map (x => repackExistential(x.tpe))) // val owntype = elimAnonymousClass(owntype0) + (selector1, selectorTp, casesAdapted, ownType, doTranslation) + } + def translateMatch(selector1: Tree, selectorTp: Type, casesAdapted: List[CaseDef], ownType: Type, doTranslation: Boolean, matchFailGen: Option[Tree => Tree] = None) = { def repeatedToSeq(tp: Type): Type = (tp baseType RepeatedParamClass) match { case TypeRef(_, RepeatedParamClass, args) => appliedType(SeqClass.typeConstructor, args) case _ => tp } - def isSynthSelector(selector: Tree): Boolean = selector match { - case Ident(_) if selector.symbol.hasFlag(SYNTHETIC | CASE) => true - case Select(sel, nme.toInt) => isSynthSelector(sel) // switch may need to convert to int first - case _ => false - } - - if (isSynthSelector(selector1)) { // a switch - (Match(selector1, casesAdapted) setType ownType, ownType) // setType of the Match to avoid recursing endlessly + if (!doTranslation) { // a switch + Match(selector1, casesAdapted) setType ownType // setType of the Match to avoid recursing endlessly } else { val scrutType = repeatedToSeq(elimAnonymousClass(selectorTp)) - (MatchTranslator(this).translateMatch(selector1, casesAdapted, repeatedToSeq(ownType), scrutType, matchFailGen), ownType) + MatchTranslator(this).translateMatch(selector1, casesAdapted, repeatedToSeq(ownType), scrutType, matchFailGen) } } - // TODO: use this to synthesize (partial)function implementation for matches from the get-go, - // instead of the dirty post-factum hacks in uncurry -- typedMatchAnonFun is currently not used due to mindboggling failures (see virtpatmat_anonfun_for.scala) - def typedMatchAnonFun(tree: Tree, cases: List[CaseDef], mode: Int, pt0: Type, selOverride: Option[(List[Symbol], Tree)] = None) = { - val pt = deskolemizeGADTSkolems(pt0) - val targs = pt.normalize.typeArgs - val arity = if (isFunctionType(pt)) targs.length - 1 else 1 - val scrutTp0 = if (arity == 1) targs.head else /* arity > 1 */ tupleType(targs.init) - val scrutTp = packCaptured(scrutTp0) - val ptRes = targs.last // may not be fully defined - val isPartial = pt.typeSymbol == PartialFunctionClass - val cname = tpnme.ANON_FUN_NAME - val funThis = This(cname) - // used to create a new context for pattern matching translation so that - // we can easily rejig the owner structure when we have the actual symbols for these methods - // (after type checking them, but type checking requires translation -- this seems like the easiest way to end this vicious cycle) - val applySentinel = NoSymbol.newMethod(nme.apply) - val idaSentinel = NoSymbol.newMethod(nme._isDefinedAt) - - def mkParams = { - val params = - for (i <- List.range(0, arity)) yield atPos(tree.pos.focusStart) { - ValDef(Modifiers(SYNTHETIC | PARAM), unit.freshTermName("x" + i + "$"), TypeTree(), EmptyTree) - } - val ids = params map (p => Ident(p.name)) + def typedMatchAnonFun(tree: Tree, cases: List[CaseDef], mode: Int, pt0: Type, selOverride: Option[(List[ValDef], Tree)] = None) = { + val pt = deskolemizeGADTSkolems(pt0) + val targs = pt.normalize.typeArgs + val arity = if (isFunctionType(pt)) targs.length - 1 else 1 + val ptRes = if (targs.isEmpty) WildcardType else targs.last // may not be fully defined + + val isPartial = pt.typeSymbol == PartialFunctionClass + val anonClass = context.owner.newAnonymousFunctionClass(tree.pos) + val funThis = This(anonClass) + val serialVersionUIDAnnotation = AnnotationInfo(SerialVersionUIDAttr.tpe, List(Literal(Constant(0))), List()) + + anonClass addAnnotation serialVersionUIDAnnotation + + def mkParams(methodSym: Symbol) = { + selOverride match { + case None => + val ps = methodSym newSyntheticValueParams targs.init // is there anything we can do if targs.isEmpty?? + val ids = ps map (p => Ident(p.name)) + val sel = atPos(tree.pos.focusStart) { if (arity == 1) ids.head else gen.mkTuple(ids) } + (ps, sel) + case Some((vparams, sel)) => + val newParamSyms = vparams map {p => + val tp = if(p.tpt.tpe == null) typedType(p.tpt).tpe else p.tpt.tpe + methodSym.newValueParameter(p.name, focusPos(p.pos), SYNTHETIC) setInfo tp + } - val paramsRef = selOverride match { - case None => atPos(tree.pos.focusStart) { if (arity == 1) ids.head else gen.mkTuple(ids) } - case Some((_, sel)) => sel.duplicate // we'll replace the symbols that refer to the function's original syms by the ones introduced by the DefDef once the method's been type checked (until then, we don't know them) + (newParamSyms, sel.duplicate) } - - (params, paramsRef) // paramsRef can't be typed until after match has been translated, thus supply explicit scrutTp to translate below } import CODE._ @@ -2209,67 +2207,46 @@ trait Typers extends Modes with Adaptations with PatMatVirtualiser { // need to duplicate the cases before typing them to generate the apply method, or the symbols will be all messed up val casesTrue = if (isPartial) cases map (c => deriveCaseDef(c)(x => TRUE).duplicate) else Nil - val (applyMethod, parents) = { - val (params, paramsRef) = mkParams - val (body, resTp) = newTyper(context.make(context.tree, applySentinel)).translateMatch(paramsRef, cases, mode, ptRes, scrutTp, if (isPartial) Some(scrut => (funThis DOT nme.missingCase) (scrut)) else None) + val applyMethod = { + // rig the show so we can get started typing the method body -- later we'll correct the infos... + anonClass setInfo ClassInfoType(List(ObjectClass.tpe, pt, SerializableClass.tpe), newScope, anonClass) + val methodSym = anonClass.newMethod(nme.apply, tree.pos, FINAL) + val (paramSyms, selector) = mkParams(methodSym) + methodSym setInfoAndEnter MethodType(paramSyms, AnyClass.tpe) - def abstractFunctionType = { - val sym = AbstractFunctionClass(arity) - typeRef(sym.typeConstructor.prefix, sym, targs.init :+ resTp) - } + val methodBodyTyper = newTyper(context.makeNewScope(context.tree, methodSym)) // should use the DefDef for the context's tree, but it doesn't exist yet (we need the typer we're creating to create it) + paramSyms foreach (methodBodyTyper.context.scope enter _) + + val (selector1, selectorTp, casesAdapted, resTp, doTranslation) = methodBodyTyper.prepareTranslateMatch(selector, cases, mode, ptRes) + val formalTypes = paramSyms map (_.tpe) val parents = - if (isFunctionType(pt)) List(abstractFunctionType, SerializableClass.tpe) - else if (isPartial) List(appliedType(AbstractPartialFunctionClass.typeConstructor, List(scrutTp, resTp)), SerializableClass.tpe) - else List(ObjectClass.tpe, pt, SerializableClass.tpe) + if (isPartial) List(appliedType(AbstractPartialFunctionClass.typeConstructor, List(formalTypes.head, resTp)), SerializableClass.tpe) + else List(appliedType(AbstractFunctionClass(arity).typeConstructor, formalTypes :+ resTp), SerializableClass.tpe) + + anonClass setInfo ClassInfoType(parents, newScope, anonClass) + methodSym setInfoAndEnter MethodType(paramSyms, resTp) - (atPos(tree.pos.focus)(DefDef(Modifiers(FINAL), nme.apply, Nil, List(params), TypeTree() setType resTp, body)), parents) + val body = methodBodyTyper.translateMatch(selector1, selectorTp, casesAdapted, resTp, doTranslation, if (isPartial) Some(scrut => (funThis DOT nme.missingCase) (scrut)) else None) + + DefDef(methodSym, body) } def isDefinedAtMethod = { - val (params, paramsRef) = mkParams - val (body, _) = newTyper(context.make(context.tree, idaSentinel)).translateMatch(paramsRef, casesTrue, mode, BooleanClass.tpe, scrutTp, Some(scrutinee => FALSE)) - atPos(tree.pos.focus)( - DefDef(Modifiers(FINAL), nme._isDefinedAt, Nil, List(params), TypeTree() setType BooleanClass.tpe, body) - ) - } + val methodSym = anonClass.newMethod(nme._isDefinedAt, tree.pos, FINAL) + val (paramSyms, selector) = mkParams(methodSym) + val methodBodyTyper = newTyper(context.makeNewScope(context.tree, methodSym)) // should use the DefDef for the context's tree, but it doesn't exist yet (we need the typer we're creating to create it) + paramSyms foreach (methodBodyTyper.context.scope enter _) + methodSym setInfoAndEnter MethodType(paramSyms, BooleanClass.tpe) - val members = if (!isPartial) List(applyMethod) else List(applyMethod, isDefinedAtMethod) + val (selector1, selectorTp, casesAdapted, resTp, doTranslation) = methodBodyTyper.prepareTranslateMatch(selector, casesTrue, mode, BooleanClass.tpe) + val body = methodBodyTyper.translateMatch(selector1, selectorTp, casesAdapted, resTp, doTranslation, Some(scrutinee => FALSE)) - val cmods = Modifiers(FINAL | SYNTHETIC /*TODO: when do we need INCONSTRUCTOR ?*/) withAnnotations ( - List(NEW(SerialVersionUIDAttr, LIT(0)))) - val cdef = - ClassDef(cmods, cname, Nil, - Template(parents map (TypeTree() setType _), emptyValDef, Modifiers(0), Nil, List(Nil), members, tree.pos) - ) - val funInst = (Block(List(cdef), Apply(Select(New(Ident(cname)), nme.CONSTRUCTOR), Nil))) - - val res = typed(funInst, mode, pt) - - // now that we have the symbols corresponding to the apply/isDefinedAt methods, - // we can fix up the result of fixerUpper... URGH - // fixerUpper nests the top-level definitions generated in the match under context.owner, but they should be owner by the apply/isDefinedAt method - res foreach { - case d: DefDef if (d.symbol.name == nme.apply) => - d.rhs.changeOwner(applySentinel -> d.symbol) - case d: DefDef if (d.symbol.name == nme._isDefinedAt) => - d.rhs.changeOwner(idaSentinel -> d.symbol) - case _ => + DefDef(methodSym, body) } - selOverride match { - case None => res - case Some((paramSyms, sel)) => - object substParamSyms extends Transformer { - override def transform(t: Tree): Tree = t match { - case d: DefDef if (d.symbol.name == nme.apply) || (d.symbol.name == nme._isDefinedAt) && (d.symbol.owner == res.tpe.typeSymbol) => - deriveDefDef(d)(rhs => rhs.substTreeSyms(paramSyms, d.vparamss.head.map(_.symbol))) - case _ => - super.transform(t) - } - } - substParamSyms.transform(res) - } + val members = if (!isPartial) List(applyMethod) else List(applyMethod, isDefinedAtMethod) + typed(Block(List(ClassDef(anonClass, NoMods, List(List()), List(List()), members, tree.pos)), New(anonClass.tpe)), mode, pt) } /** @@ -2296,7 +2273,7 @@ trait Typers extends Modes with Adaptations with PatMatVirtualiser { if (argpts.lengthCompare(numVparams) != 0) WrongNumberOfParametersError(fun, argpts) else { - val vparamSyms = map2(fun.vparams, argpts) { (vparam, argpt) => + foreach2(fun.vparams, argpts) { (vparam, argpt) => if (vparam.tpt.isEmpty) { vparam.tpt.tpe = if (isFullyDefined(argpt)) argpt @@ -2320,30 +2297,26 @@ trait Typers extends Modes with Adaptations with PatMatVirtualiser { } if (!vparam.tpt.pos.isDefined) vparam.tpt setPos vparam.pos.focus } - enterSym(context, vparam) - if (context.retyping) context.scope enter vparam.symbol - vparam.symbol } - val vparams = fun.vparams mapConserve (typedValDef) -// for (vparam <- vparams) { -// checkNoEscaping.locals(context.scope, WildcardType, vparam.tpt); () -// } - - def recompose(from: Type, to: Type) = - if(clazz == PartialFunctionClass) appliedType(PartialFunctionClass.typeConstructor, List(from, to)) - else functionType(List(from), to) - fun.body match { case Match(sel, cases) if opt.virtPatmat => - val typedSel = typed(sel, EXPRmode | BYVALmode, WildcardType) // go to outer context -- must discard the context that was created for the Function since we're discarding the function // thus, its symbol, which serves as the current context.owner, is not the right owner // you won't know you're using the wrong owner until lambda lift crashes (unless you know better than to use the wrong owner) - newTyper(context.outer).typedMatchAnonFun(fun, cases, mode, recompose(typedSel.tpe, respt), Some((vparamSyms, typedSel))) + newTyper(context.outer).typedMatchAnonFun(fun, cases, mode, pt, Some((fun.vparams, sel))) case _ => - val body1 = typed(fun.body, respt) + val vparamSyms = fun.vparams map { vparam => + enterSym(context, vparam) + if (context.retyping) context.scope enter vparam.symbol + vparam.symbol + } + val vparams = fun.vparams mapConserve (typedValDef) + // for (vparam <- vparams) { + // checkNoEscaping.locals(context.scope, WildcardType, vparam.tpt); () + // } val formals = vparamSyms map (_.tpe) + val body1 = typed(fun.body, respt) val restpe = packedType(body1, fun.symbol).deconst.resultType val funtpe = typeRef(clazz.tpe.prefix, clazz, formals :+ restpe) // body = checkNoEscaping.locals(context.scope, restpe, body) @@ -3579,8 +3552,10 @@ trait Typers extends Modes with Adaptations with PatMatVirtualiser { def typedMatch(tree: Tree, selector: Tree, cases: List[CaseDef]): Tree = { if (opt.virtPatmat && !isPastTyper) { - if (selector ne EmptyTree) typed(translateMatch(selector, cases, mode, pt)._1, mode, pt) - else typedMatchAnonFun(tree, cases, mode, pt) + if (selector ne EmptyTree) { + val (selector1, selectorTp, casesAdapted, ownType, doTranslation) = prepareTranslateMatch(selector, cases, mode, pt) + typed(translateMatch(selector1, selectorTp, casesAdapted, ownType, doTranslation), mode, pt) + } else typedMatchAnonFun(tree, cases, mode, pt) } else if (selector == EmptyTree) { val arity = if (isFunctionType(pt)) pt.normalize.typeArgs.length - 1 else 1 val params = for (i <- List.range(0, arity)) yield -- cgit v1.2.3 From f8022c216d84af482b27c04c3c38a2d8dce9a192 Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Wed, 21 Mar 2012 19:13:29 +0100 Subject: don't throw error in setError when -Ydebug --- src/compiler/scala/tools/nsc/typechecker/Infer.scala | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/typechecker/Infer.scala b/src/compiler/scala/tools/nsc/typechecker/Infer.scala index a59622d4df..a1ca4904f4 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Infer.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Infer.scala @@ -196,10 +196,12 @@ trait Infer { /* -- Error Messages --------------------------------------------------- */ def setError[T <: Tree](tree: T): T = { - if (settings.debug.value) { // DEBUG - println("set error: "+tree); - throw new Error() - } + debuglog("set error: "+ tree) + // this breaks -Ydebug pretty radically + // if (settings.debug.value) { // DEBUG + // println("set error: "+tree); + // throw new Error() + // } def name = newTermName("") def errorClass = if (context.reportErrors) context.owner.newErrorClass(name.toTypeName) else stdErrorClass def errorValue = if (context.reportErrors) context.owner.newErrorValue(name) else stdErrorValue -- cgit v1.2.3 From d786f269834c89e6de4a6a90e7a9f22c583bc30a Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Thu, 22 Mar 2012 10:39:29 +0100 Subject: [vpm] avoid verifyerror: leave jump to tail-pos label the following commit deals with the fall-out in basicblocks (double closing of blocks in ignore mode) --- src/compiler/scala/tools/nsc/transform/TailCalls.scala | 9 +++++++-- test/files/run/virtpatmat_tailcalls_verifyerror.check | 1 + test/files/run/virtpatmat_tailcalls_verifyerror.flags | 1 + test/files/run/virtpatmat_tailcalls_verifyerror.scala | 13 +++++++++++++ 4 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 test/files/run/virtpatmat_tailcalls_verifyerror.check create mode 100644 test/files/run/virtpatmat_tailcalls_verifyerror.flags create mode 100644 test/files/run/virtpatmat_tailcalls_verifyerror.scala (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/transform/TailCalls.scala b/src/compiler/scala/tools/nsc/transform/TailCalls.scala index ef76fe1b1c..9915f7e9fc 100644 --- a/src/compiler/scala/tools/nsc/transform/TailCalls.scala +++ b/src/compiler/scala/tools/nsc/transform/TailCalls.scala @@ -341,13 +341,18 @@ abstract class TailCalls extends Transform { else if (fun.symbol.isLabel && args.nonEmpty && args.tail.isEmpty && ctx.tailLabels(fun.symbol)) { // this is to detect tailcalls in translated matches // it's a one-argument call to a label that is in a tailposition and that looks like label(x) {x} - // thus, the argument to the call is in tailposition and we don't need to jump to the label, tail jump instead + // thus, the argument to the call is in tailposition val saved = ctx.tailPos ctx.tailPos = true debuglog("in tailpos label: "+ args.head) val res = transform(args.head) ctx.tailPos = saved - if (res ne args.head) res // we tail-called -- TODO: shield from false-positives where we rewrite but don't tail-call + if (res ne args.head) { + // we tail-called -- TODO: shield from false-positives where we rewrite but don't tail-call + // must leave the jump to the original tailpos-label (fun)! + // there might be *a* tailcall *in* res, but it doesn't mean res *always* tailcalls + treeCopy.Apply(tree, fun, List(res)) + } else rewriteApply(fun, fun, Nil, args) } else rewriteApply(fun, fun, Nil, args) diff --git a/test/files/run/virtpatmat_tailcalls_verifyerror.check b/test/files/run/virtpatmat_tailcalls_verifyerror.check new file mode 100644 index 0000000000..c508d5366f --- /dev/null +++ b/test/files/run/virtpatmat_tailcalls_verifyerror.check @@ -0,0 +1 @@ +false diff --git a/test/files/run/virtpatmat_tailcalls_verifyerror.flags b/test/files/run/virtpatmat_tailcalls_verifyerror.flags new file mode 100644 index 0000000000..9769db9257 --- /dev/null +++ b/test/files/run/virtpatmat_tailcalls_verifyerror.flags @@ -0,0 +1 @@ + -Yvirtpatmat -Xexperimental diff --git a/test/files/run/virtpatmat_tailcalls_verifyerror.scala b/test/files/run/virtpatmat_tailcalls_verifyerror.scala new file mode 100644 index 0000000000..1ee613f09e --- /dev/null +++ b/test/files/run/virtpatmat_tailcalls_verifyerror.scala @@ -0,0 +1,13 @@ +// shouldn't result in a verify error when run... +object Test extends App { + @annotation.tailrec + final def test(meh: Boolean): Boolean = { + Some("a") match { + case x => + x match { + case _ => if(meh) test(false) else false + } + } + } + println(test(true)) +} \ No newline at end of file -- cgit v1.2.3 From 09eda4ef92168685ef3d301439bb8b6df76f982e Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Thu, 22 Mar 2012 17:36:12 +0100 Subject: do nothing when closing closed block in ignoremode this came to light with the virtual pattern matcher, which emits jumps like `matchEnd3(_test(Test.this, false))`, where _test is a tailcall the nested jumping caused double-closing (the second time in ignore mode) thus. when closing a closed block in ignore mode, simply do nothing from genLoad for label-jumps: note: when one of the args to genLoadLabelArguments is a jump to a label, it will call back into genLoad and arrive at this case, which will then set ctx1.bb.ignore to true, this is okay, since we're jumping unconditionally, so the loads and jumps emitted by the outer call to genLoad (by calling genLoadLabelArguments and emitOnly) can safely be ignored, however, as emitOnly will close the block, which reverses its instructions (when it's still open), we better not reverse when the block has already been closed but is in ignore mode (if it's not in ignore mode, double-closing is an error) @dragos figured it out, all I did was write the comment and the `if` test case to repro basic blocks crasher the tailcall in the forward jump `matchEnd3(_test(Test.this, false))` in the following program crashes the back-end (error below) @scala.annotation.tailrec final def test(meh: Boolean): Boolean = { val _$this: Test.type = Test.this; _test(_$this,meh){ case val x1: Some[String] = new Some[String]("a"); case3(){ matchEnd2({ case val x1: Some[String] = x1; case4(){ if (x1.ne(null)) matchEnd3(if (meh) _test(Test.this, false) else false) else case5() }; case5(){ matchEnd3(_test(Test.this, false)) }; matchEnd3(x){ x } }) }; matchEnd2(x){ x } } }; The last instruction (of basic block 11) is not a control flow instruction: CONSTANT(false) // methods def test(meh: Boolean (BOOL)): Boolean { locals: value meh, value _$this, value x1, value x, value x, value x1 startBlock: 1 blocks: [1,2,3,4,5,6,7,8,9,10,11,12,13] 1: 4 JUMP 2 2: 5 NEW REF(class Some) 5 DUP(REF(class Some)) 5 CONSTANT("a") 5 CALL_METHOD scala.Some. (static-instance) 5 STORE_LOCAL(value x1) 5 SCOPE_ENTER value x1 5 JUMP 3 3: 5 LOAD_LOCAL(value x1) 7 STORE_LOCAL(value x1) 7 SCOPE_ENTER value x1 7 JUMP 4 4: 7 LOAD_LOCAL(value x1) 7 CZJUMP (REF(class Object))NE ? 5 : 6 5: 8 LOAD_LOCAL(value meh) 8 CZJUMP (BOOL)NE ? 8 : 9 6: ? JUMP 11 7: 7 DROP BOOL 7 JUMP 11 8: 8 CONSTANT(false) 8 STORE_LOCAL(value meh) 8 JUMP 2 9: 8 CONSTANT(false) 8 JUMP 10 10: 8 STORE_LOCAL(value x) 8 JUMP 12 11: 9 JUMP 2 9 STORE_LOCAL(value meh) 9 CONSTANT(false) 12: 7 LOAD_LOCAL(value x) 7 SCOPE_EXIT value x1 7 STORE_LOCAL(value x) 7 JUMP 13 13: 5 LOAD_LOCAL(value x) 5 SCOPE_EXIT value x1 5 RETURN(BOOL) --- .../scala/tools/nsc/backend/icode/BasicBlocks.scala | 14 ++++++++++---- src/compiler/scala/tools/nsc/backend/icode/GenICode.scala | 7 +++++++ test/files/run/virtpatmat_tailcalls_verifyerror.scala | 3 ++- 3 files changed, 19 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/backend/icode/BasicBlocks.scala b/src/compiler/scala/tools/nsc/backend/icode/BasicBlocks.scala index 68c4ac03f6..4f3b0bf951 100644 --- a/src/compiler/scala/tools/nsc/backend/icode/BasicBlocks.scala +++ b/src/compiler/scala/tools/nsc/backend/icode/BasicBlocks.scala @@ -386,10 +386,16 @@ trait BasicBlocks { def close() { assert(!closed || ignore, this) assert(instructionList.nonEmpty, "Empty block: " + this) - closed = true - setFlag(DIRTYSUCCS) - instructionList = instructionList.reverse - instrs = instructionList.toArray + if (ignore && closed) { // redundant `ignore &&` for clarity -- we should never be in state `!ignore && closed` + // not doing anything to this block is important... + // because the else branch reverses innocent blocks, which is wrong when they're in ignore mode (and closed) + // reversing the instructions when (closed && ignore) wreaks havoc for nested label jumps (see comments in genLoad) + } else { + closed = true + setFlag(DIRTYSUCCS) + instructionList = instructionList.reverse + instrs = instructionList.toArray + } } def open() { diff --git a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala index 9e801e3ea8..41d9d93e7a 100644 --- a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala +++ b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala @@ -868,6 +868,13 @@ abstract class GenICode extends SubComponent { abort("Unknown label target: " + sym + " at: " + (fun.pos) + ": ctx: " + ctx) } }) + // note: when one of the args to genLoadLabelArguments is a jump to a label, + // it will call back into genLoad and arrive at this case, which will then set ctx1.bb.ignore to true, + // this is okay, since we're jumping unconditionally, so the loads and jumps emitted by the outer + // call to genLoad (by calling genLoadLabelArguments and emitOnly) can safely be ignored, + // however, as emitOnly will close the block, which reverses its instructions (when it's still open), + // we better not reverse when the block has already been closed but is in ignore mode + // (if it's not in ignore mode, double-closing is an error) val ctx1 = genLoadLabelArguments(args, label, ctx) ctx1.bb.emitOnly(if (label.anchored) JUMP(label.block) else PJUMP(label)) ctx1.bb.enterIgnoreMode diff --git a/test/files/run/virtpatmat_tailcalls_verifyerror.scala b/test/files/run/virtpatmat_tailcalls_verifyerror.scala index 1ee613f09e..5ce91e8dce 100644 --- a/test/files/run/virtpatmat_tailcalls_verifyerror.scala +++ b/test/files/run/virtpatmat_tailcalls_verifyerror.scala @@ -5,7 +5,8 @@ object Test extends App { Some("a") match { case x => x match { - case _ => if(meh) test(false) else false + case Some(_) => if(meh) test(false) else false + case _ => test(false) } } } -- cgit v1.2.3 From 20426fb137ac9dfc2fb87adaaedb0646c0413320 Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Fri, 23 Mar 2012 12:39:46 +0100 Subject: [vpm] be more careful about swatches type-switch-based catches must not contain type tests on traits for some reason, so fall back to full-blown pattern match TODO: can we improve this? this simply mimics the old pattern matcher's behavior --- src/compiler/scala/tools/nsc/typechecker/PatMatVirtualiser.scala | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/typechecker/PatMatVirtualiser.scala b/src/compiler/scala/tools/nsc/typechecker/PatMatVirtualiser.scala index 2b67f7f44b..5452d13ef0 100644 --- a/src/compiler/scala/tools/nsc/typechecker/PatMatVirtualiser.scala +++ b/src/compiler/scala/tools/nsc/typechecker/PatMatVirtualiser.scala @@ -149,7 +149,7 @@ trait PatMatVirtualiser extends ast.TreeDSL { self: Analyzer => // if they're already simple enough to be handled by the back-end, we're done if (caseDefs forall treeInfo.isCatchCase) caseDefs else { - val switch = { + val swatches = { // switch-catches val bindersAndCases = caseDefs map { caseDef => // generate a fresh symbol for each case, hoping we'll end up emitting a type-switch (we don't have a global scrut there) // if we fail to emit a fine-grained switch, have to do translateCase again with a single scrutSym (TODO: uniformize substitution on treemakers so we can avoid this) @@ -157,10 +157,12 @@ trait PatMatVirtualiser extends ast.TreeDSL { self: Analyzer => (caseScrutSym, propagateSubstitution(translateCase(caseScrutSym, pt)(caseDef), EmptySubstitution)) } - (emitTypeSwitch(bindersAndCases, pt) map (_.map(fixerUpper(matchOwner, pos).apply(_).asInstanceOf[CaseDef]))) + for(cases <- emitTypeSwitch(bindersAndCases, pt) toList; + if cases forall treeInfo.isCatchCase; // must check again, since it's not guaranteed -- TODO: can we eliminate this? e.g., a type test could test for a trait or a non-trivial prefix, which are not handled by the back-end + cse <- cases) yield fixerUpper(matchOwner, pos)(cse).asInstanceOf[CaseDef] } - val catches = switch getOrElse { + val catches = if (swatches nonEmpty) swatches else { val scrutSym = freshSym(pos, pureType(ThrowableClass.tpe)) val casesNoSubstOnly = caseDefs map { caseDef => (propagateSubstitution(translateCase(scrutSym, pt)(caseDef), EmptySubstitution))} -- cgit v1.2.3