Skip to content
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

Integrate splain – implicit resolution chains and type diffs in error messages #7785

Merged
merged 4 commits into from Apr 21, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions project/ScalaOptionParser.scala
Expand Up @@ -82,7 +82,7 @@ object ScalaOptionParser {
}

// TODO retrieve these data programmatically, ala https://github.com/scala/scala-tool-support/blob/master/bash-completion/src/main/scala/BashCompletion.scala
private def booleanSettingNames = List("-X", "-Xasync", "-Xcheckinit", "-Xdev", "-Xdisable-assertions", "-Xexperimental", "-Xfatal-warnings", "-Xlog-free-terms", "-Xlog-free-types", "-Xlog-implicit-conversions", "-Xlog-implicits", "-Xlog-reflective-calls",
private def booleanSettingNames = List("-X", "-Xasync", "-Xcheckinit", "-Xdev", "-Xdisable-assertions", "-Xexperimental", "-Xfatal-warnings", "-Xlog-free-terms", "-Xlog-free-types", "-Xlog-implicit-conversions", "-Xlog-reflective-calls",
"-Xno-forwarders", "-Xno-patmat-analysis", "-Xnon-strict-patmat-analysis", "-Xprint-pos", "-Xprint-types", "-Xprompt", "-Xresident", "-Xshow-phases", "-Xverify", "-Y",
"-Ybreak-cycles", "-Ydebug", "-Ycompact-trees", "-YdisableFlatCpCaching", "-Ydoc-debug",
"-Yide-debug",
Expand All @@ -97,7 +97,7 @@ object ScalaOptionParser {
"-Vhot-statistics", "-Vide", "-Vimplicit-conversions", "-Vimplicits", "-Vissue",
"-Vmacro", "-Vmacro-lite", "-Vpatmat", "-Vphases", "-Vpos", "-Vprint-pos",
"-Vprint-types", "-Vquasiquote", "-Vreflective-calls", "-Vreify",
"-Vshow-member-pos", "-Vshow-symkinds", "-Vshow-symowners", "-Vsymbols", "-Vtyper",
"-Vshow-member-pos", "-Vshow-symkinds", "-Vshow-symowners", "-Vsymbols", "-Vtype-diffs", "-Vtyper",
"-W",
"-Wdead-code", "-Werror", "-Wextra-implicit", "-Wnumeric-widen", "-Woctal-literal",
"-Wvalue-discard", "-Wself-implicit",
Expand Down
2 changes: 0 additions & 2 deletions src/compiler/scala/reflect/reify/Taggers.scala
Expand Up @@ -102,8 +102,6 @@ abstract class Taggers {
val tpe = tpeTree.tpe
val PolyType(_, MethodType(_, tagTpe)) = fun.tpe: @unchecked
val tagModule = tagTpe.typeSymbol.companionSymbol
if (c.compilerSettings.contains("-Xlog-implicits"))
c.echo(c.enclosingPosition, s"cannot materialize ${tagModule.name}[$tpe] as $result because:\n$reason")
c.abort(c.enclosingPosition, "No %s available for %s".format(tagModule.name, tpe))
}

Expand Down
6 changes: 4 additions & 2 deletions src/compiler/scala/tools/nsc/settings/ScalaSettings.scala
Expand Up @@ -501,8 +501,10 @@ trait ScalaSettings extends StandardScalaSettings with Warnings { _: MutableSett
.withAbbreviation("-Yhot-statistics")
val Yshowsyms = BooleanSetting("-Vsymbols", "Print the AST symbol hierarchy after each phase.") withAbbreviation "-Yshow-syms"
val Ytyperdebug = BooleanSetting("-Vtyper", "Trace type assignments.") withAbbreviation "-Ytyper-debug"
val XlogImplicits = BooleanSetting("-Vimplicits", "Show more detail on why some implicits are not applicable.")
.withAbbreviation("-Xlog-implicits")
val Vimplicits = BooleanSetting("-Vimplicits", "Print dependent missing implicits.").withAbbreviation("-Xlog-implicits")
val VimplicitsVerboseTree = BooleanSetting("-Vimplicits-verbose-tree", "Display all intermediate implicits in a chain.")
val VimplicitsMaxRefined = IntSetting("-Vimplicits-max-refined", "max chars for printing refined types, abbreviate to `F {...}`", Int.MaxValue, Some((0, Int.MaxValue)), _ => None)
val VtypeDiffs = BooleanSetting("-Vtype-diffs", "Print found/required error messages as colored diffs.")
val logImplicitConv = BooleanSetting("-Vimplicit-conversions", "Print a message whenever an implicit conversion is inserted.")
.withAbbreviation("-Xlog-implicit-conversions")
val logReflectiveCalls = BooleanSetting("-Vreflective-calls", "Print a message when a reflective method call is generated")
Expand Down
19 changes: 18 additions & 1 deletion src/compiler/scala/tools/nsc/typechecker/AnalyzerPlugins.scala
Expand Up @@ -16,7 +16,7 @@ package typechecker
/**
* @author Lukas Rytz
*/
trait AnalyzerPlugins { self: Analyzer =>
trait AnalyzerPlugins { self: Analyzer with splain.SplainData =>
import global._

trait AnalyzerPlugin {
Expand Down Expand Up @@ -179,6 +179,16 @@ trait AnalyzerPlugins { self: Analyzer =>
* @param result The result to a given implicit search.
*/
def pluginsNotifyImplicitSearchResult(result: SearchResult): Unit = ()

/**
* Construct a custom error message for implicit parameters that could not be resolved.
*
* @param param The implicit parameter that was resolved
* @param errors The chain of intermediate implicits that lead to this error
* @param previous The error message constructed by the previous analyzer plugin, or the builtin default
*/
def noImplicitFoundError(param: Symbol, errors: List[ImplicitError], previous: String): String =
previous
}

/**
Expand Down Expand Up @@ -390,6 +400,13 @@ trait AnalyzerPlugins { self: Analyzer =>
def accumulate = (_, p) => p.pluginsNotifyImplicitSearchResult(result)
})

/** @see AnalyzerPlugin.noImplicitFoundError */
def pluginsNoImplicitFoundError(param: Symbol, errors: List[ImplicitError], initial: String): String =
invoke(new CumulativeOp[String] {
def default = initial
def accumulate = (previous, p) => p.noImplicitFoundError(param, errors, previous)
})

/** A list of registered macro plugins */
private var macroPlugins: List[MacroPlugin] = Nil

Expand Down
42 changes: 25 additions & 17 deletions src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala
Expand Up @@ -25,7 +25,7 @@ import scala.tools.nsc.util.stackTraceString
import scala.reflect.io.NoAbstractFile
import scala.reflect.internal.util.NoSourceFile

trait ContextErrors {
trait ContextErrors extends splain.SplainErrors {
self: Analyzer =>

import global._
Expand Down Expand Up @@ -108,7 +108,7 @@ trait ContextErrors {
def issueTypeError(err: AbsTypeError)(implicit context: Context): Unit = { context.issue(err) }

def typeErrorMsg(context: Context, found: Type, req: Type) =
if (context.openImplicits.nonEmpty && !settings.XlogImplicits.value)
if (context.openImplicits.nonEmpty && !settings.Vimplicits)
// OPT: avoid error string creation for errors that won't see the light of day, but predicate
// this on -Xsource:2.13 for bug compatibility with https://github.com/scala/scala/pull/7147#issuecomment-418233611
"type mismatch"
Expand Down Expand Up @@ -152,30 +152,38 @@ trait ContextErrors {
def MacroCantExpandIncompatibleMacrosError(internalMessage: String) =
MacroIncompatibleEngineError("macro cannot be expanded, because it was compiled by an incompatible macro engine", internalMessage)

/** The implicit not found message from the annotation, and whether it's a supplement message or not. */
def NoImplicitFoundAnnotation(tree: Tree, param: Symbol): (Boolean, String) = {
param match {
case ImplicitNotFoundMsg(msg) => (false, msg.formatParameterMessage(tree))
case _ =>
val paramTp = param.tpe
paramTp.typeSymbolDirect match {
case ImplicitNotFoundMsg(msg) => (false, msg.formatDefSiteMessage(paramTp))
case _ =>
val supplement = param.baseClasses.collectFirst {
case ImplicitNotFoundMsg(msg) => s" (${msg.formatDefSiteMessage(paramTp)})"
}.getOrElse("")
true -> supplement
}
}
}

def NoImplicitFoundError(tree: Tree, param: Symbol)(implicit context: Context): Unit = {
def errMsg = {
val (isSupplement, annotationMsg) = NoImplicitFoundAnnotation(tree, param)
def defaultErrMsg = {
val paramName = param.name
val paramTp = param.tpe
def evOrParam =
if (paramName startsWith nme.EVIDENCE_PARAM_PREFIX)
"evidence parameter of type"
else
s"parameter $paramName:"

param match {
case ImplicitNotFoundMsg(msg) => msg.formatParameterMessage(tree)
case _ =>
paramTp.typeSymbolDirect match {
case ImplicitNotFoundMsg(msg) => msg.formatDefSiteMessage(paramTp)
case _ =>
val supplement = param.baseClasses.collectFirst {
case ImplicitNotFoundMsg(msg) => s" (${msg.formatDefSiteMessage(paramTp)})"
}.getOrElse("")
s"could not find implicit value for $evOrParam $paramTp$supplement"
}
}
if (isSupplement) s"could not find implicit value for $evOrParam $paramTp$annotationMsg"
else annotationMsg
}
issueNormalTypeError(tree, errMsg)
val errMsg = splainPushOrReportNotFound(tree, param, annotationMsg)
issueNormalTypeError(tree, if (errMsg.isEmpty) defaultErrMsg else errMsg)
}

trait TyperContextErrors {
Expand Down
19 changes: 12 additions & 7 deletions src/compiler/scala/tools/nsc/typechecker/Implicits.scala
Expand Up @@ -33,7 +33,7 @@ import scala.tools.nsc.Reporting.WarningCategory
*
* @author Martin Odersky
*/
trait Implicits {
trait Implicits extends splain.SplainData {
self: Analyzer =>

import global._
Expand Down Expand Up @@ -105,12 +105,14 @@ trait Implicits {
if (shouldPrint)
typingStack.printTyping(tree, "typing implicit: %s %s".format(tree, context.undetparamsString))
val implicitSearchContext = context.makeImplicit(reportAmbiguous)
ImplicitErrors.startSearch(pt)
val dpt = if (isView) pt else dropByName(pt)
val isByName = dpt ne pt
val search = new ImplicitSearch(tree, dpt, isView, implicitSearchContext, pos, isByName)
pluginsNotifyImplicitSearch(search)
val result = search.bestImplicit
pluginsNotifyImplicitSearchResult(result)
ImplicitErrors.finishSearch(result.isSuccess, pt)

if (result.isFailure && saveAmbiguousDivergent && implicitSearchContext.reporter.hasErrors)
implicitSearchContext.reporter.propagateImplicitTypeErrorsTo(context.reporter)
Expand Down Expand Up @@ -146,7 +148,7 @@ trait Implicits {
if (result.isFailure && !silent) {
val err = context.reporter.firstError
val errPos = err.map(_.errPos).getOrElse(pos)
val errMsg = err.map(_.errMsg).getOrElse("implicit search has failed. to find out the reason, turn on -Xlog-implicits")
val errMsg = err.map(_.errMsg).getOrElse("implicit search has failed. to find out the reason, turn on -Vimplicits")
onError(errPos, errMsg)
}
result.tree
Expand Down Expand Up @@ -443,7 +445,7 @@ trait Implicits {
def pos = if (pos0 != NoPosition) pos0 else tree.pos

@inline final def failure(what: Any, reason: => String, pos: Position = this.pos): SearchResult = {
if (settings.XlogImplicits)
if (settings.debug)
reporter.echo(pos, s"$what is not a valid implicit value for $pt because:\n$reason")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This were the old style traces which are now handled by splain, no?

Copy link
Member

@dwijnand dwijnand Mar 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I was hoping but not what the tests seem to show. We lose these bits of information:

implicit-shadow.scala:6: <i2s: error> is not a valid implicit value for Int(1) => ?{def isEmpty: ?} because:
reference to i2s is ambiguous;
it is imported twice in the same scope by
import C._
and import B._
t6323a.scala:13: scala.reflect.api.`package`.materializeTypeTag[Test](scala.reflect.runtime.`package`.universe) is not a valid implicit value for reflect.runtime.universe.TypeTag[Test] because:
failed to typecheck the materialized tag:
cannot create a TypeTag referring to class Test.Test local to the reifee: use WeakTypeTag instead
      val value = u.typeOf[Test]
                          ^
implicit-log.scala:61: byVal is not a valid implicit value for Int(7) => ?{def unwrap: ?} because:
incompatible: (x: 7): 7 does not match expected type Int(7) => ?{def unwrap: ?}
    val res = 7.unwrap() // doesn't work
              ^

SearchFailure
}
Expand Down Expand Up @@ -906,7 +908,9 @@ trait Implicits {
// bounds check on the expandee tree
itree3.attachments.get[MacroExpansionAttachment] match {
case Some(MacroExpansionAttachment(exp @ TypeApply(fun, targs), _)) =>
checkBounds(exp, NoPrefix, NoSymbol, fun.symbol.typeParams, targs.map(_.tpe), "inferred ")
val targTpes = mapList(targs)(_.tpe)
val withinBounds = checkBounds(exp, NoPrefix, NoSymbol, fun.symbol.typeParams, targTpes, "inferred ")
if (!withinBounds) splainPushNonconformantBonds(pt, tree, targTpes, undetParams, None)
case _ => ()
}

Expand Down Expand Up @@ -953,6 +957,7 @@ trait Implicits {

context.reporter.firstError match {
case Some(err) =>
splainPushImplicitSearchFailure(itree3, pt, err)
fail("typing TypeApply reported errors for the implicit tree: " + err.errMsg)
case None =>
val result = new SearchResult(unsuppressMacroExpansion(itree3), subst, context.undetparams)
Expand Down Expand Up @@ -1492,17 +1497,17 @@ trait Implicits {
// so that if we find one, we could convert it to whatever universe we need by the means of the `in` method
// if no tag is found in scope, we end up here, where we ask someone to materialize the tag for us
// however, since the original search was about a tag with no particular prefix, we cannot proceed
// this situation happens very often, so emitting an error message here (even if only for -Xlog-implicits) would be too much
// this situation happens very often, so emitting an error message here (even if only for -Vimplicits) would be too much
//return failure(tp, "tag error: unsupported prefix type %s (%s)".format(pre, pre.kind))
return SearchFailure
}
)
// todo. migrate hardcoded materialization in Implicits to corresponding implicit macros
val materializer = atPos(pos.focus)(gen.mkMethodCall(TagMaterializers(tagClass), List(tp), if (prefix != EmptyTree) List(prefix) else List()))
if (settings.XlogImplicits) reporter.echo(pos, "materializing requested %s.%s[%s] using %s".format(pre, tagClass.name, tp, materializer))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use settings.debug here?

if (settings.debug) reporter.echo(pos, "materializing requested %s.%s[%s] using %s".format(pre, tagClass.name, tp, materializer))
if (context.macrosEnabled) success(materializer)
// don't call `failure` here. if macros are disabled, we just fail silently
// otherwise -Xlog-implicits will spam the long with zillions of "macros are disabled"
// otherwise -Vimplicits/-Vdebug will spam the long with zillions of "macros are disabled"
// this is ugly but temporary, since all this code will be removed once I fix implicit macros
else SearchFailure
}
Expand Down
Expand Up @@ -40,7 +40,7 @@ import scala.tools.nsc.Reporting.WarningCategory
*
* @author Paul Phillips
*/
trait TypeDiagnostics {
trait TypeDiagnostics extends splain.SplainDiagnostics {
self: Analyzer with StdAttachments =>

import global._
Expand Down Expand Up @@ -310,7 +310,7 @@ trait TypeDiagnostics {
// when the message will never be seen. I though context.reportErrors
// being false would do that, but if I return "<suppressed>" under
// that condition, I see it.
def foundReqMsg(found: Type, req: Type): String = {
def builtinFoundReqMsg(found: Type, req: Type): String = {
val foundWiden = found.widen
val reqWiden = req.widen
val sameNamesDifferentPrefixes =
Expand Down Expand Up @@ -340,6 +340,11 @@ trait TypeDiagnostics {
}
}

def foundReqMsg(found: Type, req: Type): String = {
val errMsg = splainFoundReqMsg(found, req)
if (errMsg.isEmpty) builtinFoundReqMsg(found, req) else errMsg
}

def typePatternAdvice(sym: Symbol, ptSym: Symbol) = {
val clazz = if (sym.isModuleClass) sym.companionClass else sym
val caseString =
Expand Down
94 changes: 94 additions & 0 deletions src/compiler/scala/tools/nsc/typechecker/splain/SplainData.scala
@@ -0,0 +1,94 @@
/*
* Scala (https://www.scala-lang.org)
*
* Copyright EPFL and Lightbend, Inc.
*
* Licensed under Apache License 2.0
* (http://www.apache.org/licenses/LICENSE-2.0).
*
* See the NOTICE file distributed with this work for
* additional information regarding copyright ownership.
*/

package scala.tools.nsc
package typechecker
package splain

import scala.util.matching.Regex

trait SplainData {
self: Analyzer =>

import global._

sealed trait ImplicitErrorSpecifics

object ImplicitErrorSpecifics {
case class NotFound(param: Symbol) extends ImplicitErrorSpecifics

case class NonconformantBounds(
targs: List[Type], tparams: List[Symbol], originalError: Option[AbsTypeError],
) extends ImplicitErrorSpecifics
}

object ImplicitErrors {
var stack: List[Type] = Nil
var errors: List[ImplicitError] = Nil

def push(error: ImplicitError): Unit = errors ::= error
def nesting: Int = stack.length - 1
def nested: Boolean = stack.nonEmpty
def removeErrorsFor(tpe: Type): Unit = errors = errors.dropWhile(_.tpe == tpe)

def startSearch(expectedType: Type): Unit = {
dwijnand marked this conversation as resolved.
Show resolved Hide resolved
if (settings.Vimplicits) {
if (!nested) errors = List()
stack = expectedType :: stack
}
}

def finishSearch(success: Boolean, expectedType: Type): Unit = {
dwijnand marked this conversation as resolved.
Show resolved Hide resolved
if (settings.Vimplicits) {
if (success) removeErrorsFor(expectedType)
stack = stack.drop(1)
}
}
}

case class ImplicitError(tpe: Type, candidate: Tree, nesting: Int, specifics: ImplicitErrorSpecifics) {
import ImplicitError._

override def equals(other: Any) = other match {
case o: ImplicitError => o.tpe.toString == tpe.toString && candidateName(this) == candidateName(o)
case _ => false
}

override def hashCode = (tpe.toString.##, ImplicitError.candidateName(this).##).##
override def toString = s"ImplicitError(${shortName(tpe.toString)}, ${shortName(candidate.toString)}), $nesting, $specifics)"
}

object ImplicitError {
def unapplyCandidate(e: ImplicitError): Tree =
e.candidate match {
case TypeApply(fun, _) => fun
case a => a
}

def candidateName(e: ImplicitError): String =
unapplyCandidate(e) match {
case Select(_, name) => name.toString
case Ident(name) => name.toString
case a => a.toString
}

val candidateRegex: Regex = """.*\.this\.(.*)""".r

def cleanCandidate(e: ImplicitError): String =
unapplyCandidate(e).toString match {
case candidateRegex(suf) => suf
case a => a
}

def shortName(ident: String): String = ident.substring(ident.lastIndexOf(".") + 1)
}
}
@@ -0,0 +1,26 @@
/*
* Scala (https://www.scala-lang.org)
*
* Copyright EPFL and Lightbend, Inc.
*
* Licensed under Apache License 2.0
* (http://www.apache.org/licenses/LICENSE-2.0).
*
* See the NOTICE file distributed with this work for
* additional information regarding copyright ownership.
*/

package scala.tools.nsc
package typechecker
package splain

trait SplainDiagnostics extends splain.SplainFormatting {
self: Analyzer =>

import global._

def splainFoundReqMsg(found: Type, req: Type): String = {
if (settings.VtypeDiffs) ";\n" + showFormattedL(formatDiff(found, req, top = true), break = true).indent.joinLines
else ""
}
}