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

Hint about forbidden combination of implicit values and conversions #16735

Merged
merged 19 commits into from
Feb 1, 2023
Merged
Show file tree
Hide file tree
Changes from 15 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
13 changes: 12 additions & 1 deletion compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2554,7 +2554,8 @@ class MissingImplicitArgument(
pt: Type,
where: String,
paramSymWithMethodCallTree: Option[(Symbol, tpd.Tree)] = None,
ignoredInstanceNormalImport: => Option[SearchSuccess]
ignoredInstanceNormalImport: => Option[SearchSuccess],
ignoredConvertibleImplicits: => Iterable[TermRef]
)(using Context) extends TypeMsg(MissingImplicitArgumentID), ShowMatchTrace(pt):

arg.tpe match
Expand Down Expand Up @@ -2743,8 +2744,18 @@ class MissingImplicitArgument(
// show all available additional info
def hiddenImplicitNote(s: SearchSuccess) =
i"\n\nNote: ${s.ref.symbol.showLocated} was not considered because it was not imported with `import given`."
def showImplicitAndConversions(imp: TermRef, convs: Iterable[TermRef]) =
i"\n- ${imp.symbol.showDcl}${convs.map(c => "\n - " + c.symbol.showDcl).mkString}"
def noChainConversionsNote(ignoredConvertibleImplicits: Iterable[TermRef]): Option[String] =
Option.when(ignoredConvertibleImplicits.nonEmpty)(
i"\n\nNote: implicit conversions are not automatically applied to arguments of using clauses. " +
i"You will have to pass the argument explicitly.\n" +
i"The following implicits in scope can be implicitly converted to ${pt.show}:" +
ignoredConvertibleImplicits.map { imp => s"\n- ${imp.symbol.showDcl}"}.mkString
)
super.msgPostscript
++ ignoredInstanceNormalImport.map(hiddenImplicitNote)
.orElse(noChainConversionsNote(ignoredConvertibleImplicits))
.getOrElse(ctx.typer.importSuggestionAddendum(pt))

def explain(using Context) = ""
Expand Down
46 changes: 45 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/Implicits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -923,7 +923,51 @@ trait Implicits:
// example where searching for a nested type causes an infinite loop.
None

MissingImplicitArgument(arg, pt, where, paramSymWithMethodCallTree, ignoredInstanceNormalImport)
def allImplicits(currImplicits: ContextualImplicits): List[ImplicitRef] =
if currImplicits.outerImplicits == null then currImplicits.refs
else currImplicits.refs ::: allImplicits(currImplicits.outerImplicits)

/** Ensure an implicit is not a Scala 2-style implicit conversion, based on its type */
def isScala2Conv(typ: Type): Boolean = typ match {
ysthakur marked this conversation as resolved.
Show resolved Hide resolved
case PolyType(_, resType) => isScala2Conv(resType)
case mt: MethodType => !mt.isImplicitMethod && !mt.isContextualMethod
case _ => false
}

def hasErrors(tree: Tree): Boolean =
if tree.tpe.isInstanceOf[ErrorType] then true
else
tree match {
case Apply(_, List(arg)) => arg.tpe.isInstanceOf[ErrorType]
case _ => false
}

def ignoredConvertibleImplicits = arg.tpe match
case fail: SearchFailureType =>
if (fail.expectedType eq pt) || isFullyDefined(fail.expectedType, ForceDegree.none) then
// Get every implicit in scope and try to convert each
allImplicits(ctx.implicits)
.distinctBy(_.underlyingRef.denot)
.view
.map(_.underlyingRef)
ysthakur marked this conversation as resolved.
Show resolved Hide resolved
.filter { imp =>
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, what was the problem with using ctx.implicits.eligible(ViewProto(...)) to find out if an implicit value can be converted to the type that we're searching for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking that in case the implicit value was something like given [T <: Bar]: Foo[T], it would've needed wildApprox to fill in the type argument to Foo and Foo[?] would've been too permissive, but after your comment, I just realized wildApprox takes bounds into account. It'd be clearer than the current approach and would give us the conversions that would apply to the implicits. I'll try that later.

Copy link
Contributor Author

@ysthakur ysthakur Jan 31, 2023

Choose a reason for hiding this comment

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

So I tried the following to check if there are applicable conversions:

ctx.implicits.eligible(
  ViewProto(
    wildApprox(imp.underlying.widen.finalResultType),
    fail.expectedType
  )
).nonEmpty

This works fine when the conversion doesn't have any type parameters (summon[String] and implicitly[String] work fine in the test), but when it does have type parameters, all the implicits in scope are listed (summon[Option[Int]] and implicitly[Option[Int]] don't work). Gonna have to do some more investigating there. I made another branch to test this out. Here's the first run.

Copy link
Contributor

@prolativ prolativ Jan 31, 2023

Choose a reason for hiding this comment

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

I tried to dig into that and it seems that we were trying to reinvent the wheel. There's already the method viewExists available in this context, which seems to does what canBeConverted was supposed to do, and it even has the same signature

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice find!

if isScala2Conv(imp.underlying) || imp.symbol == defn.Predef_conforms then
ysthakur marked this conversation as resolved.
Show resolved Hide resolved
false
else
// Using Mode.Printing will stop it from printing errors
val tried = Contexts.withMode(Mode.Printing) {
typed(
tpd.ref(imp).withSpan(arg.span),
fail.expectedType,
ctx.typerState.ownedVars
)
}
!hasErrors(tried) && fail.expectedType =:= tried.tpe
ysthakur marked this conversation as resolved.
Show resolved Hide resolved
}
else
Nil

