No longer crash on NoSymbol.owner.

Historically calling NoSymbol.owner has crashed the compiler.
With this commit, NoSymbol owns itself. This is consistent with
the way ownership chains are handled elsewhere in the compiler
(e.g. NoContext.owner is NoContext, NoSymbol.enclClass is
NoSymbol, and so on) and frees every call site which handles
symbols from having to perform precondition tests against
NoSymbol.

Since calling NoSymbol.owner sometimes (not always) indicates
a bug which we'd like to catch sooner than later, I have
introduced a couple more methods for selected call sites.

  def owner: Symbol       // NoSymbol.owner is self, log if -Xdev
  def safeOwner: Symbol   // NoSymbol.owner is self, ignore
  def assertOwner: Symbol // NoSymbol.owner is fatal

The idea is that everyone can call sym.owner without undue anxiety
or paranoid null-like tests. When compiling under -Xdev calls to
`owner` are logged with a stack trace, so any call sites for which
that is an expected occurrence should call safeOwner instead to
communicate the intention and stay out of the log. Conversely, any
call site where crashing on the owner call was a desirable behavior
can opt into calling assertOwner.

This commit also includes all the safeOwner calls necessary to
give us a silent log when compiling scala.

6 files changed, 38 insertions, 25 deletions

diff --git a/src/reflect/scala/reflect/internal/Definitions.scala b/src/reflect/scala/reflect/internal/Definitions.scala
index 6b7aa2dddf..6715a5d1a6 100644
--- a/src/reflect/scala/reflect/internal/Definitions.scala
+++ b/src/reflect/scala/reflect/internal/Definitions.scala
@@ -227,10 +227,7 @@ trait Definitions extends api.StandardDefinitions {
       scope
     }
     /** Is this symbol a member of Object or Any? */
-    def isUniversalMember(sym: Symbol) = (
-         (sym ne NoSymbol)
-      && (ObjectClass isSubClass sym.owner)
-    )
+    def isUniversalMember(sym: Symbol) = ObjectClass isSubClass sym.owner
 
     /** Is this symbol unimportable? Unimportable symbols include:
      *  - constructors, because <init> is not a real name
diff --git a/src/reflect/scala/reflect/internal/SymbolTable.scala b/src/reflect/scala/reflect/internal/SymbolTable.scala
index c340670635..f0be0f7d05 100644
--- a/src/reflect/scala/reflect/internal/SymbolTable.scala
+++ b/src/reflect/scala/reflect/internal/SymbolTable.scala
@@ -60,6 +60,7 @@ abstract class SymbolTable extends macros.Universe
 
   def shouldLogAtThisPhase = false
   def isPastTyper = false
+  protected def isDeveloper: Boolean = settings.debug
 
   @deprecated("Give us a reason", "2.10.0")
   def abort(): Nothing = abort("unknown error")
@@ -69,8 +70,12 @@ abstract class SymbolTable extends macros.Universe
 
   /** Override with final implementation for inlining. */
   def debuglog(msg:  => String): Unit = if (settings.debug) log(msg)
-  def devWarning(msg: => String): Unit = if (settings.debug) Console.err.println(msg)
+  def devWarning(msg: => String): Unit = if (isDeveloper) Console.err.println(msg)
   def throwableAsString(t: Throwable): String = "" + t
+  def throwableAsString(t: Throwable, maxFrames: Int): String = t.getStackTrace take maxFrames mkString "\n  at "
+
+  @inline final def devWarningDumpStack(msg: => String, maxFrames: Int): Unit =
+    devWarning(msg + "\n" + throwableAsString(new Throwable, maxFrames))
 
   /** Prints a stack trace if -Ydebug or equivalent was given, otherwise does nothing. */
   def debugStack(t: Throwable): Unit  = devWarning(throwableAsString(t))
diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala
index e41038cafc..a8efa938c8 100644
--- a/src/reflect/scala/reflect/internal/Symbols.scala
+++ b/src/reflect/scala/reflect/internal/Symbols.scala
@@ -153,7 +153,10 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     }
     def asNameType(n: Name): NameType
 
