From 1dea9916e686adc96df9d7886346af2ed1abe45f Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 31 Oct 2016 17:33:00 +0100 Subject: Fix #1637: Future defs are always OK Drop special mode that handles future defs without which we get DenotationNotDefinedHere errors. In more than a year, this has only turned up false negatives. So I think it's better to drop the check, and the contortions needed to deal with it. --- .../tools/backend/jvm/DottyBackendInterface.scala | 21 +++++-------- src/dotty/tools/dotc/ast/tpd.scala | 13 ++++---- src/dotty/tools/dotc/core/Definitions.scala | 12 +++----- src/dotty/tools/dotc/core/Denotations.scala | 36 ++++------------------ src/dotty/tools/dotc/core/Mode.scala | 12 -------- src/dotty/tools/dotc/core/SymDenotations.scala | 20 +++--------- src/dotty/tools/dotc/core/TypeErasure.scala | 2 +- src/dotty/tools/dotc/core/Types.scala | 5 +-- src/dotty/tools/dotc/printing/Formatting.scala | 2 +- .../tools/dotc/transform/PatternMatcher.scala | 3 +- src/dotty/tools/dotc/transform/TreeChecker.scala | 7 ++--- tests/pos/i1637.scala | 8 +++++ 12 files changed, 42 insertions(+), 99 deletions(-) create mode 100644 tests/pos/i1637.scala diff --git a/src/dotty/tools/backend/jvm/DottyBackendInterface.scala b/src/dotty/tools/backend/jvm/DottyBackendInterface.scala index 2d60d851c..03c4315fe 100644 --- a/src/dotty/tools/backend/jvm/DottyBackendInterface.scala +++ b/src/dotty/tools/backend/jvm/DottyBackendInterface.scala @@ -647,21 +647,16 @@ class DottyBackendInterface(outputDirectory: AbstractFile, val superCallsMap: Ma def rawowner: Symbol = { originalOwner } - def originalOwner: Symbol = { + def originalOwner: Symbol = // used to populate the EnclosingMethod attribute. // it is very tricky in presence of classes(and annonymous classes) defined inside supper calls. - try { - if (sym.exists) { - val original = toDenot(sym).initial - val validity = original.validFor - val shiftedContext = ctx.withPhase(validity.phaseId) - val r = toDenot(sym)(shiftedContext).maybeOwner.lexicallyEnclosingClass(shiftedContext) - r - } else NoSymbol - } catch { - case e: NotDefinedHere => NoSymbol // todo: do we have a method to tests this? - } - } + if (sym.exists) { + val original = toDenot(sym).initial + val validity = original.validFor + val shiftedContext = ctx.withPhase(validity.phaseId) + val r = toDenot(sym)(shiftedContext).maybeOwner.lexicallyEnclosingClass(shiftedContext) + r + } else NoSymbol def parentSymbols: List[Symbol] = toDenot(sym).info.parents.map(_.typeSymbol) def superClass: Symbol = { val t = toDenot(sym).asClass.superClass diff --git a/src/dotty/tools/dotc/ast/tpd.scala b/src/dotty/tools/dotc/ast/tpd.scala index d8db3306c..77fa88213 100644 --- a/src/dotty/tools/dotc/ast/tpd.scala +++ b/src/dotty/tools/dotc/ast/tpd.scala @@ -295,7 +295,7 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { // ------ Making references ------------------------------------------------------ def prefixIsElidable(tp: NamedType)(implicit ctx: Context) = { - def test(implicit ctx: Context) = tp.prefix match { + val typeIsElidable = tp.prefix match { case NoPrefix => true case pre: ThisType => @@ -309,10 +309,9 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { case _ => false } - try test || tp.symbol.is(JavaStatic) || tp.symbol.hasAnnotation(defn.ScalaStaticAnnot) - catch { // See remark in SymDenotations#accessWithin - case ex: NotDefinedHere => test(ctx.addMode(Mode.FutureDefsOK)) - } + typeIsElidable || + tp.symbol.is(JavaStatic) || + tp.symbol.hasAnnotation(defn.ScalaStaticAnnot) } def needsSelect(tp: Type)(implicit ctx: Context) = tp match { @@ -605,7 +604,7 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { loop(from.owner, from :: froms, to :: tos) else { //println(i"change owner ${from :: froms}%, % ==> $tos of $tree") - new TreeTypeMap(oldOwners = from :: froms, newOwners = tos)(ctx.addMode(Mode.FutureDefsOK)).apply(tree) + new TreeTypeMap(oldOwners = from :: froms, newOwners = tos).apply(tree) } } loop(from, Nil, to :: Nil) @@ -631,7 +630,7 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { traverseChildren(tree) } } - traverser.traverse(tree)(ctx.addMode(Mode.FutureDefsOK)) + traverser.traverse(tree) tree } diff --git a/src/dotty/tools/dotc/core/Definitions.scala b/src/dotty/tools/dotc/core/Definitions.scala index 541d66306..62fa2d07d 100644 --- a/src/dotty/tools/dotc/core/Definitions.scala +++ b/src/dotty/tools/dotc/core/Definitions.scala @@ -633,14 +633,10 @@ class Definitions { name.startsWith(prefix) && name.drop(prefix.length).forall(_.isDigit) } - def isBottomClass(cls: Symbol) = cls == NothingClass || cls == NullClass - def isBottomType(tp: Type) = { - def test(implicit ctx: Context) = tp.derivesFrom(NothingClass) || tp.derivesFrom(NullClass) - try test - catch { // See remark in SymDenotations#accessWithin - case ex: NotDefinedHere => test(ctx.addMode(Mode.FutureDefsOK)) - } - } + def isBottomClass(cls: Symbol) = + cls == NothingClass || cls == NullClass + def isBottomType(tp: Type) = + tp.derivesFrom(NothingClass) || tp.derivesFrom(NullClass) def isFunctionClass(cls: Symbol) = isVarArityClass(cls, tpnme.Function) def isAbstractFunctionClass(cls: Symbol) = isVarArityClass(cls, tpnme.AbstractFunction) diff --git a/src/dotty/tools/dotc/core/Denotations.scala b/src/dotty/tools/dotc/core/Denotations.scala index 7866d6697..6a39c5787 100644 --- a/src/dotty/tools/dotc/core/Denotations.scala +++ b/src/dotty/tools/dotc/core/Denotations.scala @@ -2,7 +2,7 @@ package dotty.tools package dotc package core -import SymDenotations.{ SymDenotation, ClassDenotation, NoDenotation, NotDefinedHereDenotation } +import SymDenotations.{ SymDenotation, ClassDenotation, NoDenotation } import Contexts.{Context, ContextBase} import Names.{Name, PreName} import Names.TypeName @@ -131,16 +131,9 @@ object Denotations { */ def atSignature(sig: Signature, site: Type = NoPrefix, relaxed: Boolean = false)(implicit ctx: Context): Denotation - /** The variant of this denotation that's current in the given context, or - * `NotDefinedHereDenotation` if this denotation does not exist at current phase, but - * is defined elsewhere in this run. - */ - def currentIfExists(implicit ctx: Context): Denotation - /** The variant of this denotation that's current in the given context. - * If no such denotation exists: If Mode.FutureDefs is set, the - * denotation with each alternative at its first point of definition, - * otherwise a `NotDefinedHere` exception is thrown. + * If no such denotation exists, returns the denotation with each alternative + * at its first point of definition. */ def current(implicit ctx: Context): Denotation @@ -569,8 +562,6 @@ object Denotations { final def signature(implicit ctx: Context) = Signature.OverloadedSignature def atSignature(sig: Signature, site: Type, relaxed: Boolean)(implicit ctx: Context): Denotation = derivedMultiDenotation(denot1.atSignature(sig, site, relaxed), denot2.atSignature(sig, site, relaxed)) - def currentIfExists(implicit ctx: Context): Denotation = - derivedMultiDenotation(denot1.currentIfExists, denot2.currentIfExists) def current(implicit ctx: Context): Denotation = derivedMultiDenotation(denot1.current, denot2.current) def altsWith(p: Symbol => Boolean): List[SingleDenotation] = @@ -765,7 +756,7 @@ object Denotations { * is brought forward to be valid in the new runId. Otherwise * the symbol is stale, which constitutes an internal error. */ - def currentIfExists(implicit ctx: Context): SingleDenotation = { + def current(implicit ctx: Context): SingleDenotation = { val currentPeriod = ctx.period val valid = myValidFor if (valid.code <= 0) { @@ -839,24 +830,14 @@ object Denotations { // performance: Test setup: Compile everything in dotc and immediate subdirectories // 10 times. Best out of 10: 18154ms with `prev` field, 17777ms without. cnt += 1 - if (cnt > MaxPossiblePhaseId) return NotDefinedHereDenotation + if (cnt > MaxPossiblePhaseId) + return current(ctx.withPhase(coveredInterval.firstPhaseId)) } cur } } } - def current(implicit ctx: Context): SingleDenotation = { - val d = currentIfExists - if (d ne NotDefinedHereDenotation) d else currentNoDefinedHere - } - - private def currentNoDefinedHere(implicit ctx: Context): SingleDenotation = - if (ctx.mode is Mode.FutureDefsOK) - current(ctx.withPhase(coveredInterval.firstPhaseId)) - else - throw new NotDefinedHere(demandOutsideDefinedMsg) - private def demandOutsideDefinedMsg(implicit ctx: Context): String = s"demanding denotation of $this at phase ${ctx.phase}(${ctx.phaseId}) outside defined interval: defined periods are${definedPeriodsString}" @@ -1233,9 +1214,4 @@ object Denotations { util.Stats.record("stale symbol") override def getMessage() = msg } - - class NotDefinedHere(msg: => String) extends Exception { - util.Stats.record("not defined here") - override def getMessage() = msg - } } \ No newline at end of file diff --git a/src/dotty/tools/dotc/core/Mode.scala b/src/dotty/tools/dotc/core/Mode.scala index 7a9bb0572..c16c3f15b 100644 --- a/src/dotty/tools/dotc/core/Mode.scala +++ b/src/dotty/tools/dotc/core/Mode.scala @@ -41,18 +41,6 @@ object Mode { val InSuperCall = newMode(6, "InSuperCall") - /** This mode bit is set if we want to allow accessing a symbol's denotation - * at a period before that symbol is first valid. An example where this is - * the case is if we want to examine the environment where an access is made. - * The computation might take place at an earlier phase (e.g. it is part of - * some completion such as unpickling), but the environment might contain - * synbols that are not yet defined in that phase. - * If the mode bit is set, getting the denotation of a symbol at a phase - * before the symbol is defined will return the symbol's denotation at the - * first phase where it is valid, instead of throwing a NotDefinedHere error. - */ - val FutureDefsOK = newMode(7, "FutureDefsOK") - /** Allow GADTFlexType labelled types to have their bounds adjusted */ val GADTflexible = newMode(8, "GADTflexible") diff --git a/src/dotty/tools/dotc/core/SymDenotations.scala b/src/dotty/tools/dotc/core/SymDenotations.scala index a98d6732a..a25be0a1f 100644 --- a/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/src/dotty/tools/dotc/core/SymDenotations.scala @@ -645,21 +645,10 @@ object SymDenotations { final def isAccessibleFrom(pre: Type, superAccess: Boolean = false, whyNot: StringBuffer = null)(implicit ctx: Context): Boolean = { /** Are we inside definition of `boundary`? */ - def accessWithin(boundary: Symbol) = { - def test(implicit ctx: Context) = - ctx.owner.isContainedIn(boundary) && - (!(this is JavaDefined) || // disregard package nesting for Java - ctx.owner.enclosingPackageClass == boundary.enclosingPackageClass) - try test - catch { - // It might be we are in a definition whose symbol is not defined at the - // period where the test is made. Retry with FutureDefsOK. The reason - // for not doing this outright is speed. We would like to avoid - // creating a new context object each time we call accessWithin. - // Note that the exception should be thrown only infrequently. - case ex: NotDefinedHere => test(ctx.addMode(Mode.FutureDefsOK)) - } - } + def accessWithin(boundary: Symbol) = + ctx.owner.isContainedIn(boundary) && + (!(this is JavaDefined) || // disregard package nesting for Java + ctx.owner.enclosingPackageClass == boundary.enclosingPackageClass) /** Are we within definition of linked class of `boundary`? */ def accessWithinLinked(boundary: Symbol) = { @@ -1872,7 +1861,6 @@ object SymDenotations { } @sharable val NoDenotation = new NoDenotation - @sharable val NotDefinedHereDenotation = new NoDenotation // ---- Completion -------------------------------------------------------- diff --git a/src/dotty/tools/dotc/core/TypeErasure.scala b/src/dotty/tools/dotc/core/TypeErasure.scala index 254ea3277..abbacee49 100644 --- a/src/dotty/tools/dotc/core/TypeErasure.scala +++ b/src/dotty/tools/dotc/core/TypeErasure.scala @@ -107,7 +107,7 @@ object TypeErasure { /** The current context with a phase no later than erasure */ private def erasureCtx(implicit ctx: Context) = - if (ctx.erasedTypes) ctx.withPhase(ctx.erasurePhase).addMode(Mode.FutureDefsOK) else ctx + if (ctx.erasedTypes) ctx.withPhase(ctx.erasurePhase) else ctx /** The standard erasure of a Scala type. Value classes are erased as normal classes. * diff --git a/src/dotty/tools/dotc/core/Types.scala b/src/dotty/tools/dotc/core/Types.scala index 38913a7d0..35640d910 100644 --- a/src/dotty/tools/dotc/core/Types.scala +++ b/src/dotty/tools/dotc/core/Types.scala @@ -1441,10 +1441,7 @@ object Types { } case d => if (d.validFor.runId != ctx.period.runId) loadDenot - else { - val newd = d.currentIfExists - if (newd ne NotDefinedHereDenotation) newd else loadDenot - } + else d.current } if (ctx.typerState.ephemeral) record("ephemeral cache miss: loadDenot") else if (d.exists) { diff --git a/src/dotty/tools/dotc/printing/Formatting.scala b/src/dotty/tools/dotc/printing/Formatting.scala index e7968b14a..b321d3736 100644 --- a/src/dotty/tools/dotc/printing/Formatting.scala +++ b/src/dotty/tools/dotc/printing/Formatting.scala @@ -29,7 +29,7 @@ object Formatting { protected def showArg(arg: Any)(implicit ctx: Context): String = arg match { case arg: Showable => - try arg.show(ctx.addMode(Mode.FutureDefsOK)) + try arg.show catch { case NonFatal(ex) => s"[cannot display due to $ex, raw string = $toString]" } diff --git a/src/dotty/tools/dotc/transform/PatternMatcher.scala b/src/dotty/tools/dotc/transform/PatternMatcher.scala index 8636d5084..3e25cf82e 100644 --- a/src/dotty/tools/dotc/transform/PatternMatcher.scala +++ b/src/dotty/tools/dotc/transform/PatternMatcher.scala @@ -604,9 +604,8 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer { // all potentially stored subpat binders val potentiallyStoredBinders = stored.unzip._1.toSet // compute intersection of all symbols in the tree `in` and all potentially stored subpat binders - def computeBinders(implicit ctx: Context) = new DeepFolder[Unit]((x: Unit, t:Tree) => + new DeepFolder[Unit]((x: Unit, t: Tree) => if (potentiallyStoredBinders(t.symbol)) usedBinders += t.symbol).apply((), in) - computeBinders(ctx.addMode(Mode.FutureDefsOK)) // trigged a NotDefinedHere on $outer when compiler dotc/printing if (usedBinders.isEmpty) in else { diff --git a/src/dotty/tools/dotc/transform/TreeChecker.scala b/src/dotty/tools/dotc/transform/TreeChecker.scala index 808178369..4a09d2fef 100644 --- a/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -69,11 +69,8 @@ class TreeChecker extends Phase with SymTransformer { def checkCompanion(symd: SymDenotation)(implicit ctx: Context): Unit = { val cur = symd.linkedClass - val prev = ctx.atPhase(ctx.phase.prev) { - ct => { - implicit val ctx: Context = ct.addMode(Mode.FutureDefsOK) - symd.symbol.linkedClass - } + val prev = ctx.atPhase(ctx.phase.prev) { implicit ctx => + symd.symbol.linkedClass } if (prev.exists) diff --git a/tests/pos/i1637.scala b/tests/pos/i1637.scala new file mode 100644 index 000000000..fa7defb74 --- /dev/null +++ b/tests/pos/i1637.scala @@ -0,0 +1,8 @@ +object Main extends App { + case class Foo(field: Option[String]) + val x: PartialFunction[Foo, Int] = { c => + c.field match { + case Some(s) => 42 + } + } +} -- cgit v1.2.3