Skip to content

Fix #1639: Changes around implicits and apply methods #1658

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
Dec 3, 2016
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
9 changes: 7 additions & 2 deletions compiler/src/dotty/tools/dotc/reporting/Reporter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -286,11 +286,16 @@ abstract class Reporter extends interfaces.ReporterResult {
}

/** Should this diagnostic not be reported at all? */
def isHidden(m: MessageContainer)(implicit ctx: Context): Boolean = ctx.mode.is(Mode.Printing)
def isHidden(m: MessageContainer)(implicit ctx: Context): Boolean =
ctx.mode.is(Mode.Printing)

/** Does this reporter contain not yet reported errors or warnings? */
def hasPending: Boolean = false

/** If this reporter buffers messages, remove and return all buffered messages. */
def removeBufferedMessages(implicit ctx: Context): List[MessageContainer] = Nil

/** Issue all error messages in this reporter to next outer one, or make sure they are written. */
def flush()(implicit ctx: Context): Unit = {}
def flush()(implicit ctx: Context): Unit =
removeBufferedMessages.foreach(ctx.reporter.report)
}
8 changes: 3 additions & 5 deletions compiler/src/dotty/tools/dotc/reporting/StoreReporter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,9 @@ class StoreReporter(outer: Reporter) extends Reporter {
}
}

override def flush()(implicit ctx: Context) =
if (infos != null) {
infos.foreach(ctx.reporter.report(_))
infos = null
}
override def removeBufferedMessages(implicit ctx: Context): List[MessageContainer] =
if (infos != null) try infos.toList finally infos = null
else Nil