-    private[this] var _rawowner = initOwner // Syncnote: need not be protected, as only assignment happens in owner_=, which is not exposed to api
+    // Syncnote: need not be protected, as only assignment happens in owner_=, which is not exposed to api
+    // The null check is for NoSymbol, which can't pass a reference to itself to the constructor and also
+    // can't call owner_= due to an assertion it contains.
+    private[this] var _rawowner = if (initOwner eq null) this else initOwner
     private[this] var _rawflags: Long = _
 
     def rawowner = _rawowner
@@ -610,7 +613,7 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     }
 
     final def isLazyAccessor       = isLazy && lazyAccessor != NoSymbol
-    final def isOverridableMember  = !(isClass || isEffectivelyFinal) && (this ne NoSymbol) && owner.isClass
+    final def isOverridableMember  = !(isClass || isEffectivelyFinal) && safeOwner.isClass
 
     /** Does this symbol denote a wrapper created by the repl? */
     final def isInterpreterWrapper = (
@@ -999,13 +1002,20 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
 
 // ------ owner attribute --------------------------------------------------------------
 
+    /** In general when seeking the owner of a symbol, one should call `owner`.
+     *  The other possibilities include:
+     *    - call `safeOwner` if it is expected that the target may be NoSymbol
+     *    - call `assertOwner` if it is an unrecoverable error if the target is NoSymbol
+     *
+     *  `owner` behaves like `safeOwner`, but logs NoSymbol.owner calls under -Xdev.
+     *  `assertOwner` aborts compilation immediately if called on NoSymbol.
+     */
     def owner: Symbol = {
       if (Statistics.hotEnabled) Statistics.incCounter(ownerCount)
       rawowner
     }
-
-    // Like owner, but NoSymbol.owner == NoSymbol instead of throwing an exception.
-    final def safeOwner: Symbol = if (this eq NoSymbol) NoSymbol else owner
+    final def safeOwner: Symbol   = if (this eq NoSymbol) NoSymbol else owner
+    final def assertOwner: Symbol = if (this eq NoSymbol) abort("no-symbol does not have an owner") else owner
 
     // TODO - don't allow the owner to be changed without checking invariants, at least
     // when under some flag. Define per-phase invariants for owner/owned relationships,
@@ -1781,10 +1791,6 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
       result
     }
 
-    @inline final def map(f: Symbol => Symbol): Symbol = if (this eq NoSymbol) this else f(this)
-
-    final def toOption: Option[Symbol] = if (exists) Some(this) else None
-
 // ------ cloneing -------------------------------------------------------------------
 
     /** A clone of this symbol. */
@@ -2179,8 +2185,7 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
      *  the recursive knot.
      */
     private def canMatchInheritedSymbols = (
-         (this ne NoSymbol)
-      && owner.isClass
+         owner.isClass
       && !this.isClass
       && !this.isConstructor
     )
@@ -2352,6 +2357,11 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
 
     @inline final def orElse(alt: => Symbol): Symbol = if (this ne NoSymbol) this else alt
     @inline final def andAlso(f: Symbol => Unit): Symbol = { if (this ne NoSymbol) f(this) ; this }
+    @inline final def fold[T](none: => T)(f: Symbol => T): T = if (this ne NoSymbol) f(this) else none
+    @inline final def map(f: Symbol => Symbol): Symbol = if (this eq NoSymbol) this else f(this)
+
+    final def toOption: Option[Symbol] = if (exists) Some(this) else None
+
 
 // ------ toString -------------------------------------------------------------------
 
@@ -3340,7 +3350,11 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     override def enclosingPackageClass: Symbol = this
     override def enclMethod: Symbol = this
     override def associatedFile = NoAbstractFile
-    override def ownerChain: List[Symbol] = List()
+    override def owner: Symbol = {
+      devWarningDumpStack("NoSymbol.owner", 15)
+      this
+    }
+    override def ownerChain: List[Symbol] = Nil
     override def ownersIterator: Iterator[Symbol] = Iterator.empty
     override def alternatives: List[Symbol] = List()
     override def reset(completer: Type): this.type = this
