From 0391436aa1bfd1b9fabaf9d93e8c077dbea53a38 Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Tue, 24 Jul 2012 14:32:04 +0200 Subject: move synthetic case symbol detection to treeInfo encapsulate creating synthetic case labels while we're at it --- src/compiler/scala/tools/nsc/ast/TreeGen.scala | 6 ++---- src/compiler/scala/tools/nsc/transform/TailCalls.scala | 2 +- .../scala/tools/nsc/typechecker/PatternMatching.scala | 15 +++++++++------ src/compiler/scala/tools/nsc/typechecker/RefChecks.scala | 2 +- .../scala/tools/selectivecps/SelectiveANFTransform.scala | 10 +++++----- src/reflect/scala/reflect/internal/TreeInfo.scala | 10 ++++++++++ 6 files changed, 28 insertions(+), 17 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/ast/TreeGen.scala b/src/compiler/scala/tools/nsc/ast/TreeGen.scala index be5909a67f..144fb0d5ec 100644 --- a/src/compiler/scala/tools/nsc/ast/TreeGen.scala +++ b/src/compiler/scala/tools/nsc/ast/TreeGen.scala @@ -72,8 +72,6 @@ abstract class TreeGen extends reflect.internal.TreeGen with TreeDSL { Annotated(Ident(nme.synthSwitch), expr) } - 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 { @@ -94,13 +92,13 @@ abstract class TreeGen extends reflect.internal.TreeGen with TreeDSL { case Apply(Apply(TypeApply(Select(tgt, nme.runOrElse), targs), List(scrut)), List(matcher)) if opt.virtPatmat => // println("virt match: "+ (tgt, targs, scrut, matcher) + "for:\n"+ matchExpr ) caseVirtualizedMatch(matchExpr, tgt, targs, scrut, matcher) // optimized version of virtpatmat - case Block(stats, matchEndDef) if opt.virtPatmat && (stats forall hasSynthCaseSymbol) => + case Block(stats, matchEndDef) if opt.virtPatmat && (stats forall treeInfo.hasSynthCaseSymbol) => // the assumption is once we encounter a case, the remainder of the block will consist of cases // the prologue may be empty, usually it is the valdef that stores the scrut val (prologue, cases) = stats span (s => !s.isInstanceOf[LabelDef]) caseVirtualizedMatchOpt(matchExpr, prologue, cases, matchEndDef, identity) // optimized version of virtpatmat - case Block(outerStats, orig@Block(stats, matchEndDef)) if opt.virtPatmat && (stats forall hasSynthCaseSymbol) => + case Block(outerStats, orig@Block(stats, matchEndDef)) if opt.virtPatmat && (stats forall treeInfo.hasSynthCaseSymbol) => val (prologue, cases) = stats span (s => !s.isInstanceOf[LabelDef]) caseVirtualizedMatchOpt(matchExpr, prologue, cases, matchEndDef, m => copyBlock(matchExpr, outerStats, m)) case other => diff --git a/src/compiler/scala/tools/nsc/transform/TailCalls.scala b/src/compiler/scala/tools/nsc/transform/TailCalls.scala index 492273dfdc..770aa8b7ac 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 { } } - import gen.hasSynthCaseSymbol + import treeInfo.hasSynthCaseSymbol /** * A Tail Call Transformer diff --git a/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala b/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala index 8b7c70c048..256d8db095 100644 --- a/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala +++ b/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala @@ -113,7 +113,6 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL import definitions._ import analyzer._ //Typer - val SYNTH_CASE = Flags.CASE | SYNTHETIC case class DefaultOverrideMatchAttachment(default: Tree) @@ -241,7 +240,8 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL // append the default to the list of cases and suppress the unreachable case error that may arise (once we detect that...) val matchFailGenOverride = match_.attachments.get[DefaultOverrideMatchAttachment].map{case DefaultOverrideMatchAttachment(default) => ((scrut: Tree) => default)} - val selectorSym = freshSym(selector.pos, pureType(selectorTp)) setFlag SYNTH_CASE + val selectorSym = freshSym(selector.pos, pureType(selectorTp)) setFlag treeInfo.SYNTH_CASE_FLAGS + // pt = Any* occurs when compiling test/files/pos/annotDepMethType.scala with -Xexperimental val combined = combineCases(selector, selectorSym, cases map translateCase(selectorSym, pt), pt, matchOwner, matchFailGenOverride) @@ -1316,6 +1316,9 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL def freshSym(pos: Position, tp: Type = NoType, prefix: String = "x") = NoSymbol.newTermSymbol(freshName(prefix), pos) setInfo tp + def newSynthCaseLabel(name: String) = + NoSymbol.newLabel(freshName(name), NoPosition) setFlag treeInfo.SYNTH_CASE_FLAGS + // codegen relevant to the structure of the translation (how extractors are combined) trait AbsCodegen { def matcher(scrut: Tree, scrutSym: Symbol, restpe: Type)(cases: List[Casegen => Tree], matchFailGen: Option[Tree => Tree]): Tree @@ -1486,7 +1489,7 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL case object FalseCond extends Cond {override def toString = "F"} case class AndCond(a: Cond, b: Cond) extends Cond {override def toString = a +"/\\"+ b} - case class OrCond(a: Cond, b: Cond) extends Cond {override def toString = "("+a+") \\/ ("+ b +")"} + case class OrCond(a: Cond, b: Cond) extends Cond {override def toString = "("+a+") \\/ ("+ b +")"} object EqualityCond { private val uniques = new collection.mutable.HashMap[(Tree, Tree), EqualityCond] @@ -3108,7 +3111,7 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL } } - private val defaultLabel: Symbol = NoSymbol.newLabel(freshName("default"), NoPosition) setFlag SYNTH_CASE + private val defaultLabel: Symbol = newSynthCaseLabel("default") /** Collapse guarded cases that switch on the same constant (the last case may be unguarded). * @@ -3476,11 +3479,11 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL * 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 SYNTH_CASE + val matchEnd = newSynthCaseLabel("matchEnd") val matchRes = NoSymbol.newValueParameter(newTermName("x"), NoPosition, SYNTHETIC) setInfo restpe.withoutAnnotations // matchEnd setInfo MethodType(List(matchRes), restpe) - def newCaseSym = NoSymbol.newLabel(freshName("case"), NoPosition) setInfo MethodType(Nil, restpe) setFlag SYNTH_CASE + def newCaseSym = newSynthCaseLabel("case") setInfo MethodType(Nil, restpe) var _currCase = newCaseSym val caseDefs = cases map { (mkCase: Casegen => Tree) => diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala index 3518316fbb..6912e5354f 100644 --- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala +++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala @@ -1718,7 +1718,7 @@ abstract class RefChecks extends InfoTransform with reflect.internal.transform.R val pat1 = transform(pat) inPattern = false treeCopy.CaseDef(tree, pat1, transform(guard), transform(body)) - case LabelDef(_, _, _) if gen.hasSynthCaseSymbol(result) => + case LabelDef(_, _, _) if treeInfo.hasSynthCaseSymbol(result) => val old = inPattern inPattern = true val res = deriveLabelDef(result)(transform) diff --git a/src/continuations/plugin/scala/tools/selectivecps/SelectiveANFTransform.scala b/src/continuations/plugin/scala/tools/selectivecps/SelectiveANFTransform.scala index 017c8d24fd..7760568ac3 100644 --- a/src/continuations/plugin/scala/tools/selectivecps/SelectiveANFTransform.scala +++ b/src/continuations/plugin/scala/tools/selectivecps/SelectiveANFTransform.scala @@ -99,11 +99,11 @@ abstract class SelectiveANFTransform extends PluginComponent with Transform with treeCopy.Block(body, List(transform(selDef)), transformPureMatch(mat, selector, cases)) // virtpatmat - case b@Block(matchStats@((selDef: ValDef) :: cases), matchEnd) if ext.isDefined && pureBody && (matchStats forall gen.hasSynthCaseSymbol) => + case b@Block(matchStats@((selDef: ValDef) :: cases), matchEnd) if ext.isDefined && pureBody && (matchStats forall treeInfo.hasSynthCaseSymbol) => transformPureVirtMatch(b, selDef, cases, matchEnd) // virtpatmat that stores the scrut separately -- TODO: can we eliminate this case?? - case Block(List(selDef0: ValDef), mat@Block(matchStats@((selDef: ValDef) :: cases), matchEnd)) if ext.isDefined && pureBody && (matchStats forall gen.hasSynthCaseSymbol)=> + case Block(List(selDef0: ValDef), mat@Block(matchStats@((selDef: ValDef) :: cases), matchEnd)) if ext.isDefined && pureBody && (matchStats forall treeInfo.hasSynthCaseSymbol)=> treeCopy.Block(body, List(transform(selDef0)), transformPureVirtMatch(mat, selDef, cases, matchEnd)) case _ => @@ -253,7 +253,7 @@ abstract class SelectiveANFTransform extends PluginComponent with Transform with // calling each labeldef is wrong, since some labels may be jumped over // we can get away with this for now since the only other labels we emit are for tailcalls/while loops, // which do not have consecutive labeldefs (and thus fall-through is irrelevant) - if (gen.hasSynthCaseSymbol(ldef)) (List(stm1), localTyper.typed{Literal(Constant(()))}, cpsA) + if (treeInfo.hasSynthCaseSymbol(ldef)) (List(stm1), localTyper.typed{Literal(Constant(()))}, cpsA) else { assert(params.isEmpty, "problem in ANF transforming label with non-empty params "+ ldef) (List(stm1), localTyper.typed{Apply(Ident(sym), List())}, cpsA) @@ -469,9 +469,9 @@ abstract class SelectiveANFTransform extends PluginComponent with Transform with val (anfStats, anfExpr) = rec(stms, cpsA, List()) // println("\nanf-block:\n"+ ((stms :+ expr) mkString ("{", "\n", "}")) +"\nBECAME\n"+ ((anfStats :+ anfExpr) mkString ("{", "\n", "}"))) - // println("synth case? "+ (anfStats map (t => (t, t.isDef, gen.hasSynthCaseSymbol(t))))) + // println("synth case? "+ (anfStats map (t => (t, t.isDef, treeInfo.hasSynthCaseSymbol(t))))) // SUPER UGLY HACK: handle virtpatmat-style matches, whose labels have already been turned into DefDefs - if (anfStats.nonEmpty && (anfStats forall (t => !t.isDef || gen.hasSynthCaseSymbol(t)))) { + if (anfStats.nonEmpty && (anfStats forall (t => !t.isDef || treeInfo.hasSynthCaseSymbol(t)))) { val (prologue, rest) = (anfStats :+ anfExpr) span (s => !s.isInstanceOf[DefDef]) // find first case // println("rest: "+ rest) // val (defs, calls) = rest partition (_.isInstanceOf[DefDef]) diff --git a/src/reflect/scala/reflect/internal/TreeInfo.scala b/src/reflect/scala/reflect/internal/TreeInfo.scala index 7ba749ed2c..17a01e1af9 100644 --- a/src/reflect/scala/reflect/internal/TreeInfo.scala +++ b/src/reflect/scala/reflect/internal/TreeInfo.scala @@ -464,6 +464,16 @@ abstract class TreeInfo { case _ => false } + + // used in the symbols for labeldefs and valdefs emitted by the pattern matcher + // tailcalls, cps,... use this flag combination to detect translated matches + // TODO: move to Flags + final val SYNTH_CASE_FLAGS = CASE | SYNTHETIC + + def isSynthCaseSymbol(sym: Symbol) = sym hasAllFlags SYNTH_CASE_FLAGS + def hasSynthCaseSymbol(t: Tree) = t.symbol != null && isSynthCaseSymbol(t.symbol) + + /** The method part of an application node */ def methPart(tree: Tree): Tree = tree match { -- cgit v1.2.3 From 115eede127ad96f65b5aa3943e7a2334d75c7d6b Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Tue, 24 Jul 2012 10:13:22 +0200 Subject: SI-5897 don't check sensicality in match the pattern matching analysis should be more precise anyway (don't warn twice) --- src/compiler/scala/tools/nsc/typechecker/RefChecks.scala | 3 ++- test/files/pos/t5897.flags | 1 + test/files/pos/t5897.scala | 6 ++++++ 3 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 test/files/pos/t5897.flags create mode 100644 test/files/pos/t5897.scala (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala index 6912e5354f..254a2f2376 100644 --- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala +++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala @@ -1541,7 +1541,8 @@ abstract class RefChecks extends InfoTransform with reflect.internal.transform.R transform(qual) case Apply(fn, args) => - checkSensible(tree.pos, fn, args) + // sensicality should be subsumed by the unreachability/exhaustivity/irrefutability analyses in the pattern matcher + if (!inPattern) checkSensible(tree.pos, fn, args) currentApplication = tree tree } diff --git a/test/files/pos/t5897.flags b/test/files/pos/t5897.flags new file mode 100644 index 0000000000..e8fb65d50c --- /dev/null +++ b/test/files/pos/t5897.flags @@ -0,0 +1 @@ +-Xfatal-warnings \ No newline at end of file diff --git a/test/files/pos/t5897.scala b/test/files/pos/t5897.scala new file mode 100644 index 0000000000..2e9751afe0 --- /dev/null +++ b/test/files/pos/t5897.scala @@ -0,0 +1,6 @@ +// no warning here +// (strangely, if there's an unreachable code warning *anywhere in this compilation unit*, +// the non-sensical warning goes away under -Xfatal-warnings) +class Test { + () match { case () => } +} -- cgit v1.2.3 From 82dea8af9612ed26ebb01375f369d0c6033662ba Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Tue, 24 Jul 2012 10:36:56 +0200 Subject: SI-5930 don't warn about dead code in jump to case --- src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala | 6 +++++- test/files/pos/t5930.flags | 1 + test/files/pos/t5930.scala | 4 ++++ 3 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 test/files/pos/t5930.flags create mode 100644 test/files/pos/t5930.scala (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala b/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala index 38c2c5f719..74c51ece9f 100644 --- a/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala +++ b/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala @@ -401,7 +401,11 @@ trait TypeDiagnostics { object checkDead { private var expr: Symbol = NoSymbol - private def exprOK = expr != Object_synchronized + + private def exprOK = + (expr != Object_synchronized) && + !(expr.isLabel && treeInfo.isSynthCaseSymbol(expr)) // it's okay to jump to matchEnd (or another case) with an argument of type nothing + private def treeOK(tree: Tree) = tree.tpe != null && tree.tpe.typeSymbol == NothingClass def updateExpr(fn: Tree) = { diff --git a/test/files/pos/t5930.flags b/test/files/pos/t5930.flags new file mode 100644 index 0000000000..c7d406c649 --- /dev/null +++ b/test/files/pos/t5930.flags @@ -0,0 +1 @@ +-Ywarn-dead-code -Xfatal-warnings \ No newline at end of file diff --git a/test/files/pos/t5930.scala b/test/files/pos/t5930.scala new file mode 100644 index 0000000000..de9d62cfe8 --- /dev/null +++ b/test/files/pos/t5930.scala @@ -0,0 +1,4 @@ +// should not warn about dead code (`matchEnd(throw new MatchError)`) + class Test { + 0 match { case x: Int => } +} \ No newline at end of file -- cgit v1.2.3 From 75eb8a49842075bcaa2878b262443b48aafec2ab Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Tue, 24 Jul 2012 14:55:10 +0200 Subject: use hasAllFlags to detect gadt skolems trying to compromise between - easy discovery of what special mix of flags identifies a gadt skolem - ensuring that hasAllFlags is used and not hasFlag keeping the secret combo close to the creator/detector methods instead of moving them to Flags ideally, we'd allocate a new bit in Flags, but that's to invasive right now these symbols are also relatively short-lived: thet are created in adaptConstrPattern and removed at the end of typedCase --- src/reflect/scala/reflect/internal/Symbols.scala | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala index 10d02376b1..b82d483893 100644 --- a/src/reflect/scala/reflect/internal/Symbols.scala +++ b/src/reflect/scala/reflect/internal/Symbols.scala @@ -585,10 +585,16 @@ trait Symbols extends api.Symbols { self: SymbolTable => skolem setInfo (basis.info cloneInfo skolem) } + // don't test directly -- use isGADTSkolem + // used to single out a gadt skolem symbol in deskolemizeGADT + // gadtskolems are created in adaptConstrPattern and removed at the end of typedCase + @inline final protected[Symbols] def GADT_SKOLEM_FLAGS = CASEACCESSOR | SYNTHETIC + // flags set up to maintain TypeSkolem's invariant: origin.isInstanceOf[Symbol] == !hasFlag(EXISTENTIAL) - // CASEACCESSOR | SYNTHETIC used to single this symbol out in deskolemizeGADT + // GADT_SKOLEM_FLAGS (== CASEACCESSOR | SYNTHETIC) used to single this symbol out in deskolemizeGADT + // TODO: it would be better to allocate a new bit in the flag long for GADTSkolem rather than OR'ing together CASEACCESSOR | SYNTHETIC def newGADTSkolem(name: TypeName, origin: Symbol, info: Type): TypeSkolem = - newTypeSkolemSymbol(name, origin, origin.pos, origin.flags & ~(EXISTENTIAL | PARAM) | CASEACCESSOR | SYNTHETIC) setInfo info + newTypeSkolemSymbol(name, origin, origin.pos, origin.flags & ~(EXISTENTIAL | PARAM) | GADT_SKOLEM_FLAGS) setInfo info final def freshExistential(suffix: String): TypeSymbol = newExistential(freshExistentialName(suffix), pos) @@ -2958,7 +2964,7 @@ trait Symbols extends api.Symbols { self: SymbolTable => // a type symbol bound by an existential type, for instance the T in // List[T] forSome { type T } override def isExistentialSkolem = this hasFlag EXISTENTIAL - override def isGADTSkolem = this hasFlag CASEACCESSOR | SYNTHETIC + override def isGADTSkolem = this hasAllFlags GADT_SKOLEM_FLAGS override def isTypeSkolem = this hasFlag PARAM override def isAbstractType = this hasFlag DEFERRED -- cgit v1.2.3