override def errorsReported = hasErrors || outer.errorsReported
}
104 changes: 66 additions & 38 deletions compiler/src/dotty/tools/dotc/typer/Implicits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import Constants._
import Applications._
import ProtoTypes._
import ErrorReporting._
import reporting.diagnostic.MessageContainer
import Inferencing.fullyDefinedType
import Trees._
import Hashable._
Expand Down Expand Up @@ -212,6 +213,8 @@ object Implicits {
/** A "no matching implicit found" failure */
case object NoImplicitMatches extends SearchFailure

case object DivergingImplicit extends SearchFailure

/** A search failure that can show information about the cause */
abstract class ExplainedSearchFailure extends SearchFailure {
protected def pt: Type
Expand All @@ -233,9 +236,35 @@ object Implicits {
"\n " + explanation
}

class NonMatchingImplicit(ref: TermRef, val pt: Type, val argument: tpd.Tree) extends ExplainedSearchFailure {
def explanation(implicit ctx: Context): String =
em"${err.refStr(ref)} does not $qualify"
class NonMatchingImplicit(ref: TermRef,
val pt: Type,
val argument: tpd.Tree,
trail: List[MessageContainer]) extends ExplainedSearchFailure {
private val separator = "\n**** because ****\n"

/** Replace repeated parts beginning with `separator` by ... */
private def elideRepeated(str: String): String = {
val startIdx = str.indexOfSlice(separator)
val nextIdx = str.indexOfSlice(separator, startIdx + separator.length)
if (nextIdx < 0) str
else {
val prefix = str.take(startIdx)
val first = str.slice(startIdx, nextIdx)
var rest = str.drop(nextIdx)
if (rest.startsWith(first)) {
rest = rest.drop(first.length)
val dots = "\n\n ...\n"
if (!rest.startsWith(dots)) rest = dots ++ rest
}
prefix ++ first ++ rest
}
}

def explanation(implicit ctx: Context): String = {
val headMsg = em"${err.refStr(ref)} does not $qualify"
val trailMsg = trail.map(mc => i"$separator ${mc.message}").mkString
elideRepeated(headMsg ++ trailMsg)
}
}

class ShadowedImplicit(ref: TermRef, shadowing: Type, val pt: Type, val argument: tpd.Tree) extends ExplainedSearchFailure {
Expand Down Expand Up @@ -273,9 +302,10 @@ trait ImplicitRunInfo { self: RunInfo =>
* a type variable, we need the current context, the current
* runinfo context does not do.
*/
def implicitScope(tp: Type, liftingCtx: Context): OfTypeImplicits = {
def implicitScope(rootTp: Type, liftingCtx: Context): OfTypeImplicits = {

val seen: mutable.Set[Type] = mutable.Set()
val incomplete: mutable.Set[Type] = mutable.Set()

/** Replace every typeref that does not refer to a class by a conjunction of class types
* that has the same implicit scope as the original typeref. The motivation for applying
Expand Down Expand Up @@ -309,16 +339,23 @@ trait ImplicitRunInfo { self: RunInfo =>
}
}

def iscopeRefs(tp: Type): TermRefSet =
if (seen contains tp) EmptyTermRefSet
else {
seen += tp
iscope(tp).companionRefs
}

// todo: compute implicits directly, without going via companionRefs?
def collectCompanions(tp: Type): TermRefSet = track("computeImplicitScope") {
ctx.traceIndented(i"collectCompanions($tp)", implicits) {

def iscopeRefs(t: Type): TermRefSet = implicitScopeCache.get(t) match {
case Some(is) =>
is.companionRefs
case None =>
if (seen contains t) {
incomplete += tp // all references to rootTo will be accounted for in `seen` so we return `EmptySet`.
EmptyTermRefSet // on the other hand, the refs of `tp` are now not accurate, so `tp` is marked incomplete.
} else {
seen += t
iscope(t).companionRefs
}
}

val comps = new TermRefSet
tp match {
case tp: NamedType =>
Expand Down Expand Up @@ -356,7 +393,8 @@ trait ImplicitRunInfo { self: RunInfo =>
* @param isLifted Type `tp` is the result of a `liftToClasses` application
*/
def iscope(tp: Type, isLifted: Boolean = false): OfTypeImplicits = {
def computeIScope(cacheResult: Boolean) = {
val canCache = Config.cacheImplicitScopes && tp.hash != NotCached
def computeIScope() = {
val savedEphemeral = ctx.typerState.ephemeral
ctx.typerState.ephemeral = false
try {
Expand All @@ -367,33 +405,23 @@ trait ImplicitRunInfo { self: RunInfo =>
else
collectCompanions(tp)
val result = new OfTypeImplicits(tp, refs)(ctx)
if (ctx.typerState.ephemeral) record("ephemeral cache miss: implicitScope")
else if (cacheResult) implicitScopeCache(tp) = result
if (ctx.typerState.ephemeral)
record("ephemeral cache miss: implicitScope")
else if (canCache &&
((tp eq rootTp) || // first type traversed is always cached
!incomplete.contains(tp) && // other types are cached if they are not incomplete
result.companionRefs.forall( // and all their companion refs are cached
implicitScopeCache.contains)))
implicitScopeCache(tp) = result
result
}
finally ctx.typerState.ephemeral |= savedEphemeral
}

if (tp.hash == NotCached || !Config.cacheImplicitScopes)
computeIScope(cacheResult = false)
else implicitScopeCache get tp match {
case Some(is) => is
case None =>
// Implicit scopes are tricky to cache because of loops. For example
// in `tests/pos/implicit-scope-loop.scala`, the scope of B contains
// the scope of A which contains the scope of B. We break the loop
// by returning EmptyTermRefSet in `collectCompanions` for types
// that we have already seen, but this means that we cannot cache
// the computed scope of A, it is incomplete.
// Keeping track of exactly where these loops happen would require a
// lot of book-keeping, instead we choose to be conservative and only
// cache scopes before any type has been seen. This is unfortunate
// because loops are very common for types in scala.collection.
computeIScope(cacheResult = seen.isEmpty)
}
if (canCache) implicitScopeCache.getOrElse(tp, computeIScope())
else computeIScope()
}

iscope(tp)
iscope(rootTp)
}

/** A map that counts the number of times an implicit ref was picked */
Expand Down Expand Up @@ -587,7 +615,7 @@ trait Implicits { self: Typer =>
val wildProto = implicitProto(pt, wildApprox(_))

/** Search failures; overridden in ExplainedImplicitSearch */
protected def nonMatchingImplicit(ref: TermRef): SearchFailure = NoImplicitMatches
protected def nonMatchingImplicit(ref: TermRef, trail: List[MessageContainer]): SearchFailure = NoImplicitMatches
protected def divergingImplicit(ref: TermRef): SearchFailure = NoImplicitMatches
protected def shadowedImplicit(ref: TermRef, shadowing: Type): SearchFailure = NoImplicitMatches
protected def failedSearch: SearchFailure = NoImplicitMatches
Expand Down Expand Up @@ -628,7 +656,7 @@ trait Implicits { self: Typer =>
{ implicits.println(i"invalid eqAny[$tp1, $tp2]"); false }
}
if (ctx.reporter.hasErrors)
nonMatchingImplicit(ref)
nonMatchingImplicit(ref, ctx.reporter.removeBufferedMessages)
else if (contextual && !ctx.mode.is(Mode.ImplicitShadowing) &&
!shadowing.tpe.isError && !refMatches(shadowing)) {
implicits.println(i"SHADOWING $ref in ${ref.termSymbol.owner} is shadowed by $shadowing in ${shadowing.symbol.owner}")
Expand All @@ -637,7 +665,7 @@ trait Implicits { self: Typer =>
else generated1 match {
case TypeApply(fn, targs @ (arg1 :: arg2 :: Nil))
if fn.symbol == defn.Predef_eqAny && !validEqAnyArgs(arg1.tpe, arg2.tpe) =>
nonMatchingImplicit(ref)
nonMatchingImplicit(ref, Nil)
case _ =>
SearchSuccess(generated1, ref, ctx.typerState)
}
Expand Down Expand Up @@ -743,8 +771,8 @@ trait Implicits { self: Typer =>
fail
}
def failures = myFailures.toList
override def nonMatchingImplicit(ref: TermRef) =
record(new NonMatchingImplicit(ref, pt, argument))
override def nonMatchingImplicit(ref: TermRef, trail: List[MessageContainer]) =
record(new NonMatchingImplicit(ref, pt, argument, trail))
override def divergingImplicit(ref: TermRef) =
record(new DivergingImplicit(ref, pt, argument))
override def shadowedImplicit(ref: TermRef, shadowing: Type): SearchFailure =
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/ReTyper.scala
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ class ReTyper extends Typer {

override def index(trees: List[untpd.Tree])(implicit ctx: Context) = ctx

override def tryInsertApplyOrImplicit(tree: Tree, pt: ProtoType)(fallBack: (Tree, TyperState) => Tree)(implicit ctx: Context): Tree =
fallBack(tree, ctx.typerState)
override def tryInsertApplyOrImplicit(tree: Tree, pt: ProtoType)(fallBack: => Tree)(implicit ctx: Context): Tree =
fallBack

override def completeAnnotations(mdef: untpd.MemberDef, sym: Symbol)(implicit ctx: Context): Unit = ()

Expand Down
32 changes: 23 additions & 9 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1585,18 +1585,34 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
* `fallBack`.
*
* 1st strategy: Try to insert `.apply` so that the result conforms to prototype `pt`.
* This strategy is not tried if the prototype represents already
* another `.apply` or `.apply()` selection.
* 2nd strategy: If tree is a select `qual.name`, try to insert an implicit conversion
* around the qualifier part `qual` so that the result conforms to the expected type
* with wildcard result type.
*/
def tryInsertApplyOrImplicit(tree: Tree, pt: ProtoType)(fallBack: (Tree, TyperState) => Tree)(implicit ctx: Context): Tree =
tryEither { implicit ctx =>
def tryInsertApplyOrImplicit(tree: Tree, pt: ProtoType)(fallBack: => Tree)(implicit ctx: Context): Tree = {

/** Is `pt` a prototype of an `apply` selection, or a parameterless function yielding one? */
def isApplyProto(pt: Type): Boolean = pt match {
case pt: SelectionProto => pt.name == nme.apply
case pt: FunProto => pt.args.isEmpty && isApplyProto(pt.resultType)
case pt: IgnoredProto => isApplyProto(pt.ignored)
case _ => false
}

def tryApply(implicit ctx: Context) = {
val sel = typedSelect(untpd.Select(untpd.TypedSplice(tree), nme.apply), pt)
if (sel.tpe.isError) sel else adapt(sel, pt)
} { (failedTree, failedState) =>
tryInsertImplicitOnQualifier(tree, pt).getOrElse(fallBack(failedTree, failedState))
}

def tryImplicit =
tryInsertImplicitOnQualifier(tree, pt).getOrElse(fallBack)

if (isApplyProto(pt)) tryImplicit
else tryEither(tryApply(_))((_, _) => tryImplicit)
}

/** If this tree is a select node `qual.name`, try to insert an implicit conversion
* `c` around `qual` so that `c(qual).name` conforms to `pt`.
*/
Expand Down Expand Up @@ -1688,7 +1704,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
def hasEmptyParams(denot: SingleDenotation) = denot.info.paramTypess == ListOfNil
pt match {
case pt: FunProto =>
tryInsertApplyOrImplicit(tree, pt)((_, _) => noMatches)
tryInsertApplyOrImplicit(tree, pt)(noMatches)
case _ =>
if (altDenots exists (_.info.paramTypess == ListOfNil))
typed(untpd.Apply(untpd.TypedSplice(tree), Nil), pt)
Expand Down Expand Up @@ -1727,7 +1743,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
case Apply(_, _) => " more"
case _ => ""
}
(_, _) => errorTree(tree, em"$methodStr does not take$more parameters")
errorTree(tree, em"$methodStr does not take$more parameters")
}
}

Expand Down Expand Up @@ -1946,9 +1962,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
case pt: FunProto =>
adaptToArgs(wtp, pt)
case pt: PolyProto =>
tryInsertApplyOrImplicit(tree, pt) {
(_, _) => tree // error will be reported in typedTypeApply
}
tryInsertApplyOrImplicit(tree, pt)(tree) // error will be reported in typedTypeApply
case _ =>
if (ctx.mode is Mode.Type) adaptType(tree.tpe)
else adaptNoArgs(wtp)
Expand Down
10 changes: 10 additions & 0 deletions tests/neg/i1639.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
class Bar {
implicit def f(implicit x: String): String = x

implicitly[String](f) // error: divergent (turn -explaintypes on to see it)
}

class Foo(implicit val bar: String) {
def this() = this("baz") // error: none of the alternatives match arguments
}