Cleanups in Namers and AddInterfaces emerging f...

Cleanups in Namers and AddInterfaces emerging from bugfixing attempts
and comprehension pursuits. I appear to have accidentally fixed at least
one bug, as there are new (correct) warnings when building the compiler
involving permanently hidden imports. No review.

4 files changed, 148 insertions, 163 deletions

diff --git a/src/compiler/scala/tools/nsc/transform/AddInterfaces.scala b/src/compiler/scala/tools/nsc/transform/AddInterfaces.scala
index c467ed9445..6ec55b2187 100644
--- a/src/compiler/scala/tools/nsc/transform/AddInterfaces.scala
+++ b/src/compiler/scala/tools/nsc/transform/AddInterfaces.scala
@@ -15,14 +15,9 @@ abstract class AddInterfaces extends InfoTransform {
   import global._                  // the global environment
   import definitions._             // standard classes and methods
 
-  /** <p>
-   *    The phase sets <code>lateINTERFACE</code> for non-interface traits
-   *    that now become interfaces.
-   *  </p>
-   *  <p>
-   *    It sets <code>lateDEFERRED</code> for formerly concrete methods in
-   *    such traits.
-   *  </p>
+  /** The phase sets lateINTERFACE for non-interface traits that now
+   *  become interfaces. It sets lateDEFERRED for formerly concrete
+   *  methods in such traits.
    */
   override def phaseNewFlags: Long = lateDEFERRED | lateINTERFACE
 
@@ -52,23 +47,32 @@ abstract class AddInterfaces extends InfoTransform {
   }
 
   /** Is given trait member symbol a member of the trait's interface
-   *  after this transform is performed? */
-  private def isInterfaceMember(sym: Symbol): Boolean = {
-    sym.isType ||
-    { sym.info; // to set lateMETHOD flag if necessary
-      sym.isMethod &&
-      !sym.isLabel &&
-      !sym.isPrivate &&
-      (!(sym hasFlag BRIDGE) || sym.hasBridgeAnnotation) && // count @_$bridge$_ annotated classes as interface members
-      !sym.isConstructor &&
-      !sym.isImplOnly
+   *  after this transform is performed?
+   */
+  private def isInterfaceMember(sym: Symbol) = (
+    sym.isType || {
+      // !!! Shouldn't the following code be equivalent to leaving
+      // out the "sym.info" call and starting with "sym.initialize.isMethod" ?
+      // Because, it is not, which I found a little disturbing.  The compiler
+      // fails to bootstrap with an error somewhere.
+      sym.info    // initialize to set lateMETHOD flag if necessary
+
+      (     sym.isMethod
+        && !sym.isLabel
+        && !sym.isPrivate
+        && (!(sym hasFlag BRIDGE) || sym.hasBridgeAnnotation) // count @bridge annotated classes as interface members
+        && !sym.isConstructor
+        && !sym.isImplOnly
+      )
     }
-  }
+  )
 
   /** Does symbol need an implementation method? */
-  private def needsImplMethod(sym: Symbol): Boolean =
-    sym.isMethod && isInterfaceMember(sym) &&
-    (!(sym hasFlag (DEFERRED | SUPERACCESSOR)) || (sym hasFlag lateDEFERRED))
+  private def needsImplMethod(sym: Symbol) = (
+       sym.isMethod
+    && isInterfaceMember(sym)
+    && (!sym.hasFlag(DEFERRED | SUPERACCESSOR) || (sym hasFlag lateDEFERRED))
+  )
 
   def implClassPhase = currentRun.erasurePhase.next
 
@@ -99,39 +103,20 @@ abstract class AddInterfaces extends InfoTransform {
     }
   })
 
