From f56aa22e915ec43a076773f06b586ab483d917d1 Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 18 Dec 2023 21:51:19 +0100 Subject: [PATCH] Restrict the new rules to givens that don't define a new class `given ... with` or `given ... = new { ... }` kinds of definitions now follow the old rules. This allows recursive `given...with` definitions as they are found in protoQuill. We still have the old check in a later phase against directly recursive methods. Of the three loops in the original i15474 we now detect #2 and #3 with new new restrictions. #1 slips through since it is a loop involving a `given...with` instance of `Conversion`, but is caught later with the recursive method check. Previously tests #1 and #3 were detected with the recursive methods check and #2 slipped through altogether. The new rules are enough for defining simple givens with `=` without fear of looping. --- .../dotty/tools/dotc/semanticdb/Scala3.scala | 3 --- .../dotty/tools/dotc/typer/Implicits.scala | 8 +++--- .../quoted/runtime/impl/QuotesImpl.scala | 4 --- tests/neg/i15474.check | 27 +++++-------------- tests/neg/i15474.scala | 4 --- tests/neg/i15474b.check | 5 ++++ tests/neg/i15474b.scala | 8 ++++++ tests/pos/i15474.scala | 4 --- 8 files changed, 25 insertions(+), 38 deletions(-) create mode 100644 tests/neg/i15474b.check create mode 100644 tests/neg/i15474b.scala diff --git a/compiler/src/dotty/tools/dotc/semanticdb/Scala3.scala b/compiler/src/dotty/tools/dotc/semanticdb/Scala3.scala index fdb9251951e5..f49b00089712 100644 --- a/compiler/src/dotty/tools/dotc/semanticdb/Scala3.scala +++ b/compiler/src/dotty/tools/dotc/semanticdb/Scala3.scala @@ -77,9 +77,6 @@ object Scala3: type SemanticSymbol = Symbol | FakeSymbol given SemanticSymbolOps : AnyRef with - import SymbolOps.* - import StringOps.* - extension (sym: SemanticSymbol) def name(using Context): Name = sym match case s: Symbol => s.name diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 77328cdbb33d..3d3320eb7589 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -1569,8 +1569,10 @@ trait Implicits: /** Does candidate `cand` come too late for it to be considered as an * eligible candidate? This is the case if `cand` appears in the same - * scope as a given definition enclosing the search point and comes - * later in the source or coincides with that given definition. + * scope as a given definition enclosing the search point (with no + * class methods between the given definition and the search point) + * and `cand` comes later in the source or coincides with that given + * definition. */ def comesTooLate(cand: Candidate): Boolean = val candSym = cand.ref.symbol @@ -1578,7 +1580,7 @@ trait Implicits: if sym.owner == candSym.owner then if sym.is(ModuleClass) then candSucceedsGiven(sym.sourceModule) else sym.is(Given) && sym.span.exists && sym.span.start <= candSym.span.start - else if sym.is(Package) then false + else if sym.owner.isClass then false else candSucceedsGiven(sym.owner) ctx.isTyper diff --git a/compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala b/compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala index 1203e309c484..51f133c972b4 100644 --- a/compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala +++ b/compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala @@ -1781,8 +1781,6 @@ class QuotesImpl private (using val ctx: Context) extends Quotes, QuoteUnpickler end TypeRepr given TypeReprMethods: TypeReprMethods with - import SymbolMethods.* - extension (self: TypeRepr) def show(using printer: Printer[TypeRepr]): String = printer.show(self) @@ -2610,8 +2608,6 @@ class QuotesImpl private (using val ctx: Context) extends Quotes, QuoteUnpickler end Symbol given SymbolMethods: SymbolMethods with - import FlagsMethods.* - extension (self: Symbol) def owner: Symbol = self.denot.owner def maybeOwner: Symbol = self.denot.maybeOwner diff --git a/tests/neg/i15474.check b/tests/neg/i15474.check index 0b23628d3051..f99c6778d1ae 100644 --- a/tests/neg/i15474.check +++ b/tests/neg/i15474.check @@ -1,31 +1,18 @@ --- Error: tests/neg/i15474.scala:7:35 ---------------------------------------------------------------------------------- -7 | def apply(from: String): Int = from.toInt // error - | ^^^^ +-- Error: tests/neg/i15474.scala:6:39 ---------------------------------------------------------------------------------- +6 | given c: Conversion[ String, Int ] = _.toInt // error + | ^ | Warning: result of implicit search for ?{ toInt: ? } will change. - | Current result Test1.c will be no longer eligible + | 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 Test1.c comes earlier, + | - 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: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 +-- Error: tests/neg/i15474.scala:12:56 --------------------------------------------------------------------------------- +12 | 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 diff --git a/tests/neg/i15474.scala b/tests/neg/i15474.scala index 5a66ea016630..b196d1b400ef 100644 --- a/tests/neg/i15474.scala +++ b/tests/neg/i15474.scala @@ -2,10 +2,6 @@ import scala.language.implicitConversions -object Test1: - given c: Conversion[ String, Int ] with - def apply(from: String): Int = from.toInt // error - object Test2: given c: Conversion[ String, Int ] = _.toInt // error diff --git a/tests/neg/i15474b.check b/tests/neg/i15474b.check new file mode 100644 index 000000000000..73ef720af7e3 --- /dev/null +++ b/tests/neg/i15474b.check @@ -0,0 +1,5 @@ +-- Error: tests/neg/i15474b.scala:7:40 --------------------------------------------------------------------------------- +7 | def apply(from: String): Int = from.toInt // error: infinite loop in function body + | ^^^^^^^^^^ + | Infinite loop in function body + | Test1.c.apply(from).toInt diff --git a/tests/neg/i15474b.scala b/tests/neg/i15474b.scala new file mode 100644 index 000000000000..9d496c37ef00 --- /dev/null +++ b/tests/neg/i15474b.scala @@ -0,0 +1,8 @@ +//> using options -Xfatal-warnings + +import scala.language.implicitConversions + +object Test1: + given c: Conversion[ String, Int ] with + def apply(from: String): Int = from.toInt // error: infinite loop in function body + diff --git a/tests/pos/i15474.scala b/tests/pos/i15474.scala index 8adc5ad7233d..6b9e55806ae3 100644 --- a/tests/pos/i15474.scala +++ b/tests/pos/i15474.scala @@ -2,10 +2,6 @@ import scala.language.implicitConversions import scala.language.experimental.avoidLoopingGivens -object Test1: - given c: Conversion[ String, Int ] with - def apply(from: String): Int = from.toInt // was error, now avoided - object Test2: given c: Conversion[ String, Int ] = _.toInt // now avoided, was loop not detected, could be used as a fallback to avoid the warning.