Skip to content

Commit

Permalink
Merge pull request #7609 from som-snytt/issue/10662
Browse files Browse the repository at this point in the history
A foreign definition induces ambiguity
  • Loading branch information
adriaanm authored Sep 10, 2019
2 parents ead235c + c0e349a commit d602ff4
Show file tree
Hide file tree
Showing 7 changed files with 111 additions and 13 deletions.
56 changes: 43 additions & 13 deletions src/compiler/scala/tools/nsc/typechecker/Contexts.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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) {
Expand Down
5 changes: 5 additions & 0 deletions test/files/neg/t10662.check
Original file line number Diff line number Diff line change
@@ -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
6 changes: 6 additions & 0 deletions test/files/neg/t10662/pqx_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@

package p.q

class X {
override def toString() = "p.q.X"
}
22 changes: 22 additions & 0 deletions test/files/neg/t10662/px_2.scala
Original file line number Diff line number Diff line change
@@ -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
}
}
}
6 changes: 6 additions & 0 deletions test/files/neg/t10662b.check
Original file line number Diff line number Diff line change
@@ -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
6 changes: 6 additions & 0 deletions test/files/neg/t10662b/pqx_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@

package p.q

class X {
override def toString() = "p.q.X"
}
23 changes: 23 additions & 0 deletions test/files/neg/t10662b/px_2.scala
Original file line number Diff line number Diff line change
@@ -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
}
}

0 comments on commit d602ff4

Please sign in to comment.