From 91c34cedc9ee54244f6b49dda691cbe0be182037 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Fri, 10 Oct 2014 11:24:14 +1000 Subject: SI-3439 Fix use of implicit constructor params in super call When typechecking the primary constructor body, the symbols of constructor parameters of a class are owned by the class's owner. This is done make scoping work; you shouldn't be able to refer to class members in that position. However, other parts of the compiler weren't so happy about this arrangement. The enclosed test case shows that our checks for invalid, top-level implicits was spuriously triggered, and implicit search itself would fail. Furthermore, we had to hack `Run#compiles` to special case top-level early-initialized symbols. See SI-7264 / 86e6e9290. This commit: - introduces an intermediate local dummy term symbol which will act as the owner for constructor parameters and early initialized members - adds this to the `Run#symSource` map if it is top level - simplifies `Run#compiles` accordingly - tests this all in a top-level class, and one nested in another class. --- test/files/neg/t7636.check | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'test/files/neg') diff --git a/test/files/neg/t7636.check b/test/files/neg/t7636.check index f70d50bee3..12391cccc8 100644 --- a/test/files/neg/t7636.check +++ b/test/files/neg/t7636.check @@ -4,7 +4,7 @@ t7636.scala:3: error: illegal inheritance; ^ t7636.scala:3: error: type mismatch; found : Either[_$2,_$3(in constructor C)] where type _$3(in constructor C), type _$2 - required: Either[_, _$3(in object Main)] where type _$3(in object Main) + required: Either[_, _$3(in value )] where type _$3(in value ) class C extends ResultTable(Left(5):Either[_,_])(5) ^ two errors found -- cgit v1.2.3 From 5f6663ee19a95289b5466dab6b1caa92c579bc73 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Fri, 10 Oct 2014 16:02:17 +1000 Subject: SI-5091 Move named-args cycle test from pending to neg There is a typechecking cycle, and since 4669ac180e5 we now report this in a clearer manner. Once we change the language to unconditionally interpret an assignent in argument position as a named argument would be the only way to get this over to `pos`. --- test/files/neg/t5091.check | 9 +++++++++ test/files/neg/t5091.scala | 11 +++++++++++ test/pending/pos/t5091.scala | 11 ----------- 3 files changed, 20 insertions(+), 11 deletions(-) create mode 100644 test/files/neg/t5091.check create mode 100644 test/files/neg/t5091.scala delete mode 100644 test/pending/pos/t5091.scala (limited to 'test/files/neg') diff --git a/test/files/neg/t5091.check b/test/files/neg/t5091.check new file mode 100644 index 0000000000..abd24e3145 --- /dev/null +++ b/test/files/neg/t5091.check @@ -0,0 +1,9 @@ +t5091.scala:8: error: recursive value xxx needs type + val param = bar(xxx) + ^ +t5091.scala:7: warning: type-checking the invocation of method foo checks if the named argument expression 'param = ...' is a valid assignment +in the current scope. The resulting type inference error (see above) can be fixed by providing an explicit type in the local definition for param. + val xxx = foo(param = null) + ^ +one warning found +one error found diff --git a/test/files/neg/t5091.scala b/test/files/neg/t5091.scala new file mode 100644 index 0000000000..217e83f66d --- /dev/null +++ b/test/files/neg/t5091.scala @@ -0,0 +1,11 @@ +object RecursiveValueNeedsType { + + def foo(param: String) = 42 + def bar(n: Int) = 42 + + { + val xxx = foo(param = null) + val param = bar(xxx) + } + +} diff --git a/test/pending/pos/t5091.scala b/test/pending/pos/t5091.scala deleted file mode 100644 index 217e83f66d..0000000000 --- a/test/pending/pos/t5091.scala +++ /dev/null @@ -1,11 +0,0 @@ -object RecursiveValueNeedsType { - - def foo(param: String) = 42 - def bar(n: Int) = 42 - - { - val xxx = foo(param = null) - val param = bar(xxx) - } - -} -- cgit v1.2.3 From 654912f5020d4f42dff8e2f5a17bdfa5d37befa5 Mon Sep 17 00:00:00 2001 From: Gerard Basler Date: Sun, 26 Oct 2014 23:41:28 +0100 Subject: Avoid the `CNF budget exceeded` exception via smarter translation into CNF. The exhaustivity checks in the pattern matcher build a propositional formula that must be converted into conjunctive normal form (CNF) in order to be amenable to the following DPLL decision procedure. However, the simple conversion into CNF via negation normal form and Shannon expansion that was used has exponential worst-case complexity and thus even simple problems could become untractable. A better approach is to use a transformation into an _equisatisfiable_ CNF-formula (by generating auxiliary variables) that runs with linear complexity. The commonly known Tseitin transformation uses bi- implication. I have choosen for an enhancement: the Plaisted transformation which uses implication only, thus the resulting CNF formula has (on average) only half of the clauses of a Tseitin transformation. The Plaisted transformation uses the polarities of sub-expressions to figure out which part of the bi-implication can be omitted. However, if all sub-expressions have positive polarity (e.g., after transformation into negation normal form) then the conversion is rather simple and the pseudo-normalization via NNF increases chances only one side of the bi-implication is needed. I implemented only optimizations that had a substantial effect on formula size: - formula simplification, extraction of multi argument operands - if a formula is already in CNF then the Tseitin/Plaisted transformation is omitted - Plaisted via NNF - omitted: (sharing of sub-formulas is also not implemented) - omitted: (clause subsumption) --- .../scala/tools/nsc/transform/patmat/Logic.scala | 190 +++++--- .../tools/nsc/transform/patmat/MatchAnalysis.scala | 101 ++--- .../scala/tools/nsc/transform/patmat/Solving.scala | 476 ++++++++++++++------- .../test/files/neg/virtpatmat_exhaust_big.check | 7 + .../test/files/neg/virtpatmat_exhaust_big.flags | 1 + .../test/files/neg/virtpatmat_exhaust_big.scala | 32 ++ .../test/files/pos/virtpatmat_exhaust_big.scala | 34 ++ test/files/neg/t6582_exhaust_big.check | 7 + test/files/neg/t6582_exhaust_big.flags | 1 + test/files/neg/t6582_exhaust_big.scala | 32 ++ test/files/pos/t6582_exhaust_big.scala | 33 ++ 11 files changed, 646 insertions(+), 268 deletions(-) create mode 100644 src/intellij/test/files/neg/virtpatmat_exhaust_big.check create mode 100644 src/intellij/test/files/neg/virtpatmat_exhaust_big.flags create mode 100644 src/intellij/test/files/neg/virtpatmat_exhaust_big.scala create mode 100644 src/intellij/test/files/pos/virtpatmat_exhaust_big.scala create mode 100644 test/files/neg/t6582_exhaust_big.check create mode 100644 test/files/neg/t6582_exhaust_big.flags create mode 100644 test/files/neg/t6582_exhaust_big.scala create mode 100644 test/files/pos/t6582_exhaust_big.scala (limited to 'test/files/neg') diff --git a/src/compiler/scala/tools/nsc/transform/patmat/Logic.scala b/src/compiler/scala/tools/nsc/transform/patmat/Logic.scala index d10b8eb84f..75d2cfe0f2 100644 --- a/src/compiler/scala/tools/nsc/transform/patmat/Logic.scala +++ b/src/compiler/scala/tools/nsc/transform/patmat/Logic.scala @@ -11,7 +11,6 @@ import scala.language.postfixOps import scala.collection.mutable import scala.reflect.internal.util.Statistics import scala.reflect.internal.util.HashSet -import scala.annotation.tailrec trait Logic extends Debugging { import PatternMatchingStats._ @@ -104,21 +103,16 @@ trait Logic extends Debugging { def implications: List[(Sym, List[Sym], List[Sym])] } - private def propToLinkedHashSet(ops: Seq[Prop]) = { - val set = new mutable.LinkedHashSet[Prop]() - ops.foreach(set += _) - set - } - // would be nice to statically check whether a prop is equational or pure, // but that requires typing relations like And(x: Tx, y: Ty) : (if(Tx == PureProp && Ty == PureProp) PureProp else Prop) - final case class And(ops: mutable.LinkedHashSet[Prop]) extends Prop + final case class And(ops: Set[Prop]) extends Prop object And { - def apply(ops: Prop*) = new And(propToLinkedHashSet(ops)) + def apply(ops: Prop*) = new And(ops.toSet) } - final case class Or(ops: mutable.LinkedHashSet[Prop]) extends Prop + + final case class Or(ops: Set[Prop]) extends Prop object Or { - def apply(ops: Prop*) = new Or(propToLinkedHashSet(ops)) + def apply(ops: Prop*) = new Or(ops.toSet) } final case class Not(a: Prop) extends Prop @@ -146,6 +140,98 @@ trait Logic extends Debugging { def /\(props: Iterable[Prop]) = if (props.isEmpty) True else And(props.toSeq: _*) def \/(props: Iterable[Prop]) = if (props.isEmpty) False else Or(props.toSeq: _*) + /** + * Simplifies propositional formula according to the following rules: + * - eliminate double negation (avoids unnecessary Tseitin variables) + * - flatten trees of same connectives (avoids unnecessary Tseitin variables) + * - removes constants and connectives that are in fact constant because of their operands + * - eliminates duplicate operands + * - convert formula into NNF: all sub-expressions have a positive polarity + * which makes them amenable for the subsequent Plaisted transformation + * and increases chances to figure out that the formula is already in CNF + * + * Complexity: DFS over formula tree + * + * See http://www.decision-procedures.org/slides/propositional_logic-2x3.pdf + */ + def simplify(f: Prop): Prop = { + + // limit size to avoid blow up + def hasImpureAtom(ops: Seq[Prop]): Boolean = ops.size < 10 && + ops.combinations(2).exists { + case Seq(a, Not(b)) if a == b => true + case Seq(Not(a), b) if a == b => true + case _ => false + } + + // push negation inside formula + def negationNormalFormNot(p: Prop): Prop = p match { + case And(ops) => Or(ops.map(negationNormalFormNot)) // De'Morgan + case Or(ops) => And(ops.map(negationNormalFormNot)) // De'Morgan + case Not(p) => negationNormalForm(p) + case True => False + case False => True + case s: Sym => Not(s) + } + + def negationNormalForm(p: Prop): Prop = p match { + case And(ops) => And(ops.map(negationNormalForm)) + case Or(ops) => Or(ops.map(negationNormalForm)) + case Not(negated) => negationNormalFormNot(negated) + case True + | False + | (_: Sym) => p + } + + def simplifyProp(p: Prop): Prop = p match { + case And(fv) => + // recurse for nested And (pulls all Ands up) + val ops = fv.map(simplifyProp) - True // ignore `True` + + // build up Set in order to remove duplicates + val opsFlattened = ops.flatMap { + case And(fv) => fv + case f => Set(f) + }.toSeq + + if (hasImpureAtom(opsFlattened) || opsFlattened.contains(False)) { + False + } else { + opsFlattened match { + case Seq() => True + case Seq(f) => f + case ops => And(ops: _*) + } + } + case Or(fv) => + // recurse for nested Or (pulls all Ors up) + val ops = fv.map(simplifyProp) - False // ignore `False` + + val opsFlattened = ops.flatMap { + case Or(fv) => fv + case f => Set(f) + }.toSeq + + if (hasImpureAtom(opsFlattened) || opsFlattened.contains(True)) { + True + } else { + opsFlattened match { + case Seq() => False + case Seq(f) => f + case ops => Or(ops: _*) + } + } + case Not(Not(a)) => + simplify(a) + case Not(p) => + Not(simplify(p)) + case p => + p + } + + val nnf = negationNormalForm(f) + simplifyProp(nnf) + } trait PropTraverser { def apply(x: Prop): Unit = x match { @@ -153,10 +239,12 @@ trait Logic extends Debugging { case Or(ops) => ops foreach apply case Not(a) => apply(a) case Eq(a, b) => applyVar(a); applyConst(b) + case s: Sym => applySymbol(s) case _ => } def applyVar(x: Var): Unit = {} def applyConst(x: Const): Unit = {} + def applySymbol(x: Sym): Unit = {} } def gatherVariables(p: Prop): Set[Var] = { @@ -167,6 +255,14 @@ trait Logic extends Debugging { vars.toSet } + def gatherSymbols(p: Prop): Set[Sym] = { + val syms = new mutable.HashSet[Sym]() + (new PropTraverser { + override def applySymbol(s: Sym) = syms += s + })(p) + syms.toSet + } + trait PropMap { def apply(x: Prop): Prop = x match { // TODO: mapConserve case And(ops) => And(ops map apply) @@ -176,27 +272,10 @@ trait Logic extends Debugging { } } - // to govern how much time we spend analyzing matches for unreachability/exhaustivity - object AnalysisBudget { - private val budgetProp = scala.sys.Prop[String]("scalac.patmat.analysisBudget") - private val budgetOff = "off" - val max: Int = { - val DefaultBudget = 256 - budgetProp.option match { - case Some(`budgetOff`) => - Integer.MAX_VALUE - case Some(x) => - x.toInt - case None => - DefaultBudget - } - } - - abstract class Exception(val advice: String) extends RuntimeException("CNF budget exceeded") - - object exceeded extends Exception( - s"(The analysis required more space than allowed. Please try with scalac -D${budgetProp.key}=${AnalysisBudget.max*2} or -D${budgetProp.key}=${budgetOff}.)") - + // TODO: remove since deprecated + val budgetProp = scala.sys.Prop[String]("scalac.patmat.analysisBudget") + if (budgetProp.isSet) { + reportWarning(s"Please remove -D${budgetProp.key}, it is ignored.") } // convert finite domain propositional logic with subtyping to pure boolean propositional logic @@ -217,7 +296,7 @@ trait Logic extends Debugging { // TODO: for V1 representing x1 and V2 standing for x1.head, encode that // V1 = Nil implies -(V2 = Ci) for all Ci in V2's domain (i.e., it is unassignable) // may throw an AnalysisBudget.Exception - def removeVarEq(props: List[Prop], modelNull: Boolean = false): (Formula, List[Formula]) = { + def removeVarEq(props: List[Prop], modelNull: Boolean = false): (Prop, List[Prop]) = { val start = if (Statistics.canEnable) Statistics.startTimer(patmatAnaVarEq) else null val vars = new mutable.HashSet[Var] @@ -241,10 +320,10 @@ trait Logic extends Debugging { props foreach gatherEqualities.apply if (modelNull) vars foreach (_.registerNull()) - val pure = props map (p => eqFreePropToSolvable(rewriteEqualsToProp(p))) + val pure = props map (p => rewriteEqualsToProp(p)) - val eqAxioms = formulaBuilder - @inline def addAxiom(p: Prop) = addFormula(eqAxioms, eqFreePropToSolvable(p)) + val eqAxioms = mutable.ArrayBuffer[Prop]() + @inline def addAxiom(p: Prop) = eqAxioms += p debug.patmat("removeVarEq vars: "+ vars) vars.foreach { v => @@ -270,49 +349,30 @@ trait Logic extends Debugging { } } - debug.patmat(s"eqAxioms:\n$eqAxioms") + debug.patmat(s"eqAxioms:\n${eqAxioms.mkString("\n")}") debug.patmat(s"pure:${pure.mkString("\n")}") if (Statistics.canEnable) Statistics.stopTimer(patmatAnaVarEq, start) - (toFormula(eqAxioms), pure) + (And(eqAxioms: _*), pure) } + type Solvable - // an interface that should be suitable for feeding a SAT solver when the time comes - type Formula - type FormulaBuilder - - // creates an empty formula builder to which more formulae can be added - def formulaBuilder: FormulaBuilder - - // val f = formulaBuilder; addFormula(f, f1); ... addFormula(f, fN) - // toFormula(f) == andFormula(f1, andFormula(..., fN)) - def addFormula(buff: FormulaBuilder, f: Formula): Unit - def toFormula(buff: FormulaBuilder): Formula - - // the conjunction of formulae `a` and `b` - def andFormula(a: Formula, b: Formula): Formula - - // equivalent formula to `a`, but simplified in a lightweight way (drop duplicate clauses) - def simplifyFormula(a: Formula): Formula - - // may throw an AnalysisBudget.Exception - def propToSolvable(p: Prop): Formula = { - val (eqAxioms, pure :: Nil) = removeVarEq(List(p), modelNull = false) - andFormula(eqAxioms, pure) + def propToSolvable(p: Prop): Solvable = { + val (eqAxiom, pure :: Nil) = removeVarEq(List(p), modelNull = false) + eqFreePropToSolvable(And(eqAxiom, pure)) } - // may throw an AnalysisBudget.Exception - def eqFreePropToSolvable(p: Prop): Formula - def cnfString(f: Formula): String + def eqFreePropToSolvable(f: Prop): Solvable type Model = Map[Sym, Boolean] val EmptyModel: Model val NoModel: Model - def findModelFor(f: Formula): Model - def findAllModelsFor(f: Formula): List[Model] + def findModelFor(solvable: Solvable): Model + + def findAllModelsFor(solvable: Solvable): List[Model] } } diff --git a/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala b/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala index 21e90b1d78..8650f6ef90 100644 --- a/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala +++ b/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala @@ -397,7 +397,6 @@ trait MatchAnalysis extends MatchApproximation { trait MatchAnalyzer extends MatchApproximator { def uncheckedWarning(pos: Position, msg: String) = currentRun.reporting.uncheckedWarning(pos, msg) - def warn(pos: Position, ex: AnalysisBudget.Exception, kind: String) = uncheckedWarning(pos, s"Cannot check match for $kind.\n${ex.advice}") def reportWarning(message: String) = global.reporter.warning(typer.context.tree.pos, message) // TODO: model dependencies between variables: if V1 corresponds to (x: List[_]) and V2 is (x.hd), V2 cannot be assigned when V1 = null or V1 = Nil @@ -428,49 +427,44 @@ trait MatchAnalysis extends MatchApproximation { val propsCasesOk = approximate(True) map caseWithoutBodyToProp val propsCasesFail = approximate(False) map (t => Not(caseWithoutBodyToProp(t))) - try { - val (eqAxiomsFail, symbolicCasesFail) = removeVarEq(propsCasesFail, modelNull = true) - val (eqAxiomsOk, symbolicCasesOk) = removeVarEq(propsCasesOk, modelNull = true) - val eqAxioms = simplifyFormula(andFormula(eqAxiomsOk, eqAxiomsFail)) // I'm pretty sure eqAxiomsOk == eqAxiomsFail, but not 100% sure. - - val prefix = formulaBuilder - addFormula(prefix, eqAxioms) - - var prefixRest = symbolicCasesFail - var current = symbolicCasesOk - var reachable = true - var caseIndex = 0 - - debug.patmat("reachability, vars:\n"+ ((propsCasesFail flatMap gatherVariables).distinct map (_.describe) mkString ("\n"))) - debug.patmat("equality axioms:\n"+ cnfString(eqAxiomsOk)) - - // invariant (prefixRest.length == current.length) && (prefix.reverse ++ prefixRest == symbolicCasesFail) - // termination: prefixRest.length decreases by 1 - while (prefixRest.nonEmpty && reachable) { - val prefHead = prefixRest.head - caseIndex += 1 - prefixRest = prefixRest.tail - if (prefixRest.isEmpty) reachable = true - else { - addFormula(prefix, prefHead) - current = current.tail - val model = findModelFor(andFormula(current.head, toFormula(prefix))) + val (eqAxiomsFail, symbolicCasesFail) = removeVarEq(propsCasesFail, modelNull = true) + val (eqAxiomsOk, symbolicCasesOk) = removeVarEq(propsCasesOk, modelNull = true) + val eqAxioms = simplify(And(eqAxiomsOk, eqAxiomsFail)) // I'm pretty sure eqAxiomsOk == eqAxiomsFail, but not 100% sure. - // debug.patmat("trying to reach:\n"+ cnfString(current.head) +"\nunder prefix:\n"+ cnfString(prefix)) - // if (NoModel ne model) debug.patmat("reached: "+ modelString(model)) + val prefix = mutable.ArrayBuffer[Prop]() + prefix += eqAxioms - reachable = NoModel ne model - } - } + var prefixRest = symbolicCasesFail + var current = symbolicCasesOk + var reachable = true + var caseIndex = 0 + + debug.patmat("reachability, vars:\n" + ((propsCasesFail flatMap gatherVariables).distinct map (_.describe) mkString ("\n"))) + debug.patmat(s"equality axioms:\n$eqAxiomsOk") + + // invariant (prefixRest.length == current.length) && (prefix.reverse ++ prefixRest == symbolicCasesFail) + // termination: prefixRest.length decreases by 1 + while (prefixRest.nonEmpty && reachable) { + val prefHead = prefixRest.head + caseIndex += 1 + prefixRest = prefixRest.tail + if (prefixRest.isEmpty) reachable = true + else { + prefix += prefHead + current = current.tail + val and = And((current.head +: prefix): _*) + val model = findModelFor(eqFreePropToSolvable(and)) - if (Statistics.canEnable) Statistics.stopTimer(patmatAnaReach, start) + // debug.patmat("trying to reach:\n"+ cnfString(current.head) +"\nunder prefix:\n"+ cnfString(prefix)) + // if (NoModel ne model) debug.patmat("reached: "+ modelString(model)) - if (reachable) None else Some(caseIndex) - } catch { - case ex: AnalysisBudget.Exception => - warn(prevBinder.pos, ex, "unreachability") - None // CNF budget exceeded + reachable = NoModel ne model + } } + + if (Statistics.canEnable) Statistics.stopTimer(patmatAnaReach, start) + + if (reachable) None else Some(caseIndex) } // exhaustivity @@ -518,24 +512,17 @@ trait MatchAnalysis extends MatchApproximation { // debug.patmat("\nvars:\n"+ (vars map (_.describe) mkString ("\n"))) // debug.patmat("\nmatchFails as CNF:\n"+ cnfString(propToSolvable(matchFails))) - try { - // find the models (under which the match fails) - val matchFailModels = findAllModelsFor(propToSolvable(matchFails)) - - val scrutVar = Var(prevBinderTree) - val counterExamples = matchFailModels.flatMap(modelToCounterExample(scrutVar)) - // sorting before pruning is important here in order to - // keep neg/t7020.scala stable - // since e.g. List(_, _) would cover List(1, _) - val pruned = CounterExample.prune(counterExamples.sortBy(_.toString)).map(_.toString) - - if (Statistics.canEnable) Statistics.stopTimer(patmatAnaExhaust, start) - pruned - } catch { - case ex : AnalysisBudget.Exception => - warn(prevBinder.pos, ex, "exhaustivity") - Nil // CNF budget exceeded - } + // find the models (under which the match fails) + val matchFailModels = findAllModelsFor(propToSolvable(matchFails)) + val scrutVar = Var(prevBinderTree) + val counterExamples = matchFailModels.flatMap(modelToCounterExample(scrutVar)) + // sorting before pruning is important here in order to + // keep neg/t7020.scala stable + // since e.g. List(_, _) would cover List(1, _) + val pruned = CounterExample.prune(counterExamples.sortBy(_.toString)).map(_.toString) + + if (Statistics.canEnable) Statistics.stopTimer(patmatAnaExhaust, start) + pruned } } diff --git a/src/compiler/scala/tools/nsc/transform/patmat/Solving.scala b/src/compiler/scala/tools/nsc/transform/patmat/Solving.scala index c5475b50c0..1ba13c0617 100644 --- a/src/compiler/scala/tools/nsc/transform/patmat/Solving.scala +++ b/src/compiler/scala/tools/nsc/transform/patmat/Solving.scala @@ -6,148 +6,304 @@ package scala.tools.nsc.transform.patmat -import scala.collection.mutable +import scala.collection.mutable.ArrayBuffer import scala.reflect.internal.util.Statistics import scala.language.postfixOps +import scala.collection.mutable import scala.reflect.internal.util.Collections._ -// naive CNF translation and simple DPLL solver +// a literal is a (possibly negated) variable +class Lit(val v: Int) extends AnyVal { + def unary_- : Lit = Lit(-v) + + def variable: Int = Math.abs(v) + + def positive = v >= 0 + + override def toString(): String = s"Lit#$v" +} + +object Lit { + def apply(v: Int): Lit = new Lit(v) + + implicit val LitOrdering: Ordering[Lit] = Ordering.by(_.v) +} + +/** Solve pattern matcher exhaustivity problem via DPLL. + */ trait Solving extends Logic { + import PatternMatchingStats._ - trait CNF extends PropositionalLogic { - import scala.collection.mutable.ArrayBuffer - type FormulaBuilder = ArrayBuffer[Clause] - def formulaBuilder = ArrayBuffer[Clause]() - def formulaBuilderSized(init: Int) = new ArrayBuffer[Clause](init) - def addFormula(buff: FormulaBuilder, f: Formula): Unit = buff ++= f - def toFormula(buff: FormulaBuilder): Formula = buff - // CNF: a formula is a conjunction of clauses - type Formula = FormulaBuilder - def formula(c: Clause*): Formula = ArrayBuffer(c: _*) + trait CNF extends PropositionalLogic { type Clause = Set[Lit] + // a clause is a disjunction of distinct literals def clause(l: Lit*): Clause = l.toSet - type Lit - def Lit(sym: Sym, pos: Boolean = true): Lit - - def andFormula(a: Formula, b: Formula): Formula = a ++ b - def simplifyFormula(a: Formula): Formula = a.distinct - - private def merge(a: Clause, b: Clause) = a ++ b - - // throws an AnalysisBudget.Exception when the prop results in a CNF that's too big - // TODO: be smarter/more efficient about this (http://lara.epfl.ch/w/sav09:tseitin_s_encoding) - def eqFreePropToSolvable(p: Prop): Formula = { - def negationNormalFormNot(p: Prop, budget: Int): Prop = - if (budget <= 0) throw AnalysisBudget.exceeded - else p match { - case And(ps) => Or (ps map (negationNormalFormNot(_, budget - 1))) - case Or(ps) => And(ps map (negationNormalFormNot(_, budget - 1))) - case Not(p) => negationNormalForm(p, budget - 1) - case True => False - case False => True - case s: Sym => Not(s) + /** Conjunctive normal form (of a Boolean formula). + * A formula in this form is amenable to a SAT solver + * (i.e., solver that decides satisfiability of a formula). + */ + type Cnf = Array[Clause] + + class SymbolMapping(symbols: Set[Sym]) { + val variableForSymbol: Map[Sym, Int] = { + symbols.zipWithIndex.map { + case (sym, i) => sym -> (i + 1) + }.toMap + } + + val symForVar: Map[Int, Sym] = variableForSymbol.map(_.swap) + + val relevantVars: Set[Int] = symForVar.keySet.map(math.abs) + + def lit(sym: Sym): Lit = Lit(variableForSymbol(sym)) + + def size = symbols.size + } + + case class Solvable(cnf: Cnf, symbolMapping: SymbolMapping) + + trait CnfBuilder { + private[this] val buff = ArrayBuffer[Clause]() + + var literalCount: Int + + /** + * @return new Tseitin variable + */ + def newLiteral(): Lit = { + literalCount += 1 + Lit(literalCount) + } + + lazy val constTrue: Lit = { + val constTrue = newLiteral() + addClauseProcessed(clause(constTrue)) + constTrue + } + + def constFalse: Lit = -constTrue + + def isConst(l: Lit): Boolean = l == constTrue || l == constFalse + + def addClauseProcessed(clause: Clause) { + if (clause.nonEmpty) { + buff += clause } + } - def negationNormalForm(p: Prop, budget: Int = AnalysisBudget.max): Prop = - if (budget <= 0) throw AnalysisBudget.exceeded - else p match { - case Or(ps) => Or (ps map (negationNormalForm(_, budget - 1))) - case And(ps) => And(ps map (negationNormalForm(_, budget - 1))) - case Not(negated) => negationNormalFormNot(negated, budget - 1) - case True - | False - | (_ : Sym) => p + def buildCnf: Array[Clause] = buff.toArray + + } + + /** Plaisted transformation: used for conversion of a + * propositional formula into conjunctive normal form (CNF) + * (input format for SAT solver). + * A simple conversion into CNF via Shannon expansion would + * also be possible but it's worst-case complexity is exponential + * (in the number of variables) and thus even simple problems + * could become untractable. + * The Plaisted transformation results in an _equisatisfiable_ + * CNF-formula (it generates auxiliary variables) + * but runs with linear complexity. + * The common known Tseitin transformation uses bi-implication, + * whereas the Plaisted transformation uses implication only, thus + * the resulting CNF formula has (on average) only half of the clauses + * of a Tseitin transformation. + * The Plaisted transformation uses the polarities of sub-expressions + * to figure out which part of the bi-implication can be omitted. + * However, if all sub-expressions have positive polarity + * (e.g., after transformation into negation normal form) + * then the conversion is rather simple and the pseudo-normalization + * via NNF increases chances only one side of the bi-implication + * is needed. + */ + class TransformToCnf(symbolMapping: SymbolMapping) extends CnfBuilder { + + // new literals start after formula symbols + var literalCount: Int = symbolMapping.size + + def convertSym(sym: Sym): Lit = symbolMapping.lit(sym) + + def apply(p: Prop): Solvable = { + + def convert(p: Prop): Lit = { + p match { + case And(fv) => + and(fv.map(convert)) + case Or(fv) => + or(fv.map(convert)) + case Not(a) => + not(convert(a)) + case sym: Sym => + convertSym(sym) + case True => + constTrue + case False => + constFalse + case _: Eq => + throw new MatchError(p) + } } - val TrueF = formula() - val FalseF = formula(clause()) - def lit(s: Sym) = formula(clause(Lit(s))) - def negLit(s: Sym) = formula(clause(Lit(s, pos = false))) - - def conjunctiveNormalForm(p: Prop, budget: Int = AnalysisBudget.max): Formula = { - def distribute(a: Formula, b: Formula, budget: Int): Formula = - if (budget <= 0) throw AnalysisBudget.exceeded - else - (a, b) match { - // true \/ _ = true - // _ \/ true = true - case (trueA, trueB) if trueA.size == 0 || trueB.size == 0 => TrueF - // lit \/ lit - case (a, b) if a.size == 1 && b.size == 1 => formula(merge(a(0), b(0))) - // (c1 /\ ... /\ cn) \/ d = ((c1 \/ d) /\ ... /\ (cn \/ d)) - // d \/ (c1 /\ ... /\ cn) = ((d \/ c1) /\ ... /\ (d \/ cn)) - case (cs, ds) => - val (big, small) = if (cs.size > ds.size) (cs, ds) else (ds, cs) - big flatMap (c => distribute(formula(c), small, budget - (big.size*small.size))) - } + def and(bv: Set[Lit]): Lit = { + if (bv.isEmpty) { + // this case can actually happen because `removeVarEq` could add no constraints + constTrue + } else if (bv.size == 1) { + bv.head + } else if (bv.contains(constFalse)) { + constFalse + } else { + // op1 /\ op2 /\ ... /\ opx <==> + // (o -> op1) /\ (o -> op2) ... (o -> opx) /\ (!op1 \/ !op2 \/... \/ !opx \/ o) + // (!o \/ op1) /\ (!o \/ op2) ... (!o \/ opx) /\ (!op1 \/ !op2 \/... \/ !opx \/ o) + val new_bv = bv - constTrue // ignore `True` + val o = newLiteral() // auxiliary Tseitin variable + new_bv.map(op => addClauseProcessed(clause(op, -o))) + o + } + } - if (budget <= 0) throw AnalysisBudget.exceeded - - p match { - case True => TrueF - case False => FalseF - case s: Sym => lit(s) - case Not(s: Sym) => negLit(s) - case And(ps) => - val formula = formulaBuilder - ps foreach { p => - val cnf = conjunctiveNormalForm(p, budget - 1) - addFormula(formula, cnf) - } - toFormula(formula) - case Or(ps) => - ps map (conjunctiveNormalForm(_)) reduceLeft { (a, b) => - distribute(a, b, budget - (a.size + b.size)) - } + def or(bv: Set[Lit]): Lit = { + if (bv.isEmpty) { + constFalse + } else if (bv.size == 1) { + bv.head + } else if (bv.contains(constTrue)) { + constTrue + } else { + // op1 \/ op2 \/ ... \/ opx <==> + // (op1 -> o) /\ (op2 -> o) ... (opx -> o) /\ (op1 \/ op2 \/... \/ opx \/ !o) + // (!op1 \/ o) /\ (!op2 \/ o) ... (!opx \/ o) /\ (op1 \/ op2 \/... \/ opx \/ !o) + val new_bv = bv - constFalse // ignore `False` + val o = newLiteral() // auxiliary Tseitin variable + addClauseProcessed(new_bv + (-o)) + o + } } + + // no need for auxiliary variable + def not(a: Lit): Lit = -a + + // add intermediate variable since we want the formula to be SAT! + addClauseProcessed(clause(convert(p))) + + Solvable(buildCnf, symbolMapping) } + } - val start = if (Statistics.canEnable) Statistics.startTimer(patmatCNF) else null - val res = conjunctiveNormalForm(negationNormalForm(p)) + class AlreadyInCNF(symbolMapping: SymbolMapping) { - if (Statistics.canEnable) Statistics.stopTimer(patmatCNF, start) + object ToLiteral { + def unapply(f: Prop): Option[Lit] = f match { + case Not(ToLiteral(lit)) => Some(-lit) + case sym: Sym => Some(symbolMapping.lit(sym)) + case _ => None + } + } - if (Statistics.canEnable) patmatCNFSizes(res.size).value += 1 + object ToDisjunction { + def unapply(f: Prop): Option[Array[Clause]] = f match { + case Or(fv) => + val cl = fv.foldLeft(Option(clause())) { + case (Some(clause), ToLiteral(lit)) => + Some(clause + lit) + case (_, _) => + None + } + cl.map(Array(_)) + case True => Some(Array()) // empty, no clauses needed + case False => Some(Array(clause())) // empty clause can't be satisfied + case ToLiteral(lit) => Some(Array(clause(lit))) + case _ => None + } + } -// debug.patmat("cnf for\n"+ p +"\nis:\n"+cnfString(res)) - res + /** + * Checks if propositional formula is already in CNF + */ + object ToCnf { + def unapply(f: Prop): Option[Solvable] = f match { + case ToDisjunction(clauses) => Some(Solvable(clauses, symbolMapping) ) + case And(fv) => + val clauses = fv.foldLeft(Option(mutable.ArrayBuffer[Clause]())) { + case (Some(cnf), ToDisjunction(clauses)) => + Some(cnf ++= clauses) + case (_, _) => + None + } + clauses.map(c => Solvable(c.toArray, symbolMapping)) + case _ => None + } + } + } + + def eqFreePropToSolvable(p: Prop): Solvable = { + + // collect all variables since after simplification / CNF conversion + // they could have been removed from the formula + val symbolMapping = new SymbolMapping(gatherSymbols(p)) + + val simplified = simplify(p) + val cnfExtractor = new AlreadyInCNF(symbolMapping) + simplified match { + case cnfExtractor.ToCnf(solvable) => + // this is needed because t6942 would generate too many clauses with Tseitin + // already in CNF, just add clauses + solvable + case p => + new TransformToCnf(symbolMapping).apply(p) + } } } // simple solver using DPLL trait Solver extends CNF { - // a literal is a (possibly negated) variable - def Lit(sym: Sym, pos: Boolean = true) = new Lit(sym, pos) - class Lit(val sym: Sym, val pos: Boolean) { - override def toString = if (!pos) "-"+ sym.toString else sym.toString - override def equals(o: Any) = o match { - case o: Lit => (o.sym eq sym) && (o.pos == pos) - case _ => false - } - override def hashCode = sym.hashCode + pos.hashCode + import scala.collection.mutable.ArrayBuffer - def unary_- = Lit(sym, !pos) + def cnfString(f: Array[Clause]): String = { + val lits: Array[List[String]] = f map (_.map(_.toString).toList) + val xss: List[List[String]] = lits toList + val aligned: String = alignAcrossRows(xss, "\\/", " /\\\n") + aligned } - def cnfString(f: Formula) = alignAcrossRows(f map (_.toList) toList, "\\/", " /\\\n") - // adapted from http://lara.epfl.ch/w/sav10:simple_sat_solver (original by Hossein Hojjat) + + // empty set of clauses is trivially satisfied val EmptyModel = Map.empty[Sym, Boolean] + + // no model: originates from the encounter of an empty clause, i.e., + // happens if all variables have been assigned in a way that makes the corresponding literals false + // thus there is no possibility to satisfy that clause, so the whole formula is UNSAT val NoModel: Model = null + // this model contains the auxiliary variables as well + type TseitinModel = Set[Lit] + val EmptyTseitinModel = Set.empty[Lit] + val NoTseitinModel: TseitinModel = null + // returns all solutions, if any (TODO: better infinite recursion backstop -- detect fixpoint??) - def findAllModelsFor(f: Formula): List[Model] = { + def findAllModelsFor(solvable: Solvable): List[Model] = { + debug.patmat("find all models for\n"+ cnfString(solvable.cnf)) - debug.patmat("find all models for\n"+ cnfString(f)) + // we must take all vars from non simplified formula + // otherwise if we get `T` as formula, we don't expand the variables + // that are not in the formula... + val relevantVars: Set[Int] = solvable.symbolMapping.relevantVars - val vars: Set[Sym] = f.flatMap(_ collect {case l: Lit => l.sym}).toSet // debug.patmat("vars "+ vars) // the negation of a model -(S1=True/False /\ ... /\ SN=True/False) = clause(S1=False/True, ...., SN=False/True) - def negateModel(m: Model) = clause(m.toSeq.map{ case (sym, pos) => Lit(sym, !pos) } : _*) + // (i.e. the blocking clause - used for ALL-SAT) + def negateModel(m: TseitinModel) = { + // filter out auxiliary Tseitin variables + val relevantLits = m.filter(l => relevantVars.contains(l.variable)) + relevantLits.map(lit => -lit) + } /** * The DPLL procedure only returns a minimal mapping from literal to value @@ -160,11 +316,11 @@ trait Solving extends Logic { * The expansion step will amend both solutions with the unassigned variable * i.e., {a = true} will be expanded to {a = true, b = true} and {a = true, b = false}. */ - def expandUnassigned(unassigned: List[Sym], model: Model): List[Model] = { + def expandUnassigned(unassigned: List[Int], model: TseitinModel): List[TseitinModel] = { // the number of solutions is doubled for every unassigned variable val expandedModels = 1 << unassigned.size - var current = mutable.ArrayBuffer[Model]() - var next = mutable.ArrayBuffer[Model]() + var current = mutable.ArrayBuffer[TseitinModel]() + var next = mutable.ArrayBuffer[TseitinModel]() current.sizeHint(expandedModels) next.sizeHint(expandedModels) @@ -178,10 +334,10 @@ trait Solving extends Logic { for { model <- current } { - def force(l: Lit) = model + (l.sym -> l.pos) + def force(l: Lit) = model + l - next += force(Lit(s, pos = true)) - next += force(Lit(s, pos = false)) + next += force(Lit(s)) + next += force(Lit(-s)) } val tmp = current @@ -194,67 +350,80 @@ trait Solving extends Logic { current.toList } - def findAllModels(f: Formula, - models: List[Model], - recursionDepthAllowed: Int = global.settings.YpatmatExhaustdepth.value): List[Model]= + def findAllModels(clauses: Array[Clause], + models: List[TseitinModel], + recursionDepthAllowed: Int = global.settings.YpatmatExhaustdepth.value): List[TseitinModel]= if (recursionDepthAllowed == 0) { val maxDPLLdepth = global.settings.YpatmatExhaustdepth.value reportWarning("(Exhaustivity analysis reached max recursion depth, not all missing cases are reported. " + s"Please try with scalac -Ypatmat-exhaust-depth ${maxDPLLdepth * 2} or -Ypatmat-exhaust-depth off.)") models } else { - val model = findModelFor(f) + debug.patmat("find all models for\n" + cnfString(clauses)) + val model = findTseitinModelFor(clauses) // if we found a solution, conjunct the formula with the model's negation and recurse - if (model ne NoModel) { - val unassigned = (vars -- model.keySet).toList + if (model ne NoTseitinModel) { + // note that we should not expand the auxiliary variables (from Tseitin transformation) + // since they are existentially quantified in the final solution + val unassigned: List[Int] = (relevantVars -- model.map(lit => lit.variable)).toList debug.patmat("unassigned "+ unassigned +" in "+ model) val forced = expandUnassigned(unassigned, model) debug.patmat("forced "+ forced) val negated = negateModel(model) - findAllModels(f :+ negated, forced ++ models, recursionDepthAllowed - 1) + findAllModels(clauses :+ negated, forced ++ models, recursionDepthAllowed - 1) } else models } - findAllModels(f, Nil) + val tseitinModels: List[TseitinModel] = findAllModels(solvable.cnf, Nil) + val models: List[Model] = tseitinModels.map(projectToModel(_, solvable.symbolMapping.symForVar)) + models } - private def withLit(res: Model, l: Lit): Model = if (res eq NoModel) NoModel else res + (l.sym -> l.pos) - private def dropUnit(f: Formula, unitLit: Lit): Formula = { + private def withLit(res: TseitinModel, l: Lit): TseitinModel = { + if (res eq NoTseitinModel) NoTseitinModel else res + l + } + + /** Drop trivially true clauses, simplify others by dropping negation of `unitLit`. + * + * Disjunctions that contain the literal we're making true in the returned model are trivially true. + * Clauses can be simplified by dropping the negation of the literal we're making true + * (since False \/ X == X) + */ + private def dropUnit(clauses: Array[Clause], unitLit: Lit): Array[Clause] = { val negated = -unitLit - // drop entire clauses that are trivially true - // (i.e., disjunctions that contain the literal we're making true in the returned model), - // and simplify clauses by dropping the negation of the literal we're making true - // (since False \/ X == X) - val dropped = formulaBuilderSized(f.size) - for { - clause <- f - if !(clause contains unitLit) - } dropped += (clause - negated) - dropped + val simplified = new ArrayBuffer[Clause](clauses.size) + clauses foreach { + case trivial if trivial contains unitLit => // drop + case clause => simplified += clause - negated + } + simplified.toArray } - def findModelFor(f: Formula): Model = { - @inline def orElse(a: Model, b: => Model) = if (a ne NoModel) a else b + def findModelFor(solvable: Solvable): Model = { + projectToModel(findTseitinModelFor(solvable.cnf), solvable.symbolMapping.symForVar) + } - debug.patmat("DPLL\n"+ cnfString(f)) + def findTseitinModelFor(clauses: Array[Clause]): TseitinModel = { + @inline def orElse(a: TseitinModel, b: => TseitinModel) = if (a ne NoTseitinModel) a else b + + debug.patmat(s"DPLL\n${cnfString(clauses)}") val start = if (Statistics.canEnable) Statistics.startTimer(patmatAnaDPLL) else null - val satisfiableWithModel: Model = - if (f isEmpty) EmptyModel - else if(f exists (_.isEmpty)) NoModel - else f.find(_.size == 1) match { + val satisfiableWithModel: TseitinModel = + if (clauses isEmpty) EmptyTseitinModel + else if (clauses exists (_.isEmpty)) NoTseitinModel + else clauses.find(_.size == 1) match { case Some(unitClause) => val unitLit = unitClause.head - // debug.patmat("unit: "+ unitLit) - withLit(findModelFor(dropUnit(f, unitLit)), unitLit) + withLit(findTseitinModelFor(dropUnit(clauses, unitLit)), unitLit) case _ => // partition symbols according to whether they appear in positive and/or negative literals - val pos = new mutable.HashSet[Sym]() - val neg = new mutable.HashSet[Sym]() - mforeach(f)(lit => if (lit.pos) pos += lit.sym else neg += lit.sym) + val pos = new mutable.HashSet[Int]() + val neg = new mutable.HashSet[Int]() + mforeach(clauses)(lit => if (lit.positive) pos += lit.variable else neg += lit.variable) // appearing in both positive and negative val impures = pos intersect neg @@ -262,23 +431,38 @@ trait Solving extends Logic { val pures = (pos ++ neg) -- impures if (pures nonEmpty) { - val pureSym = pures.head + val pureVar = pures.head // turn it back into a literal // (since equality on literals is in terms of equality // of the underlying symbol and its positivity, simply construct a new Lit) - val pureLit = Lit(pureSym, pos(pureSym)) + val pureLit = Lit(if (neg(pureVar)) -pureVar else pureVar) // debug.patmat("pure: "+ pureLit +" pures: "+ pures +" impures: "+ impures) - val simplified = f.filterNot(_.contains(pureLit)) - withLit(findModelFor(simplified), pureLit) + val simplified = clauses.filterNot(_.contains(pureLit)) + withLit(findTseitinModelFor(simplified), pureLit) } else { - val split = f.head.head + val split = clauses.head.head // debug.patmat("split: "+ split) - orElse(findModelFor(f :+ clause(split)), findModelFor(f :+ clause(-split))) + orElse(findTseitinModelFor(clauses :+ clause(split)), findTseitinModelFor(clauses :+ clause(-split))) } } if (Statistics.canEnable) Statistics.stopTimer(patmatAnaDPLL, start) satisfiableWithModel } + + private def projectToModel(model: TseitinModel, symForVar: Map[Int, Sym]): Model = + if (model == NoTseitinModel) NoModel + else if (model == EmptyTseitinModel) EmptyModel + else { + val mappedModels = model.toList collect { + case lit if symForVar isDefinedAt lit.variable => (symForVar(lit.variable), lit.positive) + } + if (mappedModels.isEmpty) { + // could get an empty model if mappedModels is a constant like `True` + EmptyModel + } else { + mappedModels.toMap + } + } } } diff --git a/src/intellij/test/files/neg/virtpatmat_exhaust_big.check b/src/intellij/test/files/neg/virtpatmat_exhaust_big.check new file mode 100644 index 0000000000..fddc85a362 --- /dev/null +++ b/src/intellij/test/files/neg/virtpatmat_exhaust_big.check @@ -0,0 +1,7 @@ +virtpatmat_exhaust_big.scala:27: warning: match may not be exhaustive. +It would fail on the following input: Z11() + def foo(z: Z) = z match { + ^ +error: No warnings can be incurred under -Xfatal-warnings. +one warning found +one error found diff --git a/src/intellij/test/files/neg/virtpatmat_exhaust_big.flags b/src/intellij/test/files/neg/virtpatmat_exhaust_big.flags new file mode 100644 index 0000000000..b5a8748652 --- /dev/null +++ b/src/intellij/test/files/neg/virtpatmat_exhaust_big.flags @@ -0,0 +1 @@ +-Xfatal-warnings -unchecked diff --git a/src/intellij/test/files/neg/virtpatmat_exhaust_big.scala b/src/intellij/test/files/neg/virtpatmat_exhaust_big.scala new file mode 100644 index 0000000000..dd639eb56e --- /dev/null +++ b/src/intellij/test/files/neg/virtpatmat_exhaust_big.scala @@ -0,0 +1,32 @@ +sealed abstract class Z +object Z { + object Z0 extends Z + case class Z1() extends Z + object Z2 extends Z + case class Z3() extends Z + object Z4 extends Z + case class Z5() extends Z + object Z6 extends Z + case class Z7() extends Z + object Z8 extends Z + case class Z9() extends Z + object Z10 extends Z + case class Z11() extends Z + object Z12 extends Z + case class Z13() extends Z + object Z14 extends Z + case class Z15() extends Z + object Z16 extends Z + case class Z17() extends Z + object Z18 extends Z + case class Z19() extends Z +} + +object Test { + import Z._ + def foo(z: Z) = z match { + case Z0 | Z1() | Z2 | Z3() | Z4 | Z5() | Z6 | Z7() | Z8 | Z9() | + Z10 | Z12 | Z13() | Z14 | Z15() | Z16 | Z17() | Z18 | Z19() + => + } +} diff --git a/src/intellij/test/files/pos/virtpatmat_exhaust_big.scala b/src/intellij/test/files/pos/virtpatmat_exhaust_big.scala new file mode 100644 index 0000000000..41aef3226e --- /dev/null +++ b/src/intellij/test/files/pos/virtpatmat_exhaust_big.scala @@ -0,0 +1,34 @@ +sealed abstract class Z +object Z { + object Z0 extends Z + case class Z1() extends Z + object Z2 extends Z + case class Z3() extends Z + object Z4 extends Z + case class Z5() extends Z + object Z6 extends Z + case class Z7() extends Z + object Z8 extends Z + case class Z9() extends Z + object Z10 extends Z + case class Z11() extends Z + object Z12 extends Z + case class Z13() extends Z + object Z14 extends Z + case class Z15() extends Z + object Z16 extends Z + case class Z17() extends Z + object Z18 extends Z + case class Z19() extends Z +} + +// drop any case and it will report an error +object Test { + import Z._ + def foo(z: Z) = z match { + case Z0 | Z1() | Z2 | Z3() | Z4 | Z5() | Z6 | Z7() | Z8 | Z9() | + Z10 | Z11() | Z12 | Z13() | Z14 | Z15() | Z16 | Z17() | Z18 | Z19() + => + } +} +- diff --git a/test/files/neg/t6582_exhaust_big.check b/test/files/neg/t6582_exhaust_big.check new file mode 100644 index 0000000000..9e2be038b5 --- /dev/null +++ b/test/files/neg/t6582_exhaust_big.check @@ -0,0 +1,7 @@ +t6582_exhaust_big.scala:27: warning: match may not be exhaustive. +It would fail on the following input: Z11() + def foo(z: Z) = z match { + ^ +error: No warnings can be incurred under -Xfatal-warnings. +one warning found +one error found diff --git a/test/files/neg/t6582_exhaust_big.flags b/test/files/neg/t6582_exhaust_big.flags new file mode 100644 index 0000000000..b5a8748652 --- /dev/null +++ b/test/files/neg/t6582_exhaust_big.flags @@ -0,0 +1 @@ +-Xfatal-warnings -unchecked diff --git a/test/files/neg/t6582_exhaust_big.scala b/test/files/neg/t6582_exhaust_big.scala new file mode 100644 index 0000000000..dd639eb56e --- /dev/null +++ b/test/files/neg/t6582_exhaust_big.scala @@ -0,0 +1,32 @@ +sealed abstract class Z +object Z { + object Z0 extends Z + case class Z1() extends Z + object Z2 extends Z + case class Z3() extends Z + object Z4 extends Z + case class Z5() extends Z + object Z6 extends Z + case class Z7() extends Z + object Z8 extends Z + case class Z9() extends Z + object Z10 extends Z + case class Z11() extends Z + object Z12 extends Z + case class Z13() extends Z + object Z14 extends Z + case class Z15() extends Z + object Z16 extends Z + case class Z17() extends Z + object Z18 extends Z + case class Z19() extends Z +} + +object Test { + import Z._ + def foo(z: Z) = z match { + case Z0 | Z1() | Z2 | Z3() | Z4 | Z5() | Z6 | Z7() | Z8 | Z9() | + Z10 | Z12 | Z13() | Z14 | Z15() | Z16 | Z17() | Z18 | Z19() + => + } +} diff --git a/test/files/pos/t6582_exhaust_big.scala b/test/files/pos/t6582_exhaust_big.scala new file mode 100644 index 0000000000..7bb8879805 --- /dev/null +++ b/test/files/pos/t6582_exhaust_big.scala @@ -0,0 +1,33 @@ +sealed abstract class Z +object Z { + object Z0 extends Z + case class Z1() extends Z + object Z2 extends Z + case class Z3() extends Z + object Z4 extends Z + case class Z5() extends Z + object Z6 extends Z + case class Z7() extends Z + object Z8 extends Z + case class Z9() extends Z + object Z10 extends Z + case class Z11() extends Z + object Z12 extends Z + case class Z13() extends Z + object Z14 extends Z + case class Z15() extends Z + object Z16 extends Z + case class Z17() extends Z + object Z18 extends Z + case class Z19() extends Z +} + +// drop any case and it will report an error +object Test { + import Z._ + def foo(z: Z) = z match { + case Z0 | Z1() | Z2 | Z3() | Z4 | Z5() | Z6 | Z7() | Z8 | Z9() | + Z10 | Z11() | Z12 | Z13() | Z14 | Z15() | Z16 | Z17() | Z18 | Z19() + => + } +} -- cgit v1.2.3 From d76a2812ca56cd0d093fbbeb3ab60b35cd9c3180 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Thu, 6 Nov 2014 17:35:12 +1000 Subject: SI-7602 Avoid crash in LUBs with erroneous code If a class contains a double defintion of a method that overrides an interface method, LUBs could run into a spot where filtering overloaded alternatives to those that match the interface method fails to resolve to a single overload, which crashes the compiler. This commit uses `filter` rather than `suchThat` to avoid the crash. --- .../scala/reflect/internal/tpe/GlbLubs.scala | 4 +++- test/files/neg/t7602.check | 5 +++++ test/files/neg/t7602.scala | 26 ++++++++++++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 test/files/neg/t7602.check create mode 100644 test/files/neg/t7602.scala (limited to 'test/files/neg') diff --git a/src/reflect/scala/reflect/internal/tpe/GlbLubs.scala b/src/reflect/scala/reflect/internal/tpe/GlbLubs.scala index 876685e24a..123b44aa05 100644 --- a/src/reflect/scala/reflect/internal/tpe/GlbLubs.scala +++ b/src/reflect/scala/reflect/internal/tpe/GlbLubs.scala @@ -347,7 +347,9 @@ private[internal] trait GlbLubs { def lubsym(proto: Symbol): Symbol = { val prototp = lubThisType.memberInfo(proto) val syms = narrowts map (t => - t.nonPrivateMember(proto.name).suchThat(sym => + // SI-7602 With erroneous code, we could end up with overloaded symbols after filtering + // so `suchThat` unsuitable. + t.nonPrivateMember(proto.name).filter(sym => sym.tpe matches prototp.substThis(lubThisType.typeSymbol, t))) if (syms contains NoSymbol) NoSymbol diff --git a/test/files/neg/t7602.check b/test/files/neg/t7602.check new file mode 100644 index 0000000000..5bb1450d7d --- /dev/null +++ b/test/files/neg/t7602.check @@ -0,0 +1,5 @@ +t7602.scala:16: error: method foo is defined twice + conflicting symbols both originated in file 't7602.scala' + def foo : Device + ^ +one error found diff --git a/test/files/neg/t7602.scala b/test/files/neg/t7602.scala new file mode 100644 index 0000000000..5a9444a1ab --- /dev/null +++ b/test/files/neg/t7602.scala @@ -0,0 +1,26 @@ +trait Table[T]{ + def foo : T +} +trait Computer +trait Device + +object schema{ + def lub[T]( a:T, b:T ) = ??? + lub(null:Computers,null:Devices) +} +trait Computers extends Table[Computer]{ + def foo : Computer +} +trait Devices extends Table[Device]{ + def foo : Device + def foo : Device +} +/* Was: +Exception in thread "main" java.lang.AssertionError: assertion failed: List(method foo, method foo) + at scala.Predef$.assert(Predef.scala:165) + at scala.reflect.internal.Symbols$Symbol.suchThat(Symbols.scala:1916) + at scala.reflect.internal.tpe.GlbLubs$$anonfun$23.apply(GlbLubs.scala:350) + at scala.reflect.internal.tpe.GlbLubs$$anonfun$23.apply(GlbLubs.scala:349) + at scala.collection.immutable.List.map(List.scala:272) + at scala.reflect.internal.tpe.GlbLubs$class.lubsym$1(GlbLubs.scala:349) +*/ \ No newline at end of file -- cgit v1.2.3 From a34be74e8f2630f1862a34e538cbe1162d279e5e Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Thu, 6 Nov 2014 15:21:19 +0100 Subject: [sammy] use correct type for method to override Don't naively derive types for the single method's signature from the provided function's type, as it may be a subtype of the method's MethodType. Instead, once the sam class type is fully defined, determine the sam's info as seen from the class's type, and use those to generate the correct override. ``` scala> Arrays.stream(Array(1, 2, 3)).map(n => 2 * n + 1).average.ifPresent(println) 5.0 scala> IntStream.range(1, 4).forEach(println) 1 2 3 ``` Also, minimal error reporting Can't figure out how to do it properly, but some reporting is better than crashing. Right? Test case that illustrates necessity of the clumsy stop gap `if (block exists (_.isErroneous))` enclosed as `sammy_error_exist_no_crash` added TODO for repeated and by-name params --- .../scala/tools/nsc/typechecker/Typers.scala | 51 ++++++++++++++++++++-- .../scala/reflect/internal/tpe/TypeMaps.scala | 2 +- test/files/neg/sammy_error_exist_no_crash.check | 6 +++ test/files/neg/sammy_error_exist_no_crash.flags | 1 + test/files/neg/sammy_error_exist_no_crash.scala | 6 +++ test/files/neg/sammy_restrictions.scala | 28 ++++++------ test/files/pos/sammy_exist.scala | 13 ++---- test/files/pos/sammy_override.flags | 1 + test/files/pos/sammy_override.scala | 8 ++++ 9 files changed, 88 insertions(+), 28 deletions(-) create mode 100644 test/files/neg/sammy_error_exist_no_crash.check create mode 100644 test/files/neg/sammy_error_exist_no_crash.flags create mode 100644 test/files/neg/sammy_error_exist_no_crash.scala create mode 100644 test/files/pos/sammy_override.flags create mode 100644 test/files/pos/sammy_override.scala (limited to 'test/files/neg') diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index c41b81aaf5..5eee60031e 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -2700,7 +2700,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper * `{ * def apply$body(p1: T1, ..., pN: TN): T = body * new S { - * def apply(p1: T1, ..., pN: TN): T = apply$body(p1,..., pN) + * def apply(p1: T1', ..., pN: TN'): T' = apply$body(p1,..., pN) * } * }` * @@ -2710,6 +2710,10 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper * * The `apply` method is identified by the argument `sam`; `S` corresponds to the argument `samClassTp`, * and `resPt` is derived from `samClassTp` -- it may be fully defined, or not... + * If it is not fully defined, we derive `samClassTpFullyDefined` by inferring any unknown type parameters. + * + * The types T1' ... TN' and T' are derived from the method signature of the sam method, + * as seen from the fully defined `samClassTpFullyDefined`. * * The function's body is put in a method outside of the class definition to enforce scoping. * S's members should not be in scope in `body`. @@ -2721,6 +2725,35 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper * However T must be fully defined before we type the instantiation, as it'll end up as a parent type, * which must be fully defined. Would be nice to have some kind of mechanism to insert type vars in a block of code, * and have the instantiation of the first occurrence propagate to the rest of the block. + * + * TODO: repeated and by-name params + * scala> trait LazySink { def accept(a: => Any): Unit } + * defined trait LazySink + * + * scala> val f: LazySink = (a) => (a, a) + * f: LazySink = $anonfun$1@1fb26910 + * + * scala> f(println("!")) + * :10: error: LazySink does not take parameters + * f(println("!")) + * ^ + * + * scala> f.accept(println("!")) + * ! + * ! + * This looks like a bug: + * + * scala> trait RepeatedSink { def accept(a: Any*): Unit } + * defined trait RepeatedSink + * + * scala> val f: RepeatedSink = (a) => println(a) + * f: RepeatedSink = $anonfun$1@4799abc2 + * + * scala> f.accept(1) + * WrappedArray(WrappedArray(1)) + * + * scala> f.accept(1, 2) + * WrappedArray(WrappedArray(1, 2)) */ def synthesizeSAMFunction(sam: Symbol, fun: Function, resPt: Type, samClassTp: Type, mode: Mode): Tree = { // assert(fun.vparams forall (vp => isFullyDefined(vp.tpt.tpe))) -- by construction, as we take them from sam's info @@ -2801,13 +2834,20 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper samClassTp } - // `final override def ${sam.name}($p1: $T1, ..., $pN: $TN): $resPt = ${sam.name}\$body'($p1, ..., $pN)` + // what's the signature of the method that we should actually be overriding? + val samMethTp = samClassTpFullyDefined memberInfo sam + // Before the mutation, `tp <:< vpar.tpt.tpe` should hold. + // TODO: error message when this is not the case, as the expansion won't type check + // - Ti' <:< Ti and T <: T' must hold for the samDef body to type check + val funArgTps = foreach2(samMethTp.paramTypes, fun.vparams)((tp, vpar) => vpar.tpt setType tp) + + // `final override def ${sam.name}($p1: $T1', ..., $pN: $TN'): ${samMethTp.finalResultType} = ${sam.name}\$body'($p1, ..., $pN)` val samDef = DefDef(Modifiers(FINAL | OVERRIDE | SYNTHETIC), sam.name.toTermName, Nil, List(fun.vparams), - TypeTree(samBodyDef.tpt.tpe) setPos sampos.focus, + TypeTree(samMethTp.finalResultType) setPos sampos.focus, Apply(Ident(bodyName), fun.vparams map (p => Ident(p.name))) ) @@ -2838,6 +2878,11 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper ) } + // TODO: improve error reporting -- when we're in silent mode (from `silent(_.doTypedApply(tree, fun, args, mode, pt)) orElse onError`) + // the errors in the function don't get out... + if (block exists (_.isErroneous)) + context.error(fun.pos, s"Could not derive subclass of $samClassTp\n (with SAM `def $sam$samMethTp`)\n based on: $fun.") + classDef.symbol addAnnotation SerialVersionUIDAnnotation block } diff --git a/src/reflect/scala/reflect/internal/tpe/TypeMaps.scala b/src/reflect/scala/reflect/internal/tpe/TypeMaps.scala index 662306d5ab..c705ca7069 100644 --- a/src/reflect/scala/reflect/internal/tpe/TypeMaps.scala +++ b/src/reflect/scala/reflect/internal/tpe/TypeMaps.scala @@ -432,7 +432,7 @@ private[internal] trait TypeMaps { object wildcardExtrapolation extends TypeMap(trackVariance = true) { def apply(tp: Type): Type = tp match { - case BoundedWildcardType(TypeBounds(lo, AnyTpe)) if variance.isContravariant =>lo + case BoundedWildcardType(TypeBounds(lo, AnyTpe)) if variance.isContravariant => lo case BoundedWildcardType(TypeBounds(NothingTpe, hi)) if variance.isCovariant => hi case tp => mapOver(tp) } diff --git a/test/files/neg/sammy_error_exist_no_crash.check b/test/files/neg/sammy_error_exist_no_crash.check new file mode 100644 index 0000000000..a0d2237ce0 --- /dev/null +++ b/test/files/neg/sammy_error_exist_no_crash.check @@ -0,0 +1,6 @@ +sammy_error_exist_no_crash.scala:5: error: Could not derive subclass of F[? >: String] + (with SAM `def method apply(s: String)Int`) + based on: ((x$1: String) => x$1.). + bar(_.parseInt) + ^ +one error found diff --git a/test/files/neg/sammy_error_exist_no_crash.flags b/test/files/neg/sammy_error_exist_no_crash.flags new file mode 100644 index 0000000000..e1b37447c9 --- /dev/null +++ b/test/files/neg/sammy_error_exist_no_crash.flags @@ -0,0 +1 @@ +-Xexperimental \ No newline at end of file diff --git a/test/files/neg/sammy_error_exist_no_crash.scala b/test/files/neg/sammy_error_exist_no_crash.scala new file mode 100644 index 0000000000..da7e47206f --- /dev/null +++ b/test/files/neg/sammy_error_exist_no_crash.scala @@ -0,0 +1,6 @@ +abstract class F[T] { def apply(s: T): Int } + +object NeedsNiceError { + def bar(x: F[_ >: String]) = ??? + bar(_.parseInt) +} \ No newline at end of file diff --git a/test/files/neg/sammy_restrictions.scala b/test/files/neg/sammy_restrictions.scala index 5f1a04cd20..d003cfaf36 100644 --- a/test/files/neg/sammy_restrictions.scala +++ b/test/files/neg/sammy_restrictions.scala @@ -1,28 +1,28 @@ -class NoAbstract +abstract class NoAbstract -class TwoAbstract { def ap(a: Int): Int; def pa(a: Int): Int } +abstract class TwoAbstract { def ap(a: Int): Int; def pa(a: Int): Int } -class Base // check that the super class constructor isn't considered. -class NoEmptyConstructor(a: Int) extends Base { def this(a: String) = this(0); def ap(a: Int): Int } +abstract class Base // check that the super class constructor isn't considered. +abstract class NoEmptyConstructor(a: Int) extends Base { def this(a: String) = this(0); def ap(a: Int): Int } -class OneEmptyConstructor() { def this(a: Int) = this(); def ap(a: Int): Int } +abstract class OneEmptyConstructor() { def this(a: Int) = this(); def ap(a: Int): Int } -class OneEmptySecondaryConstructor(a: Int) { def this() = this(0); def ap(a: Int): Int } +abstract class OneEmptySecondaryConstructor(a: Int) { def this() = this(0); def ap(a: Int): Int } -class MultipleConstructorLists()() { def ap(a: Int): Int } +abstract class MultipleConstructorLists()() { def ap(a: Int): Int } -class MultipleMethodLists()() { def ap(a: Int)(): Int } +abstract class MultipleMethodLists()() { def ap(a: Int)(): Int } -class ImplicitConstructorParam()(implicit a: String) { def ap(a: Int): Int } +abstract class ImplicitConstructorParam()(implicit a: String) { def ap(a: Int): Int } -class ImplicitMethodParam() { def ap(a: Int)(implicit b: String): Int } +abstract class ImplicitMethodParam() { def ap(a: Int)(implicit b: String): Int } -class PolyClass[T] { def ap(a: T): T } +abstract class PolyClass[T] { def ap(a: T): T } -class PolyMethod { def ap[T](a: T): T } +abstract class PolyMethod { def ap[T](a: T): T } -class OneAbstract { def ap(a: Any): Any } -class DerivedOneAbstract extends OneAbstract +abstract class OneAbstract { def ap(a: Int): Any } +abstract class DerivedOneAbstract extends OneAbstract object Test { implicit val s: String = "" diff --git a/test/files/pos/sammy_exist.scala b/test/files/pos/sammy_exist.scala index 95c7c7b68e..f05ae20463 100644 --- a/test/files/pos/sammy_exist.scala +++ b/test/files/pos/sammy_exist.scala @@ -9,16 +9,9 @@ class S[T](x: T) { def map[R](f: Fun[_ >: T, _ <: R]): R = f(x) } class Bla { def foo: Bla = this } +// NOTE: inferred types show unmoored skolems, should pack them to display properly as bounded wildcards object T { - val aBlaSAM = (new S(new Bla)).map(_.foo) // : Bla should be inferred, when running under -Xexperimental [TODO] + val aBlaSAM = (new S(new Bla)).map(_.foo) val fun: Fun[Bla, Bla] = (x: Bla) => x - val aBlaSAMX = (new S(new Bla)).map(fun) // : Bla should be inferred, when running under -Xexperimental [TODO] + val aBlaSAMX = (new S(new Bla)).map(fun) } -// -// // or, maybe by variance-cast? -// import annotation.unchecked.{uncheckedVariance => uv} -// type SFun[-A, +B] = Fun[_ >: A, _ <: B @uv] -// -// def jf[T, R](f: T => R): SFun[T, R] = (x: T) => f(x): R -// -// val aBlaSAM = (new S(new Bla)).map(jf(_.foo)) // : Bla should be inferred [type checks, but existential inferred] \ No newline at end of file diff --git a/test/files/pos/sammy_override.flags b/test/files/pos/sammy_override.flags new file mode 100644 index 0000000000..48fd867160 --- /dev/null +++ b/test/files/pos/sammy_override.flags @@ -0,0 +1 @@ +-Xexperimental diff --git a/test/files/pos/sammy_override.scala b/test/files/pos/sammy_override.scala new file mode 100644 index 0000000000..a1d0651c39 --- /dev/null +++ b/test/files/pos/sammy_override.scala @@ -0,0 +1,8 @@ +trait IntConsumer { + def consume(x: Int): Unit +} + +object Test { + def anyConsumer(x: Any): Unit = ??? + val f: IntConsumer = anyConsumer +} \ No newline at end of file -- cgit v1.2.3 From 5f50e0368e635db8279797095dbd34470722d5c7 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Sat, 8 Nov 2014 22:50:47 +1000 Subject: SI-8534 Avoid crash in erroneous SelectFromTypeTree PR #2374 changed the behaviour of `typedSingletonTypeTree` in the presence of an error typed reference tree. It incorrectly returns the reference tree in case on an error. However, this is a term tree, which is an inconsistent result with the input type tree. Consequently, a `typedSelectInternal` later fails when using this as the qualifier of a `SelectFromTypeTree`. Both test cases enclosed show this symptom. This commit: - Returns `tree` rather than `refTyped` when `refTyped` is error typed or when it isn't suitable as a stable prefix. - Avoids issuing a cascading "not a stable prefix" error if the `refTyped` is error typed. - Adds an extra layer of defense in `typedSelectFromTypeTree` to bail out quickly if the qualifier is error typed. The last measure is not necessary to fix this bug. --- src/compiler/scala/tools/nsc/typechecker/Typers.scala | 13 ++++++++----- test/files/neg/t8534.check | 4 ++++ test/files/neg/t8534.scala | 7 +++++++ test/files/neg/t8534b.check | 4 ++++ test/files/neg/t8534b.scala | 4 ++++ test/files/neg/t963.check | 8 ++++---- 6 files changed, 31 insertions(+), 9 deletions(-) create mode 100644 test/files/neg/t8534.check create mode 100644 test/files/neg/t8534.scala create mode 100644 test/files/neg/t8534b.check create mode 100644 test/files/neg/t8534b.scala (limited to 'test/files/neg') diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index 70acb03584..64fbad04c0 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -5134,16 +5134,19 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper typed(tree.ref, MonoQualifierModes | mode.onlyTypePat, AnyRefTpe) } - if (!refTyped.isErrorTyped) + if (refTyped.isErrorTyped) { + setError(tree) + } else { tree setType refTyped.tpe.resultType - - if (treeInfo.admitsTypeSelection(refTyped)) tree - else UnstableTreeError(refTyped) + if (refTyped.isErrorTyped || treeInfo.admitsTypeSelection(refTyped)) tree + else UnstableTreeError(tree) + } } def typedSelectFromTypeTree(tree: SelectFromTypeTree) = { val qual1 = typedType(tree.qualifier, mode) - if (qual1.tpe.isVolatile) TypeSelectionFromVolatileTypeError(tree, qual1) + if (qual1.isErrorTyped) setError(treeCopy.SelectFromTypeTree(tree, qual1, tree.name)) + else if (qual1.tpe.isVolatile) TypeSelectionFromVolatileTypeError(tree, qual1) else typedSelect(tree, qual1, tree.name) } diff --git a/test/files/neg/t8534.check b/test/files/neg/t8534.check new file mode 100644 index 0000000000..297e7c1beb --- /dev/null +++ b/test/files/neg/t8534.check @@ -0,0 +1,4 @@ +t8534.scala:6: error: MyTrait is not an enclosing class + class BugTest {def isTheBugHere(in: MyTrait.this.type#SomeData) = false} + ^ +one error found diff --git a/test/files/neg/t8534.scala b/test/files/neg/t8534.scala new file mode 100644 index 0000000000..f118d22b82 --- /dev/null +++ b/test/files/neg/t8534.scala @@ -0,0 +1,7 @@ +object line1 { + trait MyTrait +} +object line2 { + import line2._ + class BugTest {def isTheBugHere(in: MyTrait.this.type#SomeData) = false} +} diff --git a/test/files/neg/t8534b.check b/test/files/neg/t8534b.check new file mode 100644 index 0000000000..39ffa41194 --- /dev/null +++ b/test/files/neg/t8534b.check @@ -0,0 +1,4 @@ +t8534b.scala:3: error: stable identifier required, but foo.type found. + type T = foo.type#Foo + ^ +one error found diff --git a/test/files/neg/t8534b.scala b/test/files/neg/t8534b.scala new file mode 100644 index 0000000000..73b6703a9c --- /dev/null +++ b/test/files/neg/t8534b.scala @@ -0,0 +1,4 @@ +object Test { + def foo = "" + type T = foo.type#Foo +} diff --git a/test/files/neg/t963.check b/test/files/neg/t963.check index 4dc202c7bd..483e53c77d 100644 --- a/test/files/neg/t963.check +++ b/test/files/neg/t963.check @@ -1,9 +1,9 @@ -t963.scala:14: error: stable identifier required, but Test.this.y3.x found. +t963.scala:14: error: stable identifier required, but y3.x.type found. val w3 : y3.x.type = y3.x - ^ -t963.scala:17: error: stable identifier required, but Test.this.y4.x found. + ^ +t963.scala:17: error: stable identifier required, but y4.x.type found. val w4 : y4.x.type = y4.x - ^ + ^ t963.scala:10: error: type mismatch; found : AnyRef{def x: Integer} required: AnyRef{val x: Integer} -- cgit v1.2.3 From 32196749105c9f98a33712fddc21b1676250d046 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Fri, 16 May 2014 23:35:43 +0200 Subject: SI-8597 Improved pattern unchecked warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The spec says that `case _: List[Int]` should be always issue an unchecked warning: > Types which are not of one of the forms described above are > also accepted as type patterns. However, such type patterns > will be translated to their erasure (§3.7). The Scala compiler > will issue an “unchecked” warning for these patterns to flag > the possible loss of type-safety. But the implementation goes a little further to omit warnings based on the static type of the scrutinee. As a trivial example: def foo(s: Seq[Int]) = s match { case _: List[Int] => } need not issue this warning. These discriminating unchecked warnings are domain of `CheckabilityChecker`. Let's deconstruct the reported bug: def nowarn[T] = (null: Any) match { case _: Some[T] => } We used to determine that if the first case matched, the scrutinee type would be `Some[Any]` (`Some` is covariant). If this statically matches `Some[T]` in a pattern context, we don't need to issue an unchecked warning. But, our blanket use of `existentialAbstraction` in `matchesPattern` loosened the pattern type to `Some[Any]`, and the scrutinee type was deemed compatible. I've added a new method, `scrutConformsToPatternType` which replaces pattern type variables by wildcards, but leaves other abstract types intact in the pattern type. We have to use this inside `CheckabilityChecker` only. If we were to make `matchesPattern` stricter in the same way, tests like `pos/t2486.scala` would fail. I have introduced a new symbol test to (try to) identify pattern type variables introduced by `typedBind`. Its not pretty, and it might be cleaner to reserve a new flag for these. I've also included a test variation exercising with nested matches. The pattern type of the inner case can't, syntactically, refer to the pattern type variable of the enclosing case. If it could, we would have to be more selective in our wildcarding in `ptMatchesPatternType` by restricting ourselves to type variables associated with the closest enclosing `CaseDef`. As some further validation of the correctness of this patch, four stray warnings have been teased out of neg/unchecked-abstract.scala I also had to changes `typeArgsInTopLevelType` to extract the type arguments of `Array[T]` if `T` is an abstract type. This avoids the "Checkability checker says 'Uncheckable', but uncheckable type cannot be found" warning and consequent overly lenient analysis. Without this change, the warning was suppressed for: def warnArray[T] = (null: Any) match { case _: Array[T] => } --- .../scala/tools/nsc/typechecker/Checkable.scala | 25 ++++++++++++++++---- src/reflect/scala/reflect/internal/Symbols.scala | 4 ++++ test/files/neg/t8597.check | 21 +++++++++++++++++ test/files/neg/t8597.flags | 1 + test/files/neg/t8597.scala | 27 ++++++++++++++++++++++ test/files/neg/t8597b.check | 6 +++++ test/files/neg/t8597b.flags | 1 + test/files/neg/t8597b.scala | 21 +++++++++++++++++ test/files/neg/unchecked-abstract.check | 14 ++++++++++- 9 files changed, 115 insertions(+), 5 deletions(-) create mode 100644 test/files/neg/t8597.check create mode 100644 test/files/neg/t8597.flags create mode 100644 test/files/neg/t8597.scala create mode 100644 test/files/neg/t8597b.check create mode 100644 test/files/neg/t8597b.flags create mode 100644 test/files/neg/t8597b.scala (limited to 'test/files/neg') diff --git a/src/compiler/scala/tools/nsc/typechecker/Checkable.scala b/src/compiler/scala/tools/nsc/typechecker/Checkable.scala index 3a77cab919..fcc3f6901c 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Checkable.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Checkable.scala @@ -100,7 +100,7 @@ trait Checkable { private def typeArgsInTopLevelType(tp: Type): List[Type] = { val tps = tp match { case RefinedType(parents, _) => parents flatMap typeArgsInTopLevelType - case TypeRef(_, ArrayClass, arg :: Nil) => typeArgsInTopLevelType(arg) + case TypeRef(_, ArrayClass, arg :: Nil) => if (arg.typeSymbol.isAbstractType) arg :: Nil else typeArgsInTopLevelType(arg) case TypeRef(pre, sym, args) => typeArgsInTopLevelType(pre) ++ args case ExistentialType(tparams, underlying) => tparams.map(_.tpe) ++ typeArgsInTopLevelType(underlying) case _ => Nil @@ -108,14 +108,31 @@ trait Checkable { tps filterNot isUnwarnableTypeArg } + private def scrutConformsToPatternType(scrut: Type, pattTp: Type): Boolean = { + def typeVarToWildcard(tp: Type) = { + // The need for typeSymbolDirect is demonstrated in neg/t8597b.scala + if (tp.typeSymbolDirect.isPatternTypeVariable) WildcardType else tp + } + val pattTpWild = pattTp.map(typeVarToWildcard) + scrut <:< pattTpWild + } + private class CheckabilityChecker(val X: Type, val P: Type) { def Xsym = X.typeSymbol def Psym = P.typeSymbol - def XR = if (Xsym == AnyClass) classExistentialType(Psym) else propagateKnownTypes(X, Psym) + def PErased = { + P match { + case erasure.GenericArray(n, core) => existentialAbstraction(core.typeSymbol :: Nil, P) + case _ => existentialAbstraction(Psym.typeParams, Psym.tpe_*) + } + } + def XR = if (Xsym == AnyClass) PErased else propagateKnownTypes(X, Psym) + + // sadly the spec says (new java.lang.Boolean(true)).isInstanceOf[scala.Boolean] - def P1 = X matchesPattern P + def P1 = scrutConformsToPatternType(X, P) def P2 = !Psym.isPrimitiveValueClass && isNeverSubType(X, P) - def P3 = isNonRefinementClassType(P) && (XR matchesPattern P) + def P3 = isNonRefinementClassType(P) && scrutConformsToPatternType(XR, P) def P4 = !(P1 || P2 || P3) def summaryString = f""" diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala index 51f06b1d6d..b2f176e894 100644 --- a/src/reflect/scala/reflect/internal/Symbols.scala +++ b/src/reflect/scala/reflect/internal/Symbols.scala @@ -792,6 +792,10 @@ trait Symbols extends api.Symbols { self: SymbolTable => final def isDefinedInPackage = effectiveOwner.isPackageClass final def needsFlatClasses = phase.flatClasses && rawowner != NoSymbol && !rawowner.isPackageClass + // TODO introduce a flag for these? + final def isPatternTypeVariable: Boolean = + isAbstractType && !isExistential && !isTypeParameterOrSkolem && isLocalToBlock + /** change name by appending $$ * Do the same for any accessed symbols or setters/getters. * Implementation in TermSymbol. diff --git a/test/files/neg/t8597.check b/test/files/neg/t8597.check new file mode 100644 index 0000000000..bc945f9191 --- /dev/null +++ b/test/files/neg/t8597.check @@ -0,0 +1,21 @@ +t8597.scala:2: warning: abstract type T in type pattern Some[T] is unchecked since it is eliminated by erasure + def nowarn[T] = (null: Any) match { case _: Some[T] => } // warn (did not warn due to SI-8597) + ^ +t8597.scala:5: warning: abstract type pattern T is unchecked since it is eliminated by erasure + def warn1[T] = (null: Any) match { case _: T => } // warn + ^ +t8597.scala:6: warning: non-variable type argument String in type pattern Some[String] is unchecked since it is eliminated by erasure + def warn2 = (null: Any) match { case _: Some[String] => } // warn + ^ +t8597.scala:7: warning: non-variable type argument Unchecked.this.C in type pattern Some[Unchecked.this.C] is unchecked since it is eliminated by erasure + (null: Any) match { case _: Some[C] => } // warn + ^ +t8597.scala:18: warning: abstract type T in type pattern Array[T] is unchecked since it is eliminated by erasure + def warnArray[T] = (null: Any) match { case _: Array[T] => } // warn (did not warn due to SI-8597) + ^ +t8597.scala:26: warning: non-variable type argument String in type pattern Array[Array[List[String]]] is unchecked since it is eliminated by erasure + def warnArrayErasure2 = (null: Any) match {case Some(_: Array[Array[List[String]]]) => } // warn + ^ +error: No warnings can be incurred under -Xfatal-warnings. +6 warnings found +one error found diff --git a/test/files/neg/t8597.flags b/test/files/neg/t8597.flags new file mode 100644 index 0000000000..85d8eb2ba2 --- /dev/null +++ b/test/files/neg/t8597.flags @@ -0,0 +1 @@ +-Xfatal-warnings diff --git a/test/files/neg/t8597.scala b/test/files/neg/t8597.scala new file mode 100644 index 0000000000..068e87d91a --- /dev/null +++ b/test/files/neg/t8597.scala @@ -0,0 +1,27 @@ +class Unchecked[C] { + def nowarn[T] = (null: Any) match { case _: Some[T] => } // warn (did not warn due to SI-8597) + + // These warned before. + def warn1[T] = (null: Any) match { case _: T => } // warn + def warn2 = (null: Any) match { case _: Some[String] => } // warn + (null: Any) match { case _: Some[C] => } // warn + + // These must remain without warnings. These are excerpts from + // related tests that are more exhauative. + class C; class D extends C + def okay = (List(new D) : Seq[D]) match { case _: List[C] => case _ => } // nowarn + class B2[A, B] + class A2[X] extends B2[X, String] + def okay2(x: A2[Int]) = x match { case _: B2[Int, _] => true } // nowarn + def okay3(x: A2[Int]) = x match { case _: B2[Int, typeVar] => true } // nowarn + + def warnArray[T] = (null: Any) match { case _: Array[T] => } // warn (did not warn due to SI-8597) + def nowarnArrayC = (null: Any) match { case _: Array[C] => } // nowarn + + def nowarnArrayTypeVar[T] = (null: Any) match { case _: Array[t] => } // nowarn + + def noWarnArrayErasure1 = (null: Any) match {case Some(_: Array[String]) => } // nowarn + def noWarnArrayErasure2 = (null: Any) match {case Some(_: Array[List[_]]) => } // nowarn + def noWarnArrayErasure3 = (null: Any) match {case Some(_: Array[Array[List[_]]]) => } // nowarn + def warnArrayErasure2 = (null: Any) match {case Some(_: Array[Array[List[String]]]) => } // warn +} diff --git a/test/files/neg/t8597b.check b/test/files/neg/t8597b.check new file mode 100644 index 0000000000..3c45a31337 --- /dev/null +++ b/test/files/neg/t8597b.check @@ -0,0 +1,6 @@ +t8597b.scala:18: warning: non-variable type argument T in type pattern Some[T] is unchecked since it is eliminated by erasure + case _: Some[T] => // warn + ^ +error: No warnings can be incurred under -Xfatal-warnings. +one warning found +one error found diff --git a/test/files/neg/t8597b.flags b/test/files/neg/t8597b.flags new file mode 100644 index 0000000000..85d8eb2ba2 --- /dev/null +++ b/test/files/neg/t8597b.flags @@ -0,0 +1 @@ +-Xfatal-warnings diff --git a/test/files/neg/t8597b.scala b/test/files/neg/t8597b.scala new file mode 100644 index 0000000000..b29d591cb1 --- /dev/null +++ b/test/files/neg/t8597b.scala @@ -0,0 +1,21 @@ +object Unchecked { + (null: Any) match { + case _: Some[t] => + + // t is a fresh pattern type variable, despite our attempts to + // backtick our way to the enclosing `t`. Under this interpretation, + // the absense of an unchecked warning is expected. + (null: Any) match { + case _: Some[t] => // no warn + } + (null: Any) match { + case _: Some[`t`] => // no warn + } + + // here we correctly issue an unchecked warning + type T = t + (null: Any) match { + case _: Some[T] => // warn + } + } +} diff --git a/test/files/neg/unchecked-abstract.check b/test/files/neg/unchecked-abstract.check index 72019082ac..703929dca8 100644 --- a/test/files/neg/unchecked-abstract.check +++ b/test/files/neg/unchecked-abstract.check @@ -4,6 +4,9 @@ unchecked-abstract.scala:16: warning: abstract type H in type Contravariant[M.th unchecked-abstract.scala:21: warning: abstract type H in type Contravariant[M.this.H] is unchecked since it is eliminated by erasure /* warn */ println(x.isInstanceOf[Contravariant[H]]) ^ +unchecked-abstract.scala:22: warning: abstract type T in type Contravariant[M.this.T] is unchecked since it is eliminated by erasure + /* warn */ println(x.isInstanceOf[Contravariant[T]]) + ^ unchecked-abstract.scala:27: warning: abstract type T in type Invariant[M.this.T] is unchecked since it is eliminated by erasure /* warn */ println(x.isInstanceOf[Invariant[T]]) ^ @@ -22,6 +25,15 @@ unchecked-abstract.scala:36: warning: abstract type H in type Invariant[M.this.H unchecked-abstract.scala:37: warning: abstract type T in type Invariant[M.this.T] is unchecked since it is eliminated by erasure /* warn */ println(x.isInstanceOf[Invariant[T]]) ^ +unchecked-abstract.scala:42: warning: abstract type T in type Covariant[M.this.T] is unchecked since it is eliminated by erasure + /* warn */ println(x.isInstanceOf[Covariant[T]]) + ^ +unchecked-abstract.scala:43: warning: abstract type L in type Covariant[M.this.L] is unchecked since it is eliminated by erasure + /* warn */ println(x.isInstanceOf[Covariant[L]]) + ^ +unchecked-abstract.scala:48: warning: abstract type L in type Covariant[M.this.L] is unchecked since it is eliminated by erasure + /* warn */ println(x.isInstanceOf[Covariant[L]]) + ^ error: No warnings can be incurred under -Xfatal-warnings. -8 warnings found +12 warnings found one error found -- cgit v1.2.3 From 90a98f7c41cfcfdb60dc25db5ac33d0d0ff10f99 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Sun, 9 Nov 2014 18:29:35 +1000 Subject: SI-5639 Fix spurious discarding of implicit import In Scala fa0cdc7b (just before 2.9.0), a regression in implicit search, SI-2866, was discovered by Lift and fixed. The nature of the regression was that an in-scope, non-implicit symbol failed to shadow an eponymous implicit import. The fix for this introduced `isQualifyingImplicit` which discards in-scope implicits when the current `Context`'s scope contains a name-clashing entry. Incidentally, this proved to be a shallow solution, and we later improved shadowing detection in SI-4270 / 9129cfe9. That picked up cases where a locally defined symbol in an intervening scope shadowed an implicit. This commit includes the test case from the comments of SI-2866. Part of it is tested as a neg test (to test reporting of ambiguities), and the rest is tested in a run test (to test which implicits are picked.) However, in the test case of SI-5639, we see that the scope lookup performend by `isQualifyingImplicit` is fooled by a "ghost" module symbol. The apparition I'm referring to is entered when `initializeFromClassPath` / `enterClassAndModule` encounters a class file named 'Baz.class', and speculatively enters _both_ a class and module symbol. AFAIK, this is done to defer parsing the class file to determine what inside. If it happens to be a Java compiled class, the module symbol is needed to house the static members. This commit adds a condition that `Symbol#exists` which shines a torch (forces the info) in the direction of the ghost module symbol. In our test, this causes it to vanish, as we only need a class symbol for the Scala defined `class Baz`. The existing `pos` test for this actually did not exercise the bug, separate compilation is required. It was originally checked in to `pending` with this error, and then later moved to `pos` when someone noticed it was not failing. --- .../scala/tools/nsc/typechecker/Contexts.scala | 2 +- test/files/neg/t2866.check | 17 +++++++ test/files/neg/t2866.scala | 59 ++++++++++++++++++++++ test/files/pos/t5639/A_1.scala | 17 +++++++ test/files/pos/t5639/A_2.scala | 11 ++++ test/files/pos/t5639/Bar.scala | 7 --- test/files/pos/t5639/Foo.scala | 7 --- test/files/run/t2866.check | 3 ++ test/files/run/t2866.scala | 44 ++++++++++++++++ 9 files changed, 152 insertions(+), 15 deletions(-) create mode 100644 test/files/neg/t2866.check create mode 100644 test/files/neg/t2866.scala create mode 100644 test/files/pos/t5639/A_1.scala create mode 100644 test/files/pos/t5639/A_2.scala delete mode 100644 test/files/pos/t5639/Bar.scala delete mode 100644 test/files/pos/t5639/Foo.scala create mode 100644 test/files/run/t2866.check create mode 100644 test/files/run/t2866.scala (limited to 'test/files/neg') diff --git a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala index e278130437..acd8a6ea7b 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala @@ -798,7 +798,7 @@ trait Contexts { self: Analyzer => isAccessible(sym, pre) && !(imported && { val e = scope.lookupEntry(name) - (e ne null) && (e.owner == scope) + (e ne null) && (e.owner == scope) && e.sym.exists }) private def collectImplicits(syms: Scope, pre: Type, imported: Boolean = false): List[ImplicitInfo] = diff --git a/test/files/neg/t2866.check b/test/files/neg/t2866.check new file mode 100644 index 0000000000..340fb8da22 --- /dev/null +++ b/test/files/neg/t2866.check @@ -0,0 +1,17 @@ +t2866.scala:30: warning: imported `one' is permanently hidden by definition of value one + import A.one // warning: imported `one' is permanently hidden by definition of value one. + ^ +t2866.scala:42: error: ambiguous implicit values: + both value two of type Int + and value one in object A of type => Int + match expected type Int + assert(implicitly[Int] == 2) // !!! Not ambiguous in 2.8.0. Ambigous in 2.7.6 + ^ +t2866.scala:50: error: ambiguous implicit values: + both value two of type Int + and value one in object A of type => Int + match expected type Int + assert(implicitly[Int] == 2) // !!! Not ambiguous in 2.8.0. Ambiguous in 2.7.6 + ^ +one warning found +two errors found diff --git a/test/files/neg/t2866.scala b/test/files/neg/t2866.scala new file mode 100644 index 0000000000..55ebff9710 --- /dev/null +++ b/test/files/neg/t2866.scala @@ -0,0 +1,59 @@ +// for 2.7.x compatibility + +object A { + implicit val one = 1 +} + +object Test { + + locally { + import A._ + locally { + // assert(implicitly[Int] == 1) // error: could not find implicit value for parameter e: Int. + // !!! Why one A.one? + // (I assume you mean: why _not_ A.one? A.one is shadowed by local one. + // but the local one cannot be used yet because it does not have an explicit type. + implicit val one = 2 + assert(implicitly[Int] == 2) + assert(one == 2) + } + } + + locally { + import A._ + implicit val one: Int = 2 + assert(implicitly[Int] == 2) + assert(one == 2) + } + + locally { + import A.one // warning: imported `one' is permanently hidden by definition of value one. + // !!! Really? + //assert(implicitly[Int] == 1) + implicit val one = 2 + assert(implicitly[Int] == 2) // !!! why not 2? + assert(one == 2) + } + + locally { + import A.one + assert(implicitly[Int] == 1) + implicit val two = 2 + assert(implicitly[Int] == 2) // !!! Not ambiguous in 2.8.0. Ambigous in 2.7.6 + } + + locally { + import A._ + assert(implicitly[Int] == 1) + implicit val two = 2 + import A.{one => _} + assert(implicitly[Int] == 2) // !!! Not ambiguous in 2.8.0. Ambiguous in 2.7.6 + } + + locally { + import A.{one => _, _} + implicit val two = 2 + assert(implicitly[Int] == 2) // not ambiguous in 2.8.0 nor im ambiguous in 2.7.6 + } + +} diff --git a/test/files/pos/t5639/A_1.scala b/test/files/pos/t5639/A_1.scala new file mode 100644 index 0000000000..c5da10eae4 --- /dev/null +++ b/test/files/pos/t5639/A_1.scala @@ -0,0 +1,17 @@ +import Implicits._ + +class Baz + +object Test { + implicitly[Int] +} + +object Implicits { + implicit val Baz: Int = 0 + // This implicit was being ignored by `isQualifyingImplicit` + // if the classpath contained a class file for `class Baz`. + // This is because the package scope contains a speculative + // symbol for `object Baz` which is entered by `SymbolLoaders` + // before looking inside the class file. (A Java originated + // classfile results in the class/module symbol pair.) +} diff --git a/test/files/pos/t5639/A_2.scala b/test/files/pos/t5639/A_2.scala new file mode 100644 index 0000000000..2bb36273e0 --- /dev/null +++ b/test/files/pos/t5639/A_2.scala @@ -0,0 +1,11 @@ +import Implicits._ + +class Baz + +object Test { + implicitly[Int] +} + +object Implicits { + implicit val Baz: Int = 0 +} diff --git a/test/files/pos/t5639/Bar.scala b/test/files/pos/t5639/Bar.scala deleted file mode 100644 index f577500acd..0000000000 --- a/test/files/pos/t5639/Bar.scala +++ /dev/null @@ -1,7 +0,0 @@ -package pack.age - -import pack.age.Implicits._ - -object Quux { - def baz : Baz = 1 -} diff --git a/test/files/pos/t5639/Foo.scala b/test/files/pos/t5639/Foo.scala deleted file mode 100644 index 1a07734a8e..0000000000 --- a/test/files/pos/t5639/Foo.scala +++ /dev/null @@ -1,7 +0,0 @@ -package pack.age - -class Baz - -object Implicits { - implicit def Baz(n: Int): Baz = new Baz -} diff --git a/test/files/run/t2866.check b/test/files/run/t2866.check new file mode 100644 index 0000000000..7f52da85fb --- /dev/null +++ b/test/files/run/t2866.check @@ -0,0 +1,3 @@ +t2866.scala:30: warning: imported `one' is permanently hidden by definition of value one + import A.one // warning: imported `one' is permanently hidden by definition of value one. + ^ diff --git a/test/files/run/t2866.scala b/test/files/run/t2866.scala new file mode 100644 index 0000000000..8059107583 --- /dev/null +++ b/test/files/run/t2866.scala @@ -0,0 +1,44 @@ +// for 2.7.x compatibility + +object A { + implicit val one = 1 +} + +object Test extends App { + + locally { + import A._ + locally { + // assert(implicitly[Int] == 1) // error: could not find implicit value for parameter e: Int. + // !!! Why one A.one? + // (I assume you mean: why _not_ A.one? A.one is shadowed by local one. + // but the local one cannot be used yet because it does not have an explicit type. + implicit val one = 2 + assert(implicitly[Int] == 2) + assert(one == 2) + } + } + + locally { + import A._ + implicit val one: Int = 2 + assert(implicitly[Int] == 2) + assert(one == 2) + } + + locally { + import A.one // warning: imported `one' is permanently hidden by definition of value one. + // !!! Really? + //assert(implicitly[Int] == 1) + implicit val one = 2 + assert(implicitly[Int] == 2) // !!! why not 2? + assert(one == 2) + } + + locally { + import A.{one => _, _} + implicit val two = 2 + assert(implicitly[Int] == 2) // not ambiguous in 2.8.0 nor im ambiguous in 2.7.6 + } + +} -- cgit v1.2.3 From a77f01f546312cf6601f03794f909a09d34c5445 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Sun, 9 Nov 2014 18:30:23 +1000 Subject: SI-5639 Predicate bug fix on -Xsource:2.12 To be conservative, I've predicated this fix in `-Xsource:2.12`. This is done in separate commit to show that the previous fix passes the test suite, rather than just tests with `-Xsource:2.12`. --- src/compiler/scala/tools/nsc/typechecker/Contexts.scala | 2 +- test/files/neg/t5639b.check | 4 ++++ test/files/neg/t5639b/A_1.scala | 17 +++++++++++++++++ test/files/neg/t5639b/A_2.scala | 11 +++++++++++ test/files/pos/t5639.flags | 1 + 5 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 test/files/neg/t5639b.check create mode 100644 test/files/neg/t5639b/A_1.scala create mode 100644 test/files/neg/t5639b/A_2.scala create mode 100644 test/files/pos/t5639.flags (limited to 'test/files/neg') diff --git a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala index acd8a6ea7b..b13f9e94cc 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala @@ -798,7 +798,7 @@ trait Contexts { self: Analyzer => isAccessible(sym, pre) && !(imported && { val e = scope.lookupEntry(name) - (e ne null) && (e.owner == scope) && e.sym.exists + (e ne null) && (e.owner == scope) && (!settings.isScala212 || e.sym.exists) }) private def collectImplicits(syms: Scope, pre: Type, imported: Boolean = false): List[ImplicitInfo] = diff --git a/test/files/neg/t5639b.check b/test/files/neg/t5639b.check new file mode 100644 index 0000000000..faa1766660 --- /dev/null +++ b/test/files/neg/t5639b.check @@ -0,0 +1,4 @@ +A_2.scala:6: error: could not find implicit value for parameter e: Int + implicitly[Int] + ^ +one error found diff --git a/test/files/neg/t5639b/A_1.scala b/test/files/neg/t5639b/A_1.scala new file mode 100644 index 0000000000..c5da10eae4 --- /dev/null +++ b/test/files/neg/t5639b/A_1.scala @@ -0,0 +1,17 @@ +import Implicits._ + +class Baz + +object Test { + implicitly[Int] +} + +object Implicits { + implicit val Baz: Int = 0 + // This implicit was being ignored by `isQualifyingImplicit` + // if the classpath contained a class file for `class Baz`. + // This is because the package scope contains a speculative + // symbol for `object Baz` which is entered by `SymbolLoaders` + // before looking inside the class file. (A Java originated + // classfile results in the class/module symbol pair.) +} diff --git a/test/files/neg/t5639b/A_2.scala b/test/files/neg/t5639b/A_2.scala new file mode 100644 index 0000000000..2bb36273e0 --- /dev/null +++ b/test/files/neg/t5639b/A_2.scala @@ -0,0 +1,11 @@ +import Implicits._ + +class Baz + +object Test { + implicitly[Int] +} + +object Implicits { + implicit val Baz: Int = 0 +} diff --git a/test/files/pos/t5639.flags b/test/files/pos/t5639.flags new file mode 100644 index 0000000000..0acce1e7ce --- /dev/null +++ b/test/files/pos/t5639.flags @@ -0,0 +1 @@ +-Xsource:2.12 -- cgit v1.2.3 From 54d12f0581f6401c7693ccb0e3105868da19cec2 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Fri, 7 Nov 2014 23:12:43 +1000 Subject: SI-8502 Improve resiliance to absent packages When unpickling a class, we create stub symbols for references to classes absent from the current classpath. If these references only appear in method signatures that aren't called, we can proceed with compilation. This is in line with javac. We're getting better at this, but there are still some gaps. This bug is about the behaviour when a package is completely missing, rather than just a single class within that package. To make this work we have to add two special cases to the unpickler: - When unpickling a `ThisType`, convert a `StubTermSymbol` into a `StubTypeSymbol`. We hit this when unpickling `ThisType(missingPackage)`. - When unpickling a reference to `.name` where `` is a stub symbol, don't call info on that owner, but rather allow the enclosing code in `readSymbol` fall through to create a stub for the member. The test case was distilled from an a problem that a Spray user encountered when Akka was missing from the classpath. Two existing test cases have progressed, and the checkfiles are accordingly updated. --- src/reflect/scala/reflect/internal/Symbols.scala | 9 ++--- .../reflect/internal/pickling/UnPickler.scala | 19 ++++++++-- test/files/neg/t5148.check | 7 +++- test/files/run/t6440.check | 9 +++-- test/files/run/t6440.scala | 2 +- test/files/run/t8502.scala | 41 ++++++++++++++++++++++ 6 files changed, 74 insertions(+), 13 deletions(-) create mode 100644 test/files/run/t8502.scala (limited to 'test/files/neg') diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala index b2f176e894..c665f2b91a 100644 --- a/src/reflect/scala/reflect/internal/Symbols.scala +++ b/src/reflect/scala/reflect/internal/Symbols.scala @@ -3432,10 +3432,11 @@ trait Symbols extends api.Symbols { self: SymbolTable => trait StubSymbol extends Symbol { devWarning("creating stub symbol to defer error: " + missingMessage) - protected def missingMessage: String + def missingMessage: String /** Fail the stub by throwing a [[scala.reflect.internal.MissingRequirementError]]. */ - override final def failIfStub() = {MissingRequirementError.signal(missingMessage)} // + override final def failIfStub() = + MissingRequirementError.signal(missingMessage) /** Fail the stub by reporting an error to the reporter, setting the IS_ERROR flag * on this symbol, and returning the dummy value `alt`. @@ -3460,8 +3461,8 @@ trait Symbols extends api.Symbols { self: SymbolTable => override def rawInfo = fail(NoType) override def companionSymbol = fail(NoSymbol) } - class StubClassSymbol(owner0: Symbol, name0: TypeName, protected val missingMessage: String) extends ClassSymbol(owner0, owner0.pos, name0) with StubSymbol - class StubTermSymbol(owner0: Symbol, name0: TermName, protected val missingMessage: String) extends TermSymbol(owner0, owner0.pos, name0) with StubSymbol + class StubClassSymbol(owner0: Symbol, name0: TypeName, val missingMessage: String) extends ClassSymbol(owner0, owner0.pos, name0) with StubSymbol + class StubTermSymbol(owner0: Symbol, name0: TermName, val missingMessage: String) extends TermSymbol(owner0, owner0.pos, name0) with StubSymbol trait FreeSymbol extends Symbol { def origin: String diff --git a/src/reflect/scala/reflect/internal/pickling/UnPickler.scala b/src/reflect/scala/reflect/internal/pickling/UnPickler.scala index a35620a994..1fc7aebab0 100644 --- a/src/reflect/scala/reflect/internal/pickling/UnPickler.scala +++ b/src/reflect/scala/reflect/internal/pickling/UnPickler.scala @@ -211,7 +211,12 @@ abstract class UnPickler { def fromName(name: Name) = name.toTermName match { case nme.ROOT => loadingMirror.RootClass case nme.ROOTPKG => loadingMirror.RootPackage - case _ => adjust(owner.info.decl(name)) + case _ => + val decl = owner match { + case stub: StubSymbol => NoSymbol // SI-8502 Don't call .info and fail the stub + case _ => owner.info.decl(name) + } + adjust(decl) } def nestedObjectSymbol: Symbol = { // If the owner is overloaded (i.e. a method), it's not possible to select the @@ -389,13 +394,23 @@ abstract class UnPickler { case CLASSINFOtpe => ClassInfoType(parents, symScope(clazz), clazz) } + def readThisType(): Type = { + val sym = readSymbolRef() match { + case stub: StubSymbol if !stub.isClass => + // SI-8502 This allows us to create a stub for a unpickled reference to `missingPackage.Foo`. + stub.owner.newStubSymbol(stub.name.toTypeName, stub.missingMessage) + case sym => sym + } + ThisType(sym) + } + // We're stuck with the order types are pickled in, but with judicious use // of named parameters we can recapture a declarative flavor in a few cases. // But it's still a rat's nest of adhockery. (tag: @switch) match { case NOtpe => NoType case NOPREFIXtpe => NoPrefix - case THIStpe => ThisType(readSymbolRef()) + case THIStpe => readThisType() case SINGLEtpe => SingleType(readTypeRef(), readSymbolRef().filter(_.isStable)) // SI-7596 account for overloading case SUPERtpe => SuperType(readTypeRef(), readTypeRef()) case CONSTANTtpe => ConstantType(readConstantRef()) diff --git a/test/files/neg/t5148.check b/test/files/neg/t5148.check index 0de4fe2d4c..286ed9e04a 100644 --- a/test/files/neg/t5148.check +++ b/test/files/neg/t5148.check @@ -1,6 +1,11 @@ error: missing or invalid dependency detected while loading class file 'Imports.class'. +Could not access type Wrapper in class scala.tools.nsc.interpreter.IMain.Request, +because it (or its dependencies) are missing. Check your build definition for +missing or conflicting dependencies. (Re-run with `-Ylog-classpath` to see the problematic classpath.) +A full rebuild may help if 'Imports.class' was compiled against an incompatible version of scala.tools.nsc.interpreter.IMain.Request. +error: missing or invalid dependency detected while loading class file 'Imports.class'. Could not access type Request in class scala.tools.nsc.interpreter.IMain, because it (or its dependencies) are missing. Check your build definition for missing or conflicting dependencies. (Re-run with `-Ylog-classpath` to see the problematic classpath.) A full rebuild may help if 'Imports.class' was compiled against an incompatible version of scala.tools.nsc.interpreter.IMain. -one error found +two errors found diff --git a/test/files/run/t6440.check b/test/files/run/t6440.check index 2358f08fcc..4d8618182b 100644 --- a/test/files/run/t6440.check +++ b/test/files/run/t6440.check @@ -1,5 +1,4 @@ -pos: source-newSource1.scala,line-9,offset=109 missing or invalid dependency detected while loading class file 'U.class'. -Could not access term pack1 in package , -because it (or its dependencies) are missing. Check your build definition for -missing or conflicting dependencies. (Re-run with `-Ylog-classpath` to see the problematic classpath.) -A full rebuild may help if 'U.class' was compiled against an incompatible version of . ERROR +pos: source-newSource1.scala,line-9,offset=109 reference to U is ambiguous; +it is imported twice in the same scope by +import pack2._ +and import X._ ERROR diff --git a/test/files/run/t6440.scala b/test/files/run/t6440.scala index 5a3a4150d9..94eda3642e 100644 --- a/test/files/run/t6440.scala +++ b/test/files/run/t6440.scala @@ -41,7 +41,7 @@ object Test extends StoreReporterDirectTest { assert(tClass.delete()) assert(pack1.delete()) - // bad symbolic reference error expected (but no stack trace!) + // should report ambiguous import, despite the fact that a parent of pack2.U is absent compileCode(app) println(filteredInfos.mkString("\n")) } diff --git a/test/files/run/t8502.scala b/test/files/run/t8502.scala new file mode 100644 index 0000000000..903e573711 --- /dev/null +++ b/test/files/run/t8502.scala @@ -0,0 +1,41 @@ +import scala.tools.partest._ +import java.io.File + +object Test extends StoreReporterDirectTest { + def code = ??? + + def compileCode(code: String) = { + val classpath = List(sys.props("partest.lib"), testOutput.path) mkString sys.props("path.separator") + compileString(newCompiler("-cp", classpath, "-d", testOutput.path))(code) + } + + def show(): Unit = { + compileCode(""" + object U { + def foo(log: vanishing.Vanishing) = () + } + + package vanishing { + class Vanishing + } + """) + assert(filteredInfos.isEmpty, filteredInfos) + deletePackage("vanishing") + compileCode(""" + class Test { + U + } + """) + assert(storeReporter.infos.isEmpty, storeReporter.infos.mkString("\n")) // Included a MissingRequirementError before. + } + + def deletePackage(name: String) { + val directory = new File(testOutput.path, name) + for (f <- directory.listFiles()) { + assert(f.getName.endsWith(".class")) + assert(f.delete()) + } + assert(directory.listFiles().isEmpty) + assert(directory.delete()) + } +} -- cgit v1.2.3 From 64ae23f6438054a08bebf1fa6af0f14d63cfc0ea Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Thu, 27 Nov 2014 14:27:07 +1000 Subject: SI-9008 Fix regression with higher kinded existentials Allow a naked type constructor in an existential type if we are directly within a type application. Recently, 84d4671 changed nested context creation to avoid passing down the `TypeConstructorAllowed`, which led to missing kind errors in code like `type T[({type M = List})#M]`. However, when typechecking `T forSome { quantifiers }`, we create a nested context to represent the nested scope introduced for the quantifiers. But we need to propagate the `TypeConstructorAllowed` bit to the nested context to allow for higher kinded existentials. The enclosed tests show: - pos/t9008 well kinded application of an hk existential - neg/t9008 hk existential forbidden outside of type application - neg/t9008b kind error reported for hk existential Regressed in 84d4671. --- src/compiler/scala/tools/nsc/typechecker/Contexts.scala | 3 ++- src/compiler/scala/tools/nsc/typechecker/Typers.scala | 6 +++++- test/files/neg/t9008.check | 4 ++++ test/files/neg/t9008.scala | 3 +++ test/files/neg/t9008b.check | 4 ++++ test/files/neg/t9008b.scala | 3 +++ test/files/pos/t9008.scala | 5 +++++ 7 files changed, 26 insertions(+), 2 deletions(-) create mode 100644 test/files/neg/t9008.check create mode 100644 test/files/neg/t9008.scala create mode 100644 test/files/neg/t9008b.check create mode 100644 test/files/neg/t9008b.scala create mode 100644 test/files/pos/t9008.scala (limited to 'test/files/neg') diff --git a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala index b13f9e94cc..9a34e8dfed 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala @@ -480,7 +480,8 @@ trait Contexts { self: Analyzer => // SI-8245 `isLazy` need to skip lazy getters to ensure `return` binds to the right place c.enclMethod = if (isDefDef && !owner.isLazy) c else enclMethod - if (tree != outer.tree) c(TypeConstructorAllowed) = false + if (tree != outer.tree) + c(TypeConstructorAllowed) = false registerContext(c.asInstanceOf[analyzer.Context]) debuglog("[context] ++ " + c.unit + " / " + tree.summaryString) diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index 4d9a6a47ef..aaa75b5ee1 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -5207,7 +5207,11 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper def typedExistentialTypeTree(tree: ExistentialTypeTree) = { val tree1 = typerWithLocalContext(context.makeNewScope(tree, context.owner)){ - _.typedExistentialTypeTree(tree, mode) + typer => + if (context.inTypeConstructorAllowed) + typer.context.withinTypeConstructorAllowed(typer.typedExistentialTypeTree(tree, mode)) + else + typer.typedExistentialTypeTree(tree, mode) } checkExistentialsFeature(tree1.pos, tree1.tpe, "the existential type") tree1 diff --git a/test/files/neg/t9008.check b/test/files/neg/t9008.check new file mode 100644 index 0000000000..c32bc41baf --- /dev/null +++ b/test/files/neg/t9008.check @@ -0,0 +1,4 @@ +t9008.scala:2: error: type M takes type parameters + def x: List[M forSome { type M[_] }] = ??? + ^ +one error found diff --git a/test/files/neg/t9008.scala b/test/files/neg/t9008.scala new file mode 100644 index 0000000000..c6a5389e42 --- /dev/null +++ b/test/files/neg/t9008.scala @@ -0,0 +1,3 @@ +object Test { + def x: List[M forSome { type M[_] }] = ??? +} diff --git a/test/files/neg/t9008b.check b/test/files/neg/t9008b.check new file mode 100644 index 0000000000..5e911fc138 --- /dev/null +++ b/test/files/neg/t9008b.check @@ -0,0 +1,4 @@ +t9008b.scala:2: error: type M takes type parameters + type T = M forSome { type M[_] } + ^ +one error found diff --git a/test/files/neg/t9008b.scala b/test/files/neg/t9008b.scala new file mode 100644 index 0000000000..58f9d0e8de --- /dev/null +++ b/test/files/neg/t9008b.scala @@ -0,0 +1,3 @@ +object Test { + type T = M forSome { type M[_] } +} diff --git a/test/files/pos/t9008.scala b/test/files/pos/t9008.scala new file mode 100644 index 0000000000..d11b8604f2 --- /dev/null +++ b/test/files/pos/t9008.scala @@ -0,0 +1,5 @@ +trait Monad[M[_]] + +object Test { + def x: Monad[M forSome { type M[_] }] = ??? +} -- cgit v1.2.3 From be553aaa5a765b1d1511688d58f01ec675549412 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Fri, 28 Nov 2014 13:11:48 -0800 Subject: SI-9015 Reject 0x and minor parser cleanup Only print error results. Show deprecated forms. Test for rejected literals and clean up parser There was no negative test for what constitutes a legal literal. The ultimate goal is for the test to report all errors in one compilation. This commit follows up the removal of "1." syntax to simplify number parsing. It removes previous paulp code to contain the erstwhile complexity. Leading zero is not immediately put to the buffer. Instead, the empty buffer is handled on evaluation. In particular, an empty buffer due to `0x` is a syntax error. The message for obsolete octal syntax is nuanced and deferred until evaluation by the parser, which is slightly simpler to reason about. Improve comment on usage of base The slice-and-dicey usage of base deserves a better comment. The difference is that `intVal` sees an empty char buffer for input `"0"`. --- .../scala/tools/nsc/ast/parser/Scanners.scala | 192 ++++++++------------- test/files/neg/literals.check | 40 +++++ test/files/neg/literals.scala | 36 ++++ test/files/run/literals.check | 69 ++------ test/files/run/literals.flags | 1 + test/files/run/literals.scala | 39 ++--- 6 files changed, 175 insertions(+), 202 deletions(-) create mode 100644 test/files/neg/literals.check create mode 100644 test/files/neg/literals.scala create mode 100644 test/files/run/literals.flags (limited to 'test/files/neg') diff --git a/src/compiler/scala/tools/nsc/ast/parser/Scanners.scala b/src/compiler/scala/tools/nsc/ast/parser/Scanners.scala index 9ebc94b5fc..92833d647b 100644 --- a/src/compiler/scala/tools/nsc/ast/parser/Scanners.scala +++ b/src/compiler/scala/tools/nsc/ast/parser/Scanners.scala @@ -453,18 +453,15 @@ trait Scanners extends ScannersCommon { getOperatorRest() } case '0' => - def fetchZero() = { - putChar(ch) + def fetchLeadingZero(): Unit = { nextChar() - if (ch == 'x' || ch == 'X') { - nextChar() - base = 16 - } else { - base = 8 + ch match { + case 'x' | 'X' => base = 16 ; nextChar() + case _ => base = 8 // single decimal zero, perhaps } - getNumber() } - fetchZero() + fetchLeadingZero() + getNumber() case '1' | '2' | '3' | '4' | '5' | '6' | '7' | '8' | '9' => base = 10 getNumber() @@ -902,62 +899,61 @@ trait Scanners extends ScannersCommon { */ def charVal: Char = if (strVal.length > 0) strVal.charAt(0) else 0 - /** Convert current strVal, base to long value + /** Convert current strVal, base to long value. * This is tricky because of max negative value. + * + * Conversions in base 10 and 16 are supported. As a permanent migration + * path, attempts to write base 8 literals except `0` emit a verbose error. */ def intVal(negated: Boolean): Long = { - if (token == CHARLIT && !negated) { - charVal.toLong - } else { - var value: Long = 0 - val divider = if (base == 10) 1 else 2 - val limit: Long = - if (token == LONGLIT) Long.MaxValue else Int.MaxValue - var i = 0 + def malformed: Long = { + if (base == 8) syntaxError("Decimal integer literals may not have a leading zero. (Octal syntax is obsolete.)") + else syntaxError("malformed integer number") + 0 + } + def tooBig: Long = { + syntaxError("integer number too large") + 0 + } + def intConvert: Long = { val len = strVal.length - while (i < len) { - val d = digit2int(strVal charAt i, base) - if (d < 0) { - syntaxError("malformed integer number") - return 0 - } - if (value < 0 || - limit / (base / divider) < value || - limit - (d / divider) < value * (base / divider) && - !(negated && limit == value * base - 1 + d)) { - syntaxError("integer number too large") - return 0 - } - value = value * base + d - i += 1 + if (len == 0) { + if (base != 8) syntaxError("missing integer number") // e.g., 0x; + 0 + } else { + val divider = if (base == 10) 1 else 2 + val limit: Long = if (token == LONGLIT) Long.MaxValue else Int.MaxValue + @tailrec def convert(value: Long, i: Int): Long = + if (i >= len) value + else { + val d = digit2int(strVal charAt i, base) + if (d < 0) + malformed + else if (value < 0 || + limit / (base / divider) < value || + limit - (d / divider) < value * (base / divider) && + !(negated && limit == value * base - 1 + d)) + tooBig + else + convert(value * base + d, i + 1) + } + val result = convert(0, 0) + if (base == 8) malformed else if (negated) -result else result } - if (negated) -value else value } + if (token == CHARLIT && !negated) charVal.toLong else intConvert } def intVal: Long = intVal(negated = false) /** Convert current strVal, base to double value - */ + */ def floatVal(negated: Boolean): Double = { - - val limit: Double = - if (token == DOUBLELIT) Double.MaxValue else Float.MaxValue + val limit: Double = if (token == DOUBLELIT) Double.MaxValue else Float.MaxValue try { val value: Double = java.lang.Double.valueOf(strVal).doubleValue() - def isDeprecatedForm = { - val idx = strVal indexOf '.' - (idx == strVal.length - 1) || ( - (idx >= 0) - && (idx + 1 < strVal.length) - && (!Character.isDigit(strVal charAt (idx + 1))) - ) - } if (value > limit) syntaxError("floating point number too large") - if (isDeprecatedForm) - syntaxError("floating point number is missing digit after dot") - if (negated) -value else value } catch { case _: NumberFormatException => @@ -968,86 +964,44 @@ trait Scanners extends ScannersCommon { def floatVal: Double = floatVal(negated = false) - def checkNoLetter(): Unit = { + def checkNoLetter(): Unit = { if (isIdentifierPart(ch) && ch >= ' ') syntaxError("Invalid literal number") } - /** Read a number into strVal and set base */ - protected def getNumber(): Unit = { - val base1 = if (base < 10) 10 else base - // Read 8,9's even if format is octal, produce a malformed number error afterwards. - // At this point, we have already read the first digit, so to tell an innocent 0 apart - // from an octal literal 0123... (which we want to disallow), we check whether there - // are any additional digits coming after the first one we have already read. - var notSingleZero = false - while (digit2int(ch, base1) >= 0) { - putChar(ch) - nextChar() - notSingleZero = true - } - token = INTLIT - - /* When we know for certain it's a number after using a touch of lookahead */ - def restOfNumber() = { - putChar(ch) - nextChar() + /** Read a number into strVal. + * + * The `base` can be 8, 10 or 16, where base 8 flags a leading zero. + * For ints, base 8 is legal only for the case of exactly one zero. + */ + protected def getNumber(): Unit = { + // consume digits of a radix + def consumeDigits(radix: Int): Unit = + while (digit2int(ch, radix) >= 0) { + putChar(ch) + nextChar() + } + // adding decimal point is always OK because `Double valueOf "0."` is OK + def restOfNonIntegralNumber(): Unit = { + putChar('.') + if (ch == '.') nextChar() getFraction() } - def restOfUncertainToken() = { - def isEfd = ch match { case 'e' | 'E' | 'f' | 'F' | 'd' | 'D' => true ; case _ => false } - def isL = ch match { case 'l' | 'L' => true ; case _ => false } - - if (base <= 10 && isEfd) - getFraction() - else { - // Checking for base == 8 is not enough, because base = 8 is set - // as soon as a 0 is read in `case '0'` of method fetchToken. - if (base == 8 && notSingleZero) syntaxError("Non-zero integral values may not have a leading zero.") - setStrVal() - if (isL) { - nextChar() - token = LONGLIT - } - else checkNoLetter() + // after int: 5e7f, 42L, 42.toDouble but not 42b. Repair 0d. + def restOfNumber(): Unit = { + ch match { + case 'e' | 'E' | 'f' | 'F' | + 'd' | 'D' => if (cbuf.isEmpty) putChar('0'); restOfNonIntegralNumber() + case 'l' | 'L' => token = LONGLIT ; setStrVal() ; nextChar() + case _ => token = INTLIT ; setStrVal() ; checkNoLetter() } } - if (base > 10 || ch != '.') - restOfUncertainToken() - else { - val lookahead = lookaheadReader - val c = lookahead.getc() - - /* Prohibit 1. */ - if (!isDigit(c)) - return setStrVal() - - val isDefinitelyNumber = (c: @switch) match { - /** Another digit is a giveaway. */ - case '0' | '1' | '2' | '3' | '4' | '5' | '6' | '7' | '8' | '9' => - true + // consume leading digits, provisionally an Int + consumeDigits(if (base == 16) 16 else 10) - /* Backquoted idents like 22.`foo`. */ - case '`' => - return setStrVal() /** Note the early return */ - - /* These letters may be part of a literal, or a method invocation on an Int. - */ - case 'd' | 'D' | 'f' | 'F' => - !isIdentifierPart(lookahead.getc()) - - /* A little more special handling for e.g. 5e7 */ - case 'e' | 'E' => - val ch = lookahead.getc() - !isIdentifierPart(ch) || (isDigit(ch) || ch == '+' || ch == '-') - - case x => - !isIdentifierStart(x) - } - if (isDefinitelyNumber) restOfNumber() - else restOfUncertainToken() - } + val detectedFloat: Boolean = base != 16 && ch == '.' && isDigit(lookaheadReader.getc) + if (detectedFloat) restOfNonIntegralNumber() else restOfNumber() } /** Parse character literal if current character is followed by \', diff --git a/test/files/neg/literals.check b/test/files/neg/literals.check new file mode 100644 index 0000000000..148a9346c5 --- /dev/null +++ b/test/files/neg/literals.check @@ -0,0 +1,40 @@ +literals.scala:6: error: missing integer number + def missingHex: Int = { 0x } // line 4: was: not reported, taken as zero + ^ +literals.scala:8: error: Decimal integer literals may not have a leading zero. (Octal syntax is obsolete.) + def leadingZeros: Int = { 01 } // line 6: no leading zero + ^ +literals.scala:10: error: Decimal integer literals may not have a leading zero. (Octal syntax is obsolete.) + def tooManyZeros: Int = { 00 } // line 8: no leading zero + ^ +literals.scala:12: error: Decimal integer literals may not have a leading zero. (Octal syntax is obsolete.) + def zeroOfNine: Int = { 09 } // line 10: no leading zero + ^ +literals.scala:16: error: Decimal integer literals may not have a leading zero. (Octal syntax is obsolete.) + def zeroOfNineDot: Int = { 09. } // line 14: malformed integer, ident expected + ^ +literals.scala:23: error: missing integer number + def missingHex: Int = 0x // line 22: was: not reported, taken as zero + ^ +literals.scala:27: error: Decimal integer literals may not have a leading zero. (Octal syntax is obsolete.) + def tooManyZeros: Int = 00 // line 26: no leading zero + ^ +literals.scala:14: error: identifier expected but '}' found. + def orphanDot: Int = { 9. } // line 12: ident expected + ^ +literals.scala:16: error: identifier expected but '}' found. + def zeroOfNineDot: Int = { 09. } // line 14: malformed integer, ident expected + ^ +literals.scala:18: error: ';' expected but double literal found. + def noHexFloat: Double = { 0x1.2 } // line 16: ';' expected but double literal found. + ^ +literals.scala:25: error: ';' expected but 'def' found. + def leadingZeros: Int = 01 // line 24: no leading zero + ^ +literals.scala:29: error: ';' expected but 'def' found. + def zeroOfNine: Int = 09 // line 28: no leading zero + ^ +literals.scala:33: error: identifier expected but 'def' found. + def zeroOfNineDot: Int = 09. // line 32: malformed integer, ident expected + ^ +13 errors found diff --git a/test/files/neg/literals.scala b/test/files/neg/literals.scala new file mode 100644 index 0000000000..3df7f0b408 --- /dev/null +++ b/test/files/neg/literals.scala @@ -0,0 +1,36 @@ + +/* This took me literally all day. +*/ +trait RejectedLiterals { + + def missingHex: Int = { 0x } // line 4: was: not reported, taken as zero + + def leadingZeros: Int = { 01 } // line 6: no leading zero + + def tooManyZeros: Int = { 00 } // line 8: no leading zero + + def zeroOfNine: Int = { 09 } // line 10: no leading zero + + def orphanDot: Int = { 9. } // line 12: ident expected + + def zeroOfNineDot: Int = { 09. } // line 14: malformed integer, ident expected + + def noHexFloat: Double = { 0x1.2 } // line 16: ';' expected but double literal found. +} + +trait Braceless { + + def missingHex: Int = 0x // line 22: was: not reported, taken as zero + + def leadingZeros: Int = 01 // line 24: no leading zero + + def tooManyZeros: Int = 00 // line 26: no leading zero + + def zeroOfNine: Int = 09 // line 28: no leading zero + + def orphanDot: Int = 9. // line 30: ident expected + + def zeroOfNineDot: Int = 09. // line 32: malformed integer, ident expected + + def noHexFloat: Double = 0x1.2 // line 34: ';' expected but double literal found. +} diff --git a/test/files/run/literals.check b/test/files/run/literals.check index 62c5fd68ae..092340eead 100644 --- a/test/files/run/literals.check +++ b/test/files/run/literals.check @@ -1,57 +1,12 @@ -warning: there were 5 deprecation warnings; re-run with -deprecation for details -test '\u0024' == '$' was successful -test '\u005f' == '_' was successful -test 65.asInstanceOf[Char] == 'A' was successful -test "\141\142" == "ab" was successful -test "\0x61\0x62".trim() == "x61\0x62" was successful - -test (65 : Byte) == 'A' was successful - -test 0X01 == 1 was successful -test 0x01 == 1 was successful -test 0x10 == 16 was successful -test 0xa == 10 was successful -test 0x0a == 10 was successful -test +0x01 == 1 was successful -test +0x10 == 16 was successful -test +0xa == 10 was successful -test +0x0a == 10 was successful -test -0x01 == -1 was successful -test -0x10 == -16 was successful -test -0xa == -10 was successful -test -0x0a == -10 was successful -test 0x7fffffff == 2147483647 was successful -test 0x80000000 == -2147483648 was successful -test 0xffffffff == -1 was successful - -test 1l == 1L was successful -test 1L == 1l was successful -test 1.asInstanceOf[Long] == 1l was successful -test 0x7fffffffffffffffL == 9223372036854775807L was successful -test 0x8000000000000000L == -9223372036854775808L was successful -test 0xffffffffffffffffL == -1L was successful - -test 1e1f == 10.0f was successful -test .3f == 0.3f was successful -test 0f == 0.0f was successful -test 01.23f == 1.23f was successful -test 3.14f == 3.14f was successful -test 6.022e23f == 6.022e23f was successful -test 09f == 9.0f was successful -test 1.asInstanceOf[Float] == 1.0 was successful -test 1l.asInstanceOf[Float] == 1.0 was successful - -test 1e1 == 10.0 was successful -test .3 == 0.3 was successful -test 0.0 == 0.0 was successful -test 0d == 0.0 was successful -test 01.23 == 1.23 was successful -test 01.23d == 1.23d was successful -test 3.14 == 3.14 was successful -test 1e-9d == 1.0e-9 was successful -test 1e137 == 1.0e137 was successful -test 1.asInstanceOf[Double] == 1.0 was successful -test 1l.asInstanceOf[Double] == 1.0 was successful - -test "".length() was successful -test ggg == 3 was successful +literals.scala:34: warning: Octal escape literals are deprecated, use \u0061 instead. + check_success("\"\\141\\142\" == \"ab\"", "\141\142", "ab") + ^ +literals.scala:34: warning: Octal escape literals are deprecated, use \u0062 instead. + check_success("\"\\141\\142\" == \"ab\"", "\141\142", "ab") + ^ +literals.scala:37: warning: Octal escape literals are deprecated, use \u0000 instead. + "\0x61\0x62".getBytes(io.Codec.UTF8.charSet) sameElements Array[Byte](0, 120, 54, 49, 0, 120, 54, 50), + ^ +literals.scala:37: warning: Octal escape literals are deprecated, use \u0000 instead. + "\0x61\0x62".getBytes(io.Codec.UTF8.charSet) sameElements Array[Byte](0, 120, 54, 49, 0, 120, 54, 50), + ^ diff --git a/test/files/run/literals.flags b/test/files/run/literals.flags new file mode 100644 index 0000000000..dcc59ebe32 --- /dev/null +++ b/test/files/run/literals.flags @@ -0,0 +1 @@ +-deprecation diff --git a/test/files/run/literals.scala b/test/files/run/literals.scala index 5f23e6b492..13fda05876 100644 --- a/test/files/run/literals.scala +++ b/test/files/run/literals.scala @@ -14,21 +14,16 @@ object Test { def \u03b1\u03b1(that: GGG) = i + that.i } - def check_success[a](name: String, closure: => a, expected: a) { - print("test " + name) - try { - val actual: a = closure - if (actual == expected) { - print(" was successful"); - } else { - print(" failed: expected "+ expected +", found "+ actual); + def check_success[A](name: String, closure: => A, expected: A) { + val res: Option[String] = + try { + val actual: A = closure + if (actual == expected) None //print(" was successful") + else Some(s" failed: expected $expected, found $actual") + } catch { + case exception: Throwable => Some(s" raised exception $exception") } - } catch { - case exception: Throwable => { - print(" raised exception " + exception); - } - } - println + for (e <- res) println(s"test $name $e") } def main(args: Array[String]) { @@ -37,15 +32,14 @@ object Test { check_success("'\\u005f' == '_'", '\u005f', '_') check_success("65.asInstanceOf[Char] == 'A'", 65.asInstanceOf[Char], 'A') check_success("\"\\141\\142\" == \"ab\"", "\141\142", "ab") - check_success("\"\\0x61\\0x62\".trim() == \"x61\\0x62\"", "\0x61\0x62".substring(1), "x61\0x62") - - println + //check_success("\"\\0x61\\0x62\".trim() == \"x61\\0x62\"", "\0x61\0x62".substring(1), "x61\0x62") + check_success(""""\0x61\0x62".getBytes == Array(0, 120, ...)""", + "\0x61\0x62".getBytes(io.Codec.UTF8.charSet) sameElements Array[Byte](0, 120, 54, 49, 0, 120, 54, 50), + true) // boolean check_success("(65 : Byte) == 'A'", (65: Byte) == 'A', true) // contrib #176 - println - // int check_success("0X01 == 1", 0X01, 1) check_success("0x01 == 1", 0x01, 1) @@ -67,8 +61,6 @@ object Test { check_success("0x80000000 == -2147483648", 0x80000000, -2147483648) check_success("0xffffffff == -1", 0xffffffff, -1) - println - // long check_success("1l == 1L", 1l, 1L) check_success("1L == 1l", 1L, 1l) @@ -81,8 +73,6 @@ object Test { check_success("0xffffffffffffffffL == -1L", 0xffffffffffffffffL, -1L) - println - // see JLS at address: // http://java.sun.com/docs/books/jls/second_edition/html/lexical.doc.html#230798 @@ -97,8 +87,6 @@ object Test { check_success("1.asInstanceOf[Float] == 1.0", 1.asInstanceOf[Float], 1.0f) check_success("1l.asInstanceOf[Float] == 1.0", 1l.asInstanceOf[Float], 1.0f) - println - // double check_success("1e1 == 10.0", 1e1, 10.0) check_success(".3 == 0.3", .3, 0.3) @@ -112,7 +100,6 @@ object Test { check_success("1.asInstanceOf[Double] == 1.0", 1.asInstanceOf[Double], 1.0) check_success("1l.asInstanceOf[Double] == 1.0", 1l.asInstanceOf[Double], 1.0) - println check_success("\"\".length()", "\u001a".length(), 1) val ggg = GGG(1) \u03b1\u03b1 GGG(2) -- cgit v1.2.3