-  /** <p>
-   *    A lazy type to set the info of an implementation class
-   *    The parents of an implementation class for trait <code>iface</code> are:
-   *  </p>
-   *  <ul>
-   *    <li>superclass: <code>Object</code></li>
-   *    <li>
-   *      mixin classes: mixin classes of <code>iface</code> where every
-   *      non-interface trait is mapped to its implementation class, followed
-   *      by <code>iface</code> itself.
-   *    </li>
-   *  </ul>
-   *  <p>
-   *    The declarations of a mixin class are:
-   *  </p>
-   *  <ul>
-   *    <li>
-   *      for every interface member of <code>iface</code> its implementation
-   *      method, if one is needed.
-   *    </li>
-   *    <li>
-   *      every former member of <code>iface</code> that is implementation only
-   *    </li>
-   *  </ul>
+  /** A lazy type to set the info of an implementation class
+   *  The parents of an implementation class for trait iface are:
+   *
+   *  - superclass: Object
+   *  - mixin classes: mixin classes of iface where every non-interface
+   *  trait is mapped to its implementation class, followed by iface itself.
+   *
+   *  The declarations of a mixin class are:
+   *  - for every interface member of iface: its implementation method, if one is needed
+   *  - every former member of iface that is implementation only
    */
   private class LazyImplClassType(iface: Symbol) extends LazyType {
-
-    /** Compute the decls of implementation class <code>implClass</code>,
-     *  given the decls <code>ifaceDecls</code> of its interface.
-     *
-     *  @param implClass  ...
-     *  @param ifaceDecls ...
-     *  @return           ...
+    /** Compute the decls of implementation class implClass,
+     *  given the decls ifaceDecls of its interface.
      */
     private def implDecls(implClass: Symbol, ifaceDecls: Scope): Scope = {
       val decls = new Scope
@@ -274,9 +259,8 @@ abstract class AddInterfaces extends InfoTransform {
   }
 
   /** Add calls to supermixin constructors
-   *  <blockquote><pre>super[mix].$init$()</pre></blockquote>
-   *  to <code>tree</code>. <code>tree</code> which is assumed to be the body
-   *  of a constructor of class <code>clazz</code>.
+   *    `super[mix].$init$()`
+   *  to tree, which is assumed to be the body of a constructor of class clazz.
    */
   private def addMixinConstructorCalls(tree: Tree, clazz: Symbol): Tree = {
     def mixinConstructorCall(impl: Symbol): Tree = atPos(tree.pos) {
@@ -291,7 +275,7 @@ abstract class AddInterfaces extends InfoTransform {
       case Block(stats, expr) =>
         // needs `hasSymbol` check because `supercall` could be a block (named / default args)
         val (presuper, supercall :: rest) = stats span (t => t.hasSymbolWhich(_ hasFlag PRESUPER))
-        //assert(supercall.symbol.isClassConstructor, supercall)
+        // assert(supercall.symbol.isClassConstructor, supercall)
         treeCopy.Block(tree, presuper ::: (supercall :: mixinConstructorCalls ::: rest), expr)
     }
   }
diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala
index dde34284b7..d7b665e214 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala
@@ -152,12 +152,11 @@ trait Namers { self: Analyzer =>
     private def setInfo[Sym <: Symbol](sym : Sym)(tpe : LazyType) : Sym = sym.setInfo(tpe)
 
     private def doubleDefError(pos: Position, sym: Symbol) {
-      context.error(pos,
-        sym.name.toString() + " is already defined as " +
-        (if (sym.isSynthetic)
-          "(compiler-generated) "+ (if (sym.isModule) "case class companion " else "")
-         else "") +
-        (if (sym.isCase) "case class " + sym.name else sym.toString()))
+      val s1 = if (sym.isModule) "case class companion " else ""
+      val s2 = if (sym.isSynthetic) "(compiler-generated) " + s1 else ""
+      val s3 = if (sym.isCase) "case class " + sym.name else "" + sym
+
+      context.error(pos, sym.name + " is already defined as " + s2 + s3)
     }
 
     private def inCurrentScope(m: Symbol): Boolean = {
@@ -177,9 +176,9 @@ trait Namers { self: Analyzer =>
           doubleDefError(sym.pos, prev.sym)
           sym setInfo ErrorType
           scope unlink prev.sym // let them co-exist...
-          scope enter sym
-        } else scope enter sym
-      } else scope enter sym
+        }
+      }
+      scope enter sym
     }
 
     def enterPackageSymbol(pos: Position, pid: RefTree, pkgOwner: Symbol): Symbol = {
@@ -233,8 +232,7 @@ trait Namers { self: Analyzer =>
       // .pos, mods.flags | MODULE | FINAL, name
       var m: Symbol = context.scope.lookup(tree.name)
       val moduleFlags = tree.mods.flags | MODULE | FINAL
-      if (m.isModule && !m.isPackage && inCurrentScope(m) &&
-          (currentRun.canRedefine(m) || m.isSynthetic)) {
+      if (m.isModule && !m.isPackage && inCurrentScope(m) && (currentRun.canRedefine(m) || m.isSynthetic)) {
         updatePosFlags(m, tree.pos, moduleFlags)
         setPrivateWithin(tree, m, tree.mods)
         if (m.moduleClass != NoSymbol)
@@ -274,8 +272,7 @@ trait Namers { self: Analyzer =>
           sym setInfo sym.deSkolemize.info.substSym(tparams, tskolems) //@M the info of a skolem is the skolemized info of the actual type parameter of the skolem
         }
       }
-      tskolems foreach (_.setInfo(ltp))
-      tskolems
+      tskolems map (_ setInfo ltp)
     }
 
     /** Replace type parameters with their TypeSkolems, which can later be deskolemized to the original type param
@@ -295,13 +292,77 @@ trait Namers { self: Analyzer =>
      *  class definition tree.
      *  @return the companion object symbol.
      */
-     def ensureCompanionObject(tree: ClassDef, creator: => Tree): Symbol = {
-       val m = companionModuleOf(tree.symbol, context)
-       // @luc: not sure why "currentRun.compiles(m)" is needed, things breaks
-       // otherwise. documentation welcome.
-       if (m != NoSymbol && currentRun.compiles(m)) m
-       else enterSyntheticSym(creator)
-     }
+    def ensureCompanionObject(tree: ClassDef, creator: => Tree): Symbol = {
+      val m = companionModuleOf(tree.symbol, context)
+      // @luc: not sure why "currentRun.compiles(m)" is needed, things breaks
+      // otherwise. documentation welcome.
+      if (m != NoSymbol && currentRun.compiles(m)) m
+      else enterSyntheticSym(creator)
+    }
+
+    private def checkSelectors(tree: Import): Unit = {
+      val Import(expr, selectors) = tree
+      val base = expr.tpe
+
+      def isValid(from: Name) =
+        from.bothNames forall (x => (base nonLocalMember x) == NoSymbol)
+
+      def checkNotRedundant(pos: Position, from: Name, to0: Name) {
+        def warnRedundant(sym: Symbol) = {
+          context.unit.warning(pos,
+            "imported `"+to0+"' is permanently hidden by definition of "+sym.fullLocationString
+          )
+        }
+        def check(to: Name) = {
+          val e = context.scope.lookupEntry(to)
+          if (e != null && e.owner == context.scope && e.sym.exists)
+            warnRedundant(e.sym)
+          else if (context ne context.enclClass) {
+            val defSym = context.prefix.member(to) filter (
+              sym => sym.exists && context.isAccessible(sym, context.prefix, false))
+
+            if (defSym != NoSymbol)
+              warnRedundant(defSym)
+          }
+        }
+        if (tree.symbol.isSynthetic || expr.symbol == null || expr.symbol.isInterpreterWrapper) ()
+        else {
+          if (base.member(from) != NoSymbol)
+            check(to0)
+          if (base.member(from.toTypeName) != NoSymbol)
+            check(to0.toTypeName)
+        }
+      }
+      def checkSelector(s: ImportSelector) = {
+        val ImportSelector(from, _, to, _) = s
+        if (from != nme.WILDCARD && base != ErrorType) {
+          if (isValid(from)) {
+            if (currentRun.compileSourceFor(expr, from)) {
+              // side effecting, apparently
+              typeSig(tree)
+            }
+            // for Java code importing Scala objects
+            else if (!nme.isModuleName(from) || isValid(nme.stripModuleSuffix(from)))
+              notAMemberError(tree.pos, expr, from)
+          }
+          checkNotRedundant(tree.pos, from, to)
+        }
+      }
+      def noDuplicates(names: List[Name], message: String) {
+        def loop(xs: List[Name]): Unit = xs match {
+          case Nil      => ()
+          case hd :: tl =>
+            if (hd == nme.WILDCARD || !(tl contains hd)) loop(tl)
+            else context.error(tree.pos, hd.decode + " " + message)
+        }
+        loop(names filterNot (x => x == null || x == nme.WILDCARD))
+      }
+      selectors foreach checkSelector
+
+      // checks on the whole set
+      noDuplicates(selectors map (_.name), "is renamed twice")
+      noDuplicates(selectors map (_.rename), "appears twice as a target of a renaming")
+    }
 
     private def enterSymFinishWith(tree: Tree, tparams: List[TypeDef]) {
       val sym = tree.symbol
@@ -795,9 +856,8 @@ trait Namers { self: Analyzer =>
       // if default getters (for constructor defaults) need to be added to that module, here's the namer
       // to use. clazz is the ModuleClass. sourceModule works also for classes defined in methods.
       val module = clazz.sourceModule
-      classAndNamerOfModule get module match {
-        case Some((cdef, _)) => classAndNamerOfModule(module) = (cdef, templateNamer)
-        case None =>
+      classAndNamerOfModule get module foreach {
+        case (cdef, _) => classAndNamerOfModule(module) = (cdef, templateNamer)
       }
 
       if (opt.verbose) {
@@ -817,6 +877,7 @@ trait Namers { self: Analyzer =>
     private def methodSig(mods: Modifiers, tparams: List[TypeDef],
                           vparamss: List[List[ValDef]], tpt: Tree, rhs: Tree): Type = {
       val meth = context.owner
+      val isJavaMethod = meth.isJavaDefined
 
       // enters the skolemized version into scope, returns the deSkolemized symbols
       val tparamSyms = typer.reenterTypeParams(tparams)
@@ -829,7 +890,6 @@ trait Namers { self: Analyzer =>
         tpt setPos meth.pos.focus
       }
 
-
       /** Called for all value parameter lists, right to left
        *  @param vparams the symbols of one parameter list
        *  @param restpe  the result type (possibly a MethodType)
@@ -841,9 +901,9 @@ trait Namers { self: Analyzer =>
         // check that params only depend on ones in earlier sections, not the same. (done by checkDependencies,
         // so re-use / adapt that)
         val params = vparams map (vparam =>
-          if (meth hasFlag JAVA) vparam.setInfo(objToAny(vparam.tpe)) else vparam)
+          if (isJavaMethod) vparam.setInfo(objToAny(vparam.tpe)) else vparam)
          // TODODEPMET necessary?? new dependent types: replace symbols in restpe with the ones in vparams
-        if (meth hasFlag JAVA) JavaMethodType(params, restpe)
+        if (isJavaMethod) JavaMethodType(params, restpe)
         else MethodType(params, restpe)
       }
 
@@ -1216,69 +1276,16 @@ trait Namers { self: Analyzer =>
 
             case Import(expr, selectors) =>
               val expr1 = typer.typedQualifier(expr)
-              val base = expr1.tpe
-              typer.checkStable(expr1)
-              if ((expr1.symbol ne null) && expr1.symbol.isRootPackage) context.error(tree.pos, "_root_ cannot be imported")
-              def checkNotRedundant(pos: Position, from: Name, to: Name): Boolean = {
-                if (!tree.symbol.isSynthetic &&
-                    !((expr1.symbol ne null) && expr1.symbol.isInterpreterWrapper) &&
-                    base.member(from) != NoSymbol) {
-                  val e = context.scope.lookupEntry(to)
-                  def warnRedundant(sym: Symbol) =
-                    context.unit.warning(pos, "imported `"+to+
-                                         "' is permanently hidden by definition of "+sym+
-                                         sym.locationString)
-                  if ((e ne null) && e.owner == context.scope && e.sym.exists) {
-                    warnRedundant(e.sym); return false
-                  } else if (context eq context.enclClass) {
-                    val defSym = context.prefix.member(to) filter (
-                      sym => sym.exists && context.isAccessible(sym, context.prefix, false))
-                    if (defSym != NoSymbol) { warnRedundant(defSym); return false }
-                  }
-                }
-                true
-              }
-
-              def isValidSelector(from: Name)(fun : => Unit) {
-                if (from.bothNames forall (x => (base nonLocalMember x) == NoSymbol))
-                  fun
-              }
-
-              def checkSelectors(selectors: List[ImportSelector]): Unit = selectors match {
-                case ImportSelector(from, _, to, _) :: rest =>
-                  if (from != nme.WILDCARD && base != ErrorType) {
-                    isValidSelector(from) {
-                      if (currentRun.compileSourceFor(expr, from)) {
-                        // XXX This used to be "return typeSig(tree)" but since this method
-                        // returns Unit, that is deceptive at best.  Just in case it is side-effecting
-                        // somehow, I left the call in before the return; if you know it is
-                        // not side effecting, please delete the call.
-                        typeSig(tree)
-                        return
-                      }
-
-                      def notMember() = context.error(tree.pos, from.decode + " is not a member of " + expr)
-                      // for Java code importing Scala objects
-                      if (nme.isModuleName(from))
-                        isValidSelector(nme.stripModuleSuffix(from))(notMember())
-                      else
-                        notMember()
-                    }
-
-                    if (checkNotRedundant(tree.pos, from, to))
-                      checkNotRedundant(tree.pos, from.toTypeName, to.toTypeName)
-                  }
-                  if (from != nme.WILDCARD && (rest.exists (sel => sel.name == from)))
-                    context.error(tree.pos, from.decode + " is renamed twice")
-                  if ((to ne null) && to != nme.WILDCARD && (rest exists (sel => sel.rename == to)))
-                    context.error(tree.pos, to.decode + " appears twice as a target of a renaming")
-                  checkSelectors(rest)
-                case Nil =>
-              }
-              checkSelectors(selectors)
-              transformed(tree) = treeCopy.Import(tree, expr1, selectors)
-              expr.symbol = expr1.symbol // copy symbol and type attributes back into old expression
-                                         // so that the structure builder will find it.
+              typer checkStable expr1
+              if (expr1.symbol != null && expr1.symbol.isRootPackage)
+                context.error(tree.pos, "_root_ cannot be imported")
+
+              val newImport = treeCopy.Import(tree, expr1, selectors).asInstanceOf[Import]
+              checkSelectors(newImport)
+              transformed(tree) = newImport
+              // copy symbol and type attributes back into old expression
+              // so that the structure builder will find it.
+              expr.symbol = expr1.symbol
               expr.tpe = expr1.tpe
               ImportType(expr1)
           }
diff --git a/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala b/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala
index 81caf38762..513522e017 100644
--- a/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala
@@ -37,16 +37,10 @@ trait TypeDiagnostics {
 
   import global._
   import definitions._
-  import global.typer.infer
+  import global.typer.{ infer, context }
 
   private def currentUnit = currentRun.currentUnit
 
-  /** It can be quite difficult to know which of the many functions called "error"
-   *  is being called at any given point in the compiler.  To alleviate this I am
-   *  renaming such functions inside this trait based on where it originated.
-   */
-  def inferError(pos: Position, msg: String) = infer.error(pos, msg)
-
   /** The common situation of making sure nothing is erroneous could be
    *  nicer if Symbols, Types, and Trees all implemented some common interface
    *  in which isErroneous and similar would be placed.
@@ -85,9 +79,10 @@ trait TypeDiagnostics {
 
   /** Does the positioned line assigned to t1 precede that of t2?
    */
-  def linePrecedes(t1: Tree, t2: Tree) = t1.pos.isDefined && t1.pos.isDefined && t1.pos.line < t2.pos.line
+  def posPrecedes(p1: Position, p2: Position) = p1.isDefined && p2.isDefined && p1.line < p2.line
+  def linePrecedes(t1: Tree, t2: Tree) = posPrecedes(t1.pos, t2.pos)
 
-  def notAMember(sel: Tree, qual: Tree, name: Name) = {
+  def notAMemberMessage(pos: Position, qual: Tree, name: Name) = {
     val owner            = qual.tpe.typeSymbol
     val target           = qual.tpe.widen
     def targetKindString = if (owner.isTypeParameterOrSkolem) "type parameter " else ""
@@ -104,7 +99,7 @@ trait TypeDiagnostics {
         else ""
       }
       val semicolon = (
-        if (linePrecedes(qual, sel))
+        if (posPrecedes(qual.pos, pos))
           "\npossible cause: maybe a semicolon is missing before `"+nameString+"'?"
         else
           ""
@@ -112,14 +107,13 @@ trait TypeDiagnostics {
       companion + semicolon
     }
 
-    inferError(
-      sel.pos,
-      withAddendum(qual.pos)(
-        if (name == nme.CONSTRUCTOR) target + " does not have a constructor"
-        else nameString + " is not a member of " + targetKindString + target + addendum
-      )
+    withAddendum(qual.pos)(
+      if (name == nme.CONSTRUCTOR) target + " does not have a constructor"
+      else nameString + " is not a member of " + targetKindString + target + addendum
     )
   }
+  def notAMemberError(pos: Position, qual: Tree, name: Name) =
+    context.error(pos, notAMemberMessage(pos, qual, name))
 
   /** Only prints the parameter names if they're not synthetic,
    *  since "x$1: Int" does not offer any more information than "Int".
diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
index e0639cb6cf..d8c09cc685 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
@@ -3632,7 +3632,7 @@ trait Typers extends Modes with Adaptations {
             return makeErrorTree
 
           if (!qual.tpe.widen.isErroneous)
-            notAMember(tree, qual, name)
+            notAMemberError(tree.pos, qual, name)
 
           if (forInteractive) makeErrorTree else setError(tree)
         } else {