Skip to content

Fix #8736: Handle alternatives with same signature in overloading resolution #8822

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 6 additions & 9 deletions compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import printing.Formatting
import ErrorMessageID._
import ast.Trees
import config.{Feature, ScalaVersion}
import typer.ErrorReporting.{Errors, err}
import typer.ErrorReporting.err
import typer.ProtoTypes.ViewProto
import scala.util.control.NonFatal
import StdNames.nme
Expand Down Expand Up @@ -1308,14 +1308,13 @@ object messages {

}

class AmbiguousOverload(tree: tpd.Tree, val alternatives: List[SingleDenotation], pt: Type)(
err: Errors)(
class AmbiguousOverload(tree: tpd.Tree, val alternatives: List[SingleDenotation], pt: Type, addendum: String = "")(
implicit ctx: Context)
extends ReferenceMsg(AmbiguousOverloadID) {
private def all = if (alternatives.length == 2) "both" else "all"
def msg =
s"""|Ambiguous overload. The ${err.overloadedAltsStr(alternatives)}
|$all match ${err.expectedTypeStr(pt)}""".stripMargin
em"""|Ambiguous overload. The ${err.overloadedAltsStr(alternatives)}
|$all match ${err.expectedTypeStr(pt)}$addendum""".stripMargin
def explain =
em"""|There are ${alternatives.length} methods that could be referenced as the compiler knows too little
|about the expected type.
Expand Down Expand Up @@ -1389,8 +1388,7 @@ object messages {
def explain = em"A fully applied type is expected but $tpe takes $numParams $parameters"
}

class DoesNotConformToBound(tpe: Type, which: String, bound: Type)(
err: Errors)(implicit ctx: Context)
class DoesNotConformToBound(tpe: Type, which: String, bound: Type)(implicit ctx: Context)
extends TypeMismatchMsg(DoesNotConformToBoundID) {
def msg = em"Type argument ${tpe} does not conform to $which bound $bound${err.whyNoMatchStr(tpe, bound)}"
def explain = ""
Expand Down Expand Up @@ -2141,8 +2139,7 @@ object messages {
|Refinements cannot contain overloaded definitions.""".stripMargin
}

class NoMatchingOverload(val alternatives: List[SingleDenotation], pt: Type)(
err: Errors)(using ctx: Context)
class NoMatchingOverload(val alternatives: List[SingleDenotation], pt: Type)(using ctx: Context)
extends TypeMismatchMsg(NoMatchingOverloadID) {
def msg =
em"""None of the ${err.overloadedAltsStr(alternatives)}
Expand Down
1 change: 0 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/Applications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1364,7 +1364,6 @@ trait Applications extends Compatibility {
*/
def compare(alt1: TermRef, alt2: TermRef)(using Context): Int = trace(i"compare($alt1, $alt2)", overload) {
record("compare")
assert(alt1 ne alt2)

/** Is alternative `alt1` with type `tp1` as specific as alternative
* `alt2` with type `tp2` ?
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/typer/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ object Checking {
}
for (arg, which, bound) <- ctx.boundsViolations(args, boundss, instantiate, app) do
ctx.error(
showInferred(DoesNotConformToBound(arg.tpe, which, bound)(err),
showInferred(DoesNotConformToBound(arg.tpe, which, bound),
app, tpt),
arg.sourcePos.focus)

Expand Down
22 changes: 16 additions & 6 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2796,8 +2796,9 @@ class Typer extends Namer

def adaptOverloaded(ref: TermRef) = {
val altDenots = ref.denot.alternatives
typr.println(i"adapt overloaded $ref with alternatives ${altDenots map (_.info)}%, %")
val alts = altDenots.map(TermRef(ref.prefix, ref.name, _))
typr.println(i"adapt overloaded $ref with alternatives ${altDenots map (_.info)}%\n\n %")
def altRef(alt: SingleDenotation) = TermRef(ref.prefix, ref.name, alt)
val alts = altDenots.map(altRef)
resolveOverloaded(alts, pt) match {
case alt :: Nil =>
readaptSimplified(tree.withType(alt))
Expand All @@ -2807,7 +2808,7 @@ class Typer extends Namer
// 2. If context is not an application, pick a alternative that does
// not take parameters.
def noMatches =
errorTree(tree, NoMatchingOverload(altDenots, pt)(err))
errorTree(tree, NoMatchingOverload(altDenots, pt))
def hasEmptyParams(denot: SingleDenotation) = denot.info.paramInfoss == ListOfNil
pt match {
case pt: FunOrPolyProto if !pt.isUsingApply =>
Expand All @@ -2823,11 +2824,20 @@ class Typer extends Namer
noMatches
}
}
case alts =>
case ambiAlts =>
if (tree.tpe.isErroneous || pt.isErroneous) tree.withType(UnspecifiedErrorType)
else {
val remainingDenots = alts map (_.denot.asInstanceOf[SingleDenotation])
errorTree(tree, AmbiguousOverload(tree, remainingDenots, pt)(err))
val remainingDenots = altDenots.filter(denot => ambiAlts.contains(altRef(denot)))
val addendum =
if ambiAlts.toSet.size != ambiAlts.size then
// Several variants have the same signature. This can happen for structural
// type selections. See i8736.scala
"""|
|
|Note: this happens because two or more alternatives have the same erasure,
| so they cannot be distinguished by overloading resolution""".stripMargin
else ""
errorTree(tree, AmbiguousOverload(tree, remainingDenots, pt, addendum))
}
}
}
Expand Down
23 changes: 23 additions & 0 deletions tests/neg/i8736.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
-- [E007] Type Mismatch Error: tests/neg/i8736.scala:29:29 -------------------------------------------------------------
29 | def res1: String = rec.get("k") // error: type mismatch
| ^^^^^^^^^^^^
| Found: Any
| Required: String
-- [E007] Type Mismatch Error: tests/neg/i8736.scala:30:29 -------------------------------------------------------------
30 | def res2: Int = rec.get("v") // error: type mismatch
| ^^^^^^^^^^^^
| Found: Any
| Required: Int
-- [E051] Reference Error: tests/neg/i8736.scala:31:26 -----------------------------------------------------------------
31 | def res3: Boolean = rec.get("z") // error: ambiguous
| ^^^^^^^
| Ambiguous overload. The overloaded alternatives of method (k: ("k" : String)): String with types
| (k: ("k" : String)): String
| (k: ("v" : String)): Int
| (k: ("z" : String)): Boolean
| all match arguments (("z" : String))
|
| Note: this happens because two or more alternatives have the same erasure,
| so they cannot be distinguished by overloading resolution

longer explanation available when compiling with `-explain`
36 changes: 36 additions & 0 deletions tests/neg/i8736.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import reflect.Selectable.reflectiveSelectable
object App extends App {

trait Rec0[K <: String] {
private[App] val map: Map[String, Any]
def get(k: K): Any
}
def Rec0(map0: Map[String, Any]) = new Rec0[String] {
val map = map0
def get(k: String): Any = map(k)
}

type Rec[K <: String, V0] = Rec0[K] { def get(k: K): V0 }
def field[V](s: String)(v: V): Rec[s.type, V] = Rec0(Map(s -> v)).asInstanceOf[Rec[s.type, V]]

implicit class RecOps[R <: Rec0[_]](has: R) {
def +[K1 <: String, V1](that: Rec[K1, V1]): R with Rec[K1, V1] = Rec0(has.map ++ that.map).asInstanceOf[R with Rec[K1, V1]]
}

def rec:
Rec["k", String]
with Rec["v", Int]
with Rec["z", Boolean]
= {
field("k")("Str") +
field("v")(0) +
field("z")(true)
}
def res1: String = rec.get("k") // error: type mismatch
def res2: Int = rec.get("v") // error: type mismatch
def res3: Boolean = rec.get("z") // error: ambiguous

// def res4: Boolean = rec.get("nofield")

println((res1, res2, res3))
}