Skip to content

Commit

Permalink
Make the new behavior dependent on source >= 3.4
Browse files Browse the repository at this point in the history
  • Loading branch information
@odersky
odersky committed Dec 18, 2023
1 parent df79601 commit 473b9af
Show file tree
Hide file tree
Showing 7 changed files with 122 additions and 69 deletions.
91 changes: 49 additions & 42 deletions compiler/src/dotty/tools/dotc/typer/Implicits.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1587,46 +1587,54 @@ trait Implicits:
&& candSucceedsGiven(ctx.owner)
end comesTooLate

val eligible = if contextual then preEligible.filterNot(comesTooLate) else preEligible
val eligible = // the eligible candidates that come before the search point
if contextual && sourceVersion.isAtLeast(SourceVersion.`3.4`)
then preEligible.filterNot(comesTooLate)
else preEligible

def checkResolutionChange(result: SearchResult) =
if (eligible ne preEligible)
&& !Feature.enabled(Feature.avoidLoopingGivens)
then searchImplicit(preEligible.diff(eligible), contextual) match
case prevResult: SearchSuccess =>
def remedy = pt match
case _: SelectionProto =>
"conversion,\n - use an import to get extension method into scope"
case _: ViewProto =>
"conversion"
case _ =>
"argument"

def showResult(r: SearchResult) = r match
case r: SearchSuccess => ctx.printer.toTextRef(r.ref).show
case r => r.show

result match
case result: SearchSuccess if prevResult.ref frozen_=:= result.ref =>
// OK
case _ =>
report.error(
em"""Warning: result of implicit search for $pt will change.
|Current result ${showResult(prevResult)} will be no longer eligible
| because it is not defined before the search position.
|Result with new rules: ${showResult(result)}.
|To opt into the new rules, use the `avoidLoopingGivens` language import,
|
|To fix the problem you could try one of the following:
| - rearrange definitions,
| - use an explicit $remedy.""",
srcPos)
case _ =>
then
val prevResult = searchImplicit(preEligible, contextual)
prevResult match
case prevResult: SearchSuccess =>
def remedy = pt match
case _: SelectionProto =>
"conversion,\n - use an import to get extension method into scope"
case _: ViewProto =>
"conversion"
case _ =>
"argument"

def showResult(r: SearchResult) = r match
case r: SearchSuccess => ctx.printer.toTextRef(r.ref).show
case r => r.show

result match
case result: SearchSuccess if prevResult.ref frozen_=:= result.ref =>
// OK
case _ =>
report.error(
em"""Warning: result of implicit search for $pt will change.
|Current result ${showResult(prevResult)} will be no longer eligible
| because it is not defined before the search position.
|Result with new rules: ${showResult(result)}.
|To opt into the new rules, use the `experimental.avoidLoopingGivens` language import.
|
|To fix the problem without the language import, you could try one of the following:
| - rearrange definitions so that ${showResult(prevResult)} comes earlier,
| - use an explicit $remedy.""",
srcPos)
case _ =>
prevResult
else result
end checkResolutionChange

searchImplicit(eligible, contextual) match
val result = searchImplicit(eligible, contextual)
result match
case result: SearchSuccess =>
result
checkResolutionChange(result)
case failure: SearchFailure =>
failure.reason match
case _: AmbiguousImplicits => failure
Expand All @@ -1641,15 +1649,14 @@ trait Implicits:
else ctxImplicits.nn.outerImplicits: ContextualImplicits | Null
// !!! Dotty problem: without the ContextualImplicits | Null type ascription
// we get a Ycheck failure after arrayConstructors due to "Types differ"
val result = searchImplicit(newCtxImplicits).recoverWith:
failure2 => failure2.reason match
case _: AmbiguousImplicits => failure2
case _ =>
reason match
case (_: DivergingImplicit) => failure
case _ => List(failure, failure2).maxBy(_.tree.treeSize)
checkResolutionChange(result)
result
checkResolutionChange:
searchImplicit(newCtxImplicits).recoverWith:
failure2 => failure2.reason match
case _: AmbiguousImplicits => failure2
case _ =>
reason match
case (_: DivergingImplicit) => failure
case _ => List(failure, failure2).maxBy(_.tree.treeSize)
else failure
end searchImplicit