@@ -3350,9 +3364,6 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     override def accessBoundary(base: Symbol): Symbol = enclosingRootClass
     def cloneSymbolImpl(owner: Symbol, newFlags: Long) = abort("NoSymbol.clone()")
     override def originalEnclosingMethod = this
-
-    override def owner: Symbol =
-      abort("no-symbol does not have an owner")
   }
 
   protected def makeNoSymbol: NoSymbol = new NoSymbol
diff --git a/src/reflect/scala/reflect/internal/Trees.scala b/src/reflect/scala/reflect/internal/Trees.scala
index 02bee5e369..a5a78cb3b8 100644
--- a/src/reflect/scala/reflect/internal/Trees.scala
+++ b/src/reflect/scala/reflect/internal/Trees.scala
@@ -185,8 +185,8 @@ trait Trees extends api.Trees { self: SymbolTable =>
     def replace(from: Tree, to: Tree): Tree =
       new TreeReplacer(from, to, positionAware = false) transform this
 
-    def hasSymbolWhich(f: Symbol => Boolean) =
-      (symbol ne null) && (symbol ne NoSymbol) && f(symbol)
+    def hasExistingSymbol = (symbol ne null) && (symbol ne NoSymbol)
+    def hasSymbolWhich(f: Symbol => Boolean) = hasExistingSymbol && f(symbol)
 
     def isErroneous = (tpe ne null) && tpe.isErroneous
     def isTyped     = (tpe ne null) && !tpe.isErroneous
diff --git a/src/reflect/scala/reflect/internal/Types.scala b/src/reflect/scala/reflect/internal/Types.scala
index 71f46fedb7..cab2303dd0 100644
--- a/src/reflect/scala/reflect/internal/Types.scala
+++ b/src/reflect/scala/reflect/internal/Types.scala
@@ -3475,7 +3475,7 @@ trait Types
   def copyRefinedType(original: RefinedType, parents: List[Type], decls: Scope) =
     if ((parents eq original.parents) && (decls eq original.decls)) original
     else {
-      val owner = if (original.typeSymbol == NoSymbol) NoSymbol else original.typeSymbol.owner
+      val owner = original.typeSymbol.owner
       val result = refinedType(parents, owner)
       val syms1 = decls.toList
       for (sym <- syms1)
diff --git a/src/reflect/scala/reflect/internal/tpe/TypeMaps.scala b/src/reflect/scala/reflect/internal/tpe/TypeMaps.scala
index bebc419c7c..6662ec522a 100644
--- a/src/reflect/scala/reflect/internal/tpe/TypeMaps.scala
+++ b/src/reflect/scala/reflect/internal/tpe/TypeMaps.scala
@@ -524,7 +524,7 @@ private[internal] trait TypeMaps {
     private def correspondingTypeArgument(lhs: Type, rhs: Type): Type = {
       val TypeRef(_, lhsSym, lhsArgs) = lhs
       val TypeRef(_, rhsSym, rhsArgs) = rhs
-      require(lhsSym.safeOwner == rhsSym, s"$lhsSym is not a type parameter of $rhsSym")
+      require(lhsSym.owner == rhsSym, s"$lhsSym is not a type parameter of $rhsSym")
 
       // Find the type parameter position; we'll use the corresponding argument.
       // Why are we checking by name rather than by equality? Because for
@@ -539,7 +539,7 @@ private[internal] trait TypeMaps {
       else {
         // It's easy to get here when working on hardcore type machinery (not to
         // mention when not doing so, see above) so let's provide a standout error.
-        def own_s(s: Symbol) = s.nameString + " in " + s.safeOwner.nameString
+        def own_s(s: Symbol) = s.nameString + " in " + s.owner.nameString
         def explain =
           sm"""|   sought  ${own_s(lhsSym)}
                | classSym  ${own_s(rhsSym)}