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.

Author: odersky

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

compiler/src/dotty/tools/dotc/typer/Implicits.scala

@@ -1569,16 +1569,18 @@ 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
           def candSucceedsGiven(sym: Symbol): Boolean =
             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

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

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

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
 

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

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
+

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.