Adress reviewers comments

Major changes

 - Drop -Yupdate-stale and simplify bringForward accordingly
 - Make designator a private field of TypeRef and TermRef

Author: odersky

compiler/src/dotty/tools/dotc/config/ScalaSettings.scala

@@ -106,7 +106,6 @@ class ScalaSettings extends Settings.SettingGroup {
   val YkeepComments = BooleanSetting("-Ykeep-comments", "Keep comments when scanning source files.")
   val YforceSbtPhases = BooleanSetting("-Yforce-sbt-phases", "Run the phases used by sbt for incremental compilation (ExtractDependencies and ExtractAPI) even if the compiler is ran outside of sbt, for debugging.")
   val YdumpSbtInc = BooleanSetting("-Ydump-sbt-inc", "For every compiled foo.scala, output the API representation and dependencies used for sbt incremental compilation in foo.inc, implies -Yforce-sbt-phases.")
-  val YupdateStale = BooleanSetting("-Yupdate-stale", "Automically update stale top-level classes and objects to latest version")
   val YcheckAllPatmat = BooleanSetting("-Ycheck-all-patmat", "Check exhaustivity and redundancy of all pattern matching (used for testing the algorithm)")
   val YretainTrees = BooleanSetting("-Yretain-trees", "Retain trees for top-level classes, accessible from ClassSymbol#tree")
   val YshowTreeIds = BooleanSetting("-Yshow-tree-ids", "Uniquely tag all tree nodes in debugging output.")

compiler/src/dotty/tools/dotc/core/Denotations.scala

@@ -776,9 +776,7 @@ object Denotations {
      *  if denotation is no longer valid.
      *  However, StaleSymbol error is not thrown in the following situations:
      *
-     *   - If the symbol is a toplevel class or object and -Yupdate-stale is set.
-     *     update the denotation to the new symbol with the same name instead.
-     *   - If ctx.acceptStale returns true (because we are in the IDE),
+     *   - If ctx.acceptStale returns true (e.g. because we are in the IDE),
      *     update the symbol to the new version if it exists, or return
      *     the old version otherwise.
      *   - If the symbol did not have a denotation that was defined at the current phase
@@ -788,11 +786,9 @@ object Denotations {
       this match {
         case symd: SymDenotation =>
           if (ctx.stillValid(symd)) return updateValidity()
-          lazy val staleOK = ctx.acceptStale(symd)
-          if (ctx.settings.YupdateStale.value && symd.owner.is(Package) || staleOK) {
+          if (ctx.acceptStale(symd)) {
             val newd = symd.owner.info.decls.lookup(symd.name)
-            if (newd.exists) return (newd.denot: SingleDenotation).updateValidity()
-            else if (staleOK) return updateValidity()
+            return (newd.denot: SingleDenotation).orElse(symd).updateValidity()
           }
         case _ =>
       }

compiler/src/dotty/tools/dotc/core/SymDenotations.scala

@@ -102,8 +102,11 @@ trait SymDenotations { this: Context =>
   }
 
   /** Configurable: Accept stale symbol with warning if in IDE */
+  def staleOK = Config.ignoreStaleInIDE && mode.is(Mode.Interactive)
+
+  /** Possibly accept stale symbol with warning if in IDE */
   def acceptStale(denot: SingleDenotation): Boolean =
-    mode.is(Mode.Interactive) && Config.ignoreStaleInIDE && {
+    staleOK && {
       ctx.echo(denot.staleSymbolMsg)
       true
     }

compiler/src/dotty/tools/dotc/core/Symbols.scala

@@ -451,8 +451,8 @@ object Symbols {
     final def isValidInCurrentRun(implicit ctx: Context): Boolean =
       (lastDenot.validFor.runId == ctx.runId || ctx.stillValid(lastDenot)) &&
       (lastDenot.symbol eq this)
-        // the last condition is needed because under option -Yupdate-stale overwritten
-        // package members keep denotations pointing to the new symbol, so the validity
+        // the last condition is needed because under ctx.staleOK overwritten
+        // members keep denotations pointing to the new symbol, so the validity
         // periods check out OK. But once a package member is overridden it is not longer
         // valid. If the option would be removed, the check would be no longer needed.
 

compiler/src/dotty/tools/dotc/core/Types.scala

@@ -1695,7 +1695,8 @@ object Types {
 
     private def memberDenot(name: Name, allowPrivate: Boolean)(implicit ctx: Context): Denotation = {
       var d = memberDenot(prefix, name, allowPrivate)
-      if (!d.exists && ctx.mode.is(Mode.Interactive))
+      if (!d.exists && !allowPrivate && ctx.mode.is(Mode.Interactive))
+        // In the IDE we might change a public symbol to private, and would still expect to find it.
         d = memberDenot(prefix, name, true)
       if (!d.exists && ctx.phaseId > FirstPhaseId && lastDenotation.isInstanceOf[SymDenotation])
         // name has changed; try load in earlier phase and make current
@@ -1900,7 +1901,10 @@ object Types {
       if ((designator ne sym) && sym.exists) NamedType(prefix, sym).asInstanceOf[ThisType]
       else this
 
-    /** A reference like this one, but with the given denotation, if it exists */
+    /** A reference like this one, but with the given denotation, if it exists.
+     *  If the symbol of `denot` is the same as the current symbol, the denotation
+     *  is re-used, otherwise a new one is created.
+     */
     final def withDenot(denot: Denotation)(implicit ctx: Context): ThisType =
       if (denot.exists) {
         val adapted = withSym(denot.symbol)
@@ -1965,12 +1969,16 @@ object Types {
     def underlyingRef: TermRef
   }
 
-  abstract case class TermRef(override val prefix: Type, var designator: Designator)
-  extends NamedType with SingletonType with ImplicitRef {
+  abstract case class TermRef(override val prefix: Type,
+                              private var myDesignator: Designator)
+    extends NamedType with SingletonType with ImplicitRef {
 
     type ThisType = TermRef
     type ThisName = TermName
 
+    override def designator = myDesignator
+    override protected def designator_=(d: Designator) = myDesignator = d
+
     //assert(name.toString != "<local Coder>")
     override def underlying(implicit ctx: Context): Type = {
       val d = denot
@@ -1989,11 +1997,16 @@ object Types {
     def underlyingRef = this
   }
 
-  abstract case class TypeRef(override val prefix: Type, var designator: Designator) extends NamedType {
+  abstract case class TypeRef(override val prefix: Type,
+                              private var myDesignator: Designator)
+    extends NamedType {
 
     type ThisType = TypeRef
     type ThisName = TypeName
 
+    override def designator = myDesignator
+    override protected def designator_=(d: Designator) = myDesignator = d
+
     override def underlying(implicit ctx: Context): Type = info
   }