MissingImplicitArgument(arg, pt, where, paramSymWithMethodCallTree, ignoredInstanceNormalImport, ignoredConvertibleImplicits)
}

/** A string indicating the formal parameter corresponding to a missing argument */
Expand Down
45 changes: 45 additions & 0 deletions tests/neg/i16453.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
-- [E172] Type Error: tests/neg/i16453.scala:21:19 ---------------------------------------------------------------------
21 | summon[List[Int]] // error
| ^
| No given instance of type List[Int] was found for parameter x of method summon in object Predef
-- [E172] Type Error: tests/neg/i16453.scala:23:21 ---------------------------------------------------------------------
23 | summon[Option[Int]] // error
| ^
|No given instance of type Option[Int] was found for parameter x of method summon in object Predef
|
|Note: implicit conversions are not automatically applied to arguments of using clauses. You will have to pass the argument explicitly.
|The following implicits in scope can be implicitly converted to Option[Int]:
|- final lazy given val baz3: Char
|- final lazy given val bar3: Int
-- [E172] Type Error: tests/neg/i16453.scala:24:26 ---------------------------------------------------------------------
24 | implicitly[Option[Char]] // error
| ^
|No given instance of type Option[Char] was found for parameter e of method implicitly in object Predef
|
|Note: implicit conversions are not automatically applied to arguments of using clauses. You will have to pass the argument explicitly.
|The following implicits in scope can be implicitly converted to Option[Char]:
|- final lazy given val baz3: Char
-- [E172] Type Error: tests/neg/i16453.scala:25:20 ---------------------------------------------------------------------
25 | implicitly[String] // error
| ^
|No given instance of type String was found for parameter e of method implicitly in object Predef
|
|Note: implicit conversions are not automatically applied to arguments of using clauses. You will have to pass the argument explicitly.
|The following implicits in scope can be implicitly converted to String:
|- final lazy given val baz3: Char
-- [E172] Type Error: tests/neg/i16453.scala:35:16 ---------------------------------------------------------------------
35 | summon[String] // error
| ^
|No given instance of type String was found for parameter x of method summon in object Predef
|
|Note: implicit conversions are not automatically applied to arguments of using clauses. You will have to pass the argument explicitly.
|The following implicits in scope can be implicitly converted to String:
|- implicit val baz2: Char
-- [E172] Type Error: tests/neg/i16453.scala:36:25 ---------------------------------------------------------------------
36 | implicitly[Option[Int]] // error
| ^
|No given instance of type Option[Int] was found for parameter e of method implicitly in object Predef
|
|Note: implicit conversions are not automatically applied to arguments of using clauses. You will have to pass the argument explicitly.
|The following implicits in scope can be implicitly converted to Option[Int]:
|- implicit val bar2: Int
37 changes: 37 additions & 0 deletions tests/neg/i16453.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import scala.language.implicitConversions

trait Foo { type T }

// This one is irrelevant, shouldn't be included in error message
given irrelevant: Long = ???

/** Use Scala 3 givens/conversions */
def testScala3() = {
given c1[T]: Conversion[T, Option[T]] = ???
given c2[F <: Foo](using f: F): Conversion[f.T, Option[f.T]] = ???
given Conversion[Char, String] = ???
given Conversion[Char, Option[Int]] = ???

given foo: Foo with
type T = Int
given bar3: Int = 0
given baz3: Char = 'a'

// This should get the usual error
summon[List[Int]] // error

summon[Option[Int]] // error
implicitly[Option[Char]] // error
implicitly[String] // error
}

/** Use Scala 2 implicits */
def testScala2() = {
implicit def toOpt[T](t: T): Option[T] = ???
implicit def char2Str(c: Char): String = ???
implicit val bar2: Int = 1
implicit val baz2: Char = 'b'

summon[String] // error
implicitly[Option[Int]] // error
}