Merge pull request #7609 from som-snytt/issue/10662

A foreign definition induces ambiguity

Author: adriaanm

src/compiler/scala/tools/nsc/typechecker/Contexts.scala

@@ -56,6 +56,8 @@ trait Contexts { self: Analyzer =>
     LookupAmbiguous(s"it is imported twice in the same scope by\n$imp1\nand $imp2")
   def ambiguousDefnAndImport(owner: Symbol, imp: ImportInfo) =
     LookupAmbiguous(s"it is both defined in $owner and imported subsequently by \n$imp")
+  def ambiguousDefinitions(owner: Symbol, other: Symbol) =
+    LookupAmbiguous(s"it is both defined in $owner and available as ${other.fullLocationString}")
 
   private lazy val startContext = NoContext.make(
     Template(List(), noSelfType, List()) setSymbol global.NoSymbol setType global.NoType,
@@ -1410,15 +1412,17 @@ trait Contexts { self: Analyzer =>
       }
 
       // cx.scope eq null arises during FixInvalidSyms in Duplicators
-      while (defSym == NoSymbol && (cx ne NoContext) && (cx.scope ne null)) {
-        pre    = cx.enclClass.prefix
-        defSym = lookupInScope(cx.owner, cx.enclClass.prefix, cx.scope) match {
-          case NoSymbol                  => searchPrefix
-          case found                     => found
+      def nextDefinition(): Unit =
+        while (defSym == NoSymbol && (cx ne NoContext) && (cx.scope ne null)) {
+          pre    = cx.enclClass.prefix
+          defSym = lookupInScope(cx.owner, cx.enclClass.prefix, cx.scope) match {
+            case NoSymbol => searchPrefix
+            case found    => found
+          }
+          if (!defSym.exists) cx = cx.outer // push further outward
         }
-        if (!defSym.exists)
-          cx = cx.outer // push further outward
-      }
+      nextDefinition()
+
       if (symbolDepth < 0)
         symbolDepth = cx.depth
 
@@ -1458,24 +1462,50 @@ trait Contexts { self: Analyzer =>
           importCursor.advanceImp1Imp2()
       }
 
-      if (defSym.exists && impSym.exists) {
+      val preferDef: Boolean = defSym.exists && (!impSym.exists || {
         // 4) root imported symbols have same (lowest) precedence as package-owned symbols in different compilation units.
         if (imp1.depth < symbolDepth && imp1.isRootImport && foreignDefined)
-          impSym = NoSymbol
+          true
         // 4) imported symbols have higher precedence than package-owned symbols in different compilation units.
         else if (imp1.depth >= symbolDepth && foreignDefined)
-          defSym = NoSymbol
+          false
         // Defined symbols take precedence over erroneous imports.
         else if (impSym.isError || impSym.name == nme.CONSTRUCTOR)
-          impSym = NoSymbol
+          true
         // Try to reconcile them before giving up, at least if the def is not visible
         else if (foreignDefined && thisContext.reconcileAmbiguousImportAndDef(name, impSym, defSym))
-          impSym = NoSymbol
+          true
         // Otherwise they are irreconcilably ambiguous
         else
           return ambiguousDefnAndImport(defSym.alternatives.head.owner, imp1)
+      })
+
+      // If the defSym is at 4, and there is a def at 1 in scope, then the reference is ambiguous.
+      if (foreignDefined && !defSym.isPackage) {
+        val defSym0 = defSym
+        val pre0    = pre
+        val cx0     = cx
+        while ((cx ne NoContext) && cx.depth >= symbolDepth) cx = cx.outer
+        var done = false
+        while (!done) {
+          defSym = NoSymbol
+          nextDefinition()
+          done = (cx eq NoContext) || defSym.exists && !foreignDefined
+          if (!done && (cx ne NoContext)) cx = cx.outer
+        }
+        if (defSym.exists && (defSym ne defSym0)) {
+          val ambiguity =
+            if (preferDef) ambiguousDefinitions(owner = defSym.owner, defSym0)
+            else ambiguousDefnAndImport(owner = defSym.owner, imp1)
+          return ambiguity
+        }
+        defSym = defSym0
+        pre    = pre0
+        cx     = cx0
       }
 
+      if (preferDef) impSym = NoSymbol else defSym = NoSymbol
+
       // At this point only one or the other of defSym and impSym might be set.
       if (defSym.exists) finishDefSym(defSym, pre)
       else if (impSym.exists) {

test/files/neg/t10662.check

@@ -0,0 +1,5 @@
+px_2.scala:19: error: reference to X is ambiguous;
+it is both defined in package p and available as class X in package q
+      implicitly[T[X]]   // ambiguous
+                   ^
+one error found

test/files/neg/t10662/pqx_1.scala

@@ -0,0 +1,6 @@
+
+package p.q
+
+class X {
+  override def toString() = "p.q.X"
+}

test/files/neg/t10662/px_2.scala

@@ -0,0 +1,22 @@
+
+package p {
+
+  trait T[A]
+
+  class X {
+    override def toString() = "p.X"
+  }
+  object X {
+    implicit val tx: T[X] = new T[X] { }
+  }
+
+  package q {
+    //import p.X  // "permanently hidden"
+    object Test {
+      // previously, picked p.q.X
+      // This file compiles by itself;
+      // from our perspective, the other X renders our X ambiguous
+      implicitly[T[X]]   // ambiguous
+    }
+  }
+}

test/files/neg/t10662b.check

@@ -0,0 +1,6 @@
+px_2.scala:16: error: reference to X is ambiguous;
+it is both defined in package p and imported subsequently by
+import r.X
+      implicitly[T[X]]   // ambiguous
+                   ^
+one error found

test/files/neg/t10662b/pqx_1.scala

@@ -0,0 +1,6 @@
+
+package p.q
+
+class X {
+  override def toString() = "p.q.X"
+}

test/files/neg/t10662b/px_2.scala

@@ -0,0 +1,23 @@
+
+package p {
+
+  trait T[A]
+
+  class X {
+    override def toString() = "p.X"
+  }
+  object X {
+    implicit val tx: T[X] = new T[X] { }
+  }
+
+  package q {
+    import r.X
+    object Test {
+      implicitly[T[X]]   // ambiguous
+    }
+  }
+
+  package r {
+    class X
+  }
+}