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.

12 files changed, 42 insertions, 99 deletions

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
+    }
+  }
+}