Expand Down
42 changes: 34 additions & 8 deletions tests/neg/i15474.check
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,38 @@
-- Error: tests/neg/i15474.scala:7:35 ----------------------------------------------------------------------------------
7 | def apply(from: String): Int = from.toInt // error
| ^^^^
| Warning: result of implicit search for ?{ toInt: ? } will change.
| Current result Test1.c will be no longer eligible
| because it is not defined before the search position.
| Result with new rules: augmentString.
| To opt into the new rules, use the `experimental.avoidLoopingGivens` language import.
|
| To fix the problem without the language import, you could try one of the following:
| - rearrange definitions so that Test1.c comes earlier,
| - use an explicit conversion,
| - use an import to get extension method into scope.
-- Error: tests/neg/i15474.scala:10:39 ---------------------------------------------------------------------------------
10 | given c: Conversion[ String, Int ] = _.toInt // error
| ^
| Warning: result of implicit search for ?{ toInt: ? } will change.
| Current result Test2.c will be no longer eligible
| because it is not defined before the search position.
| Result with new rules: augmentString.
| To opt into the new rules, use the `experimental.avoidLoopingGivens` language import.
|
| To fix the problem without the language import, you could try one of the following:
| - rearrange definitions so that Test2.c comes earlier,
| - use an explicit conversion,
| - use an import to get extension method into scope.
-- Error: tests/neg/i15474.scala:16:56 ---------------------------------------------------------------------------------
16 | given Ordering[Price] = summon[Ordering[BigDecimal]] // error
| ^
| Warning: result of implicit search for Ordering[BigDecimal] will change.
| Current result Prices.Price.given_Ordering_Price will be no longer eligible
| because it is not defined before the search position.
| Result with new rules: scala.math.Ordering.BigDecimal.
| To opt into the new rules, use the `avoidLoopingGivens` language import,
| Warning: result of implicit search for Ordering[BigDecimal] will change.
| Current result Prices.Price.given_Ordering_Price will be no longer eligible
| because it is not defined before the search position.
| Result with new rules: scala.math.Ordering.BigDecimal.
| To opt into the new rules, use the `experimental.avoidLoopingGivens` language import.
|
| To fix the problem you could try one of the following:
| - rearrange definitions,
| - use an explicit argument.
| To fix the problem without the language import, you could try one of the following:
| - rearrange definitions so that Prices.Price.given_Ordering_Price comes earlier,
| - use an explicit argument.
4 changes: 2 additions & 2 deletions tests/neg/i15474.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ import scala.language.implicitConversions

object Test1:
given c: Conversion[ String, Int ] with
def apply(from: String): Int = from.toInt // was error, now avoided
def apply(from: String): Int = from.toInt // error

object Test2:
given c: Conversion[ String, Int ] = _.toInt // now avoided, was loop not detected, could be used as a fallback to avoid the warning.
given c: Conversion[ String, Int ] = _.toInt // error

object Prices {
opaque type Price = BigDecimal
Expand Down
16 changes: 8 additions & 8 deletions tests/neg/i6716.check
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
-- Error: tests/neg/i6716.scala:12:39 ----------------------------------------------------------------------------------
12 | given Monad[Bar] = summon[Monad[Foo]] // error
| ^
| Warning: result of implicit search for Monad[Foo] will change.
| Current result Bar.given_Monad_Bar will be no longer eligible
| because it is not defined before the search position.
| Result with new rules: Foo.given_Monad_Foo.
| To opt into the new rules, use the `avoidLoopingGivens` language import,
| Warning: result of implicit search for Monad[Foo] will change.
| Current result Bar.given_Monad_Bar will be no longer eligible
| because it is not defined before the search position.
| Result with new rules: Foo.given_Monad_Foo.
| To opt into the new rules, use the `experimental.avoidLoopingGivens` language import.
|
| To fix the problem you could try one of the following:
| - rearrange definitions,
| - use an explicit argument.
| To fix the problem without the language import, you could try one of the following:
| - rearrange definitions so that Bar.given_Monad_Bar comes earlier,
| - use an explicit argument.
18 changes: 9 additions & 9 deletions tests/neg/i7294-a.check
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,19 @@
| Current result foo.i7294-a$package.f will be no longer eligible
| because it is not defined before the search position.
| Result with new rules: No Matching Implicit.
| To opt into the new rules, use the `avoidLoopingGivens` language import,
| To opt into the new rules, use the `experimental.avoidLoopingGivens` language import.
|
| To fix the problem you could try one of the following:
| - rearrange definitions,
| To fix the problem without the language import, you could try one of the following:
| - rearrange definitions so that foo.i7294-a$package.f comes earlier,
| - use an explicit argument.
|
| where: T is a type in given instance f with bounds <: foo.Foo
-- [E007] Type Mismatch Error: tests/neg/i7294-a.scala:6:15 ------------------------------------------------------------
-- [E007] Type Mismatch Error: tests/neg/i7294-a.scala:6:18 ------------------------------------------------------------
6 | case x: T => x.g(10) // error // error
| ^
| Found: (x : Nothing)
| Required: ?{ g: ? }
| Note that implicit conversions were not tried because the result of an implicit conversion
| must be more specific than ?{ g: [applied to (10) returning T] }
| ^^^^^^^
| Found: Any
| Required: T
|
| where: T is a type in given instance f with bounds <: foo.Foo
|
| longer explanation available when compiling with `-explain`
9 changes: 9 additions & 0 deletions tests/neg/looping-givens.scala
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
class A
class B

given joint(using a: A, b: B): (A & B) = ???

def foo(using a: A, b: B) =
given aa: A = summon // error
given bb: B = summon // error
given ab: (A & B) = summon // error
11 changes: 11 additions & 0 deletions tests/pos/looping-givens.scala
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import language.experimental.avoidLoopingGivens

class A
class B

given joint(using a: A, b: B): (A & B) = ???

def foo(using a: A, b: B) =
given aa: A = summon // error
given bb: B = summon // error
given ab: (A & B) = summon // error

0 comments on commit 473b9af

Please sign in to comment.