-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
ea13cad
Search for Conversions if implicit search fails
ysthakur fc9acd7
Use ctx.implicits.eligible directly
ysthakur 393dab4
Use symbol.showDcl, add sentence on explicitly passing arg
ysthakur 1d7f8dc
Remove test from negAll
ysthakur 208071a
Test implicitly() too; change err msg
ysthakur 3f42e53
Merge branch 'lampepfl:main' into imp-with-conversion-msg
ysthakur 6bded27
Use clearer name
ysthakur 76c826d
Update message to be more generic
ysthakur adc1995
Edit message according to suggestion
ysthakur 82d69a7
Update check
ysthakur 859d30b
Find all implicits first
ysthakur b5755de
Use finalResultType
ysthakur 9579aa5
Filter out Scala 2-style implicit conversions
ysthakur ebb616c
Use typed directly instead of finding conversions
ysthakur 98bf4b2
Fix inversion of if, check for errors properly
ysthakur dda91bb
Make code more readable
ysthakur 2584cf6
Remove unnecessary check for Predef.conforms
ysthakur 95c9dfb
Put check for Predef.conforms back
ysthakur 809f9fc
Use viewExists
ysthakur File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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?There was a problem hiding this comment.
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 neededwildApprox
to fill in the type argument toFoo
andFoo[?]
would've been too permissive, but after your comment, I just realizedwildApprox
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.There was a problem hiding this comment.
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]
andimplicitly[String]
work fine in the test), but when it does have type parameters, all the implicits in scope are listed (summon[Option[Int]]
andimplicitly[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.There was a problem hiding this comment.
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 whatcanBeConverted
was supposed to do, and it even has the same signatureThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice find!