Refine AsSeenFrom approximation scheme (#15957)

The previous scheme did not set the approximated flag in some cases, which led
to the appearance of unhelpful Nothing types in error messages.

We now always mark the AsSeenFrom map as approximated when encountering an illegal prefix at variance <= 0.
But we reset it again when a range in a prefix is dropped because we can follow a type alias or use a
singleton type info.

Furthermore, we use an `Int`, `approxCount` instead of `approximated` to keep track of
multiple approximation points.

Fixes #15939 in the sense that the error message now makes more sense. We still cannot find the implicit conversion; that would require a more global measure to fix the problem (such as going to ANF) or existential types.

Author: smarter

compiler/src/dotty/tools/dotc/core/TypeOps.scala

@@ -45,7 +45,7 @@ object TypeOps:
         val widenedAsf = new AsSeenFromMap(pre.info, cls)
         val ret = widenedAsf.apply(tp)
 
-        if (!widenedAsf.approximated)
+        if widenedAsf.approxCount == 0 then
           return ret
 
         Stats.record("asSeenFrom skolem prefix required")
@@ -57,8 +57,14 @@ object TypeOps:
 
   /** The TypeMap handling the asSeenFrom */
   class AsSeenFromMap(pre: Type, cls: Symbol)(using Context) extends ApproximatingTypeMap, IdempotentCaptRefMap {
-    /** Set to true when the result of `apply` was approximated to avoid an unstable prefix. */
-    var approximated: Boolean = false
+
+    /** The number of range approximations in invariant or contravariant positions
+     *  performed by this TypeMap.
+     *   - Incremented each time we produce a range.
+     *   - Decremented each time we drop a prefix range by forwarding to a type alias
+     *     or singleton type.
+     */
+    private[TypeOps] var approxCount: Int = 0
 
     def apply(tp: Type): Type = {
 
@@ -76,17 +82,8 @@ object TypeOps:
           case _ =>
             if (thiscls.derivesFrom(cls) && pre.baseType(thiscls).exists)
               if (variance <= 0 && !isLegalPrefix(pre))
-                if (variance < 0) {
-                  approximated = true
-                  defn.NothingType
-                }
-                else
-                  // Don't set the `approximated` flag yet: if this is a prefix
-                  // of a path, we might be able to dealias the path instead
-                  // (this is handled in `ApproximatingTypeMap`). If dealiasing
-                  // is not possible, then `expandBounds` will end up being
-                  // called which we override to set the `approximated` flag.
-                  range(defn.NothingType, pre)
+                approxCount += 1
+                range(defn.NothingType, pre)
               else pre
             else if (pre.termSymbol.is(Package) && !thiscls.is(Package))
               toPrefix(pre.select(nme.PACKAGE), cls, thiscls)
@@ -119,10 +116,10 @@ object TypeOps:
       // derived infos have already been subjected to asSeenFrom, hence to need to apply the map again.
       tp
 
-    override protected def expandBounds(tp: TypeBounds): Type = {
-      approximated = true
-      super.expandBounds(tp)
-    }
+    override protected def useAlternate(tp: Type): Type =
+      assert(approxCount > 0)
+      approxCount -= 1
+      tp
   }
 
   def isLegalPrefix(pre: Type)(using Context): Boolean =

compiler/src/dotty/tools/dotc/core/Types.scala

@@ -5766,6 +5766,13 @@ object Types {
 
     private var expandingBounds: Boolean = false
 
+    /** Use an alterate type `tp` that replaces a range. This can happen if the
+     *  prefix of a Select is a range and the selected symbol is an alias type
+     *  or a value with a singleton type. In both cases we can forget the prefix
+     *  and use the symbol's type.
+     */
+    protected def useAlternate(tp: Type): Type = reapply(tp)
+
     /** Whether it is currently expanding bounds
      *
      *  It is used to avoid following LazyRef in F-Bounds
@@ -5789,15 +5796,15 @@ object Types {
           case TypeAlias(alias) =>
             // if H#T = U, then for any x in L..H, x.T =:= U,
             // hence we can replace with U under all variances
-            reapply(alias.rewrapAnnots(tp1))
+            useAlternate(alias.rewrapAnnots(tp1))
           case bounds: TypeBounds =>
             // If H#T = ? >: S <: U, then for any x in L..H, S <: x.T <: U,
             // hence we can replace with S..U under all variances
             expandBounds(bounds)
           case info: SingletonType =>
             // if H#x: y.type, then for any x in L..H, x.type =:= y.type,
             // hence we can replace with y.type under all variances
-            reapply(info)
+            useAlternate(info)
           case _ =>
             NoType
         }
@@ -5813,10 +5820,10 @@ object Types {
         case arg @ TypeRef(pre, _) if pre.isArgPrefixOf(arg.symbol) =>
           arg.info match {
             case argInfo: TypeBounds => expandBounds(argInfo)
-            case argInfo => reapply(arg)
+            case argInfo => useAlternate(arg)
           }
         case arg: TypeBounds => expandBounds(arg)
-        case arg => reapply(arg)
+        case arg => useAlternate(arg)
       }
 
     /** Derived selection.

tests/neg/i15939.check

@@ -0,0 +1,45 @@
+-- [E007] Type Mismatch Error: tests/neg/i15939.scala:19:16 ------------------------------------------------------------
+19 |  mkFoo.andThen(mkBarString)  // error
+   |                ^^^^^^^^^^^
+   |                Found:    String
+   |                Required: ?1.Bar
+   |
+   |                where:    ?1 is an unknown value of type Test.Foo
+   |
+   | longer explanation available when compiling with `-explain`
+-- [E007] Type Mismatch Error: tests/neg/i15939.scala:20:2 -------------------------------------------------------------
+20 |  mkBarString andThen_: mkFoo  // error
+   |  ^^^^^^^^^^^
+   |  Found:    String
+   |  Required: ?2.Bar
+   |
+   |  where:    ?2 is an unknown value of type Test.Foo
+   |
+   | longer explanation available when compiling with `-explain`
+-- [E007] Type Mismatch Error: tests/neg/i15939.scala:21:18 ------------------------------------------------------------
+21 |  mkFoo.andThen_:(mkBarString)  // error
+   |                  ^^^^^^^^^^^
+   |                  Found:    String
+   |                  Required: ?3.Bar
+   |
+   |                  where:    ?3 is an unknown value of type Test.Foo
+   |
+   | longer explanation available when compiling with `-explain`
+-- [E007] Type Mismatch Error: tests/neg/i15939.scala:22:2 -------------------------------------------------------------
+22 |  mkBarString andThenByName_: mkFoo // error
+   |  ^^^^^^^^^^^
+   |  Found:    String
+   |  Required: ?4.Bar
+   |
+   |  where:    ?4 is an unknown value of type Test.Foo
+   |
+   | longer explanation available when compiling with `-explain`
+-- [E007] Type Mismatch Error: tests/neg/i15939.scala:23:24 ------------------------------------------------------------
+23 |  mkFoo.andThenByName_:(mkBarString) // error
+   |                        ^^^^^^^^^^^
+   |                        Found:    String
+   |                        Required: ?5.Bar
+   |
+   |                        where:    ?5 is an unknown value of type Test.Foo
+   |
+   | longer explanation available when compiling with `-explain`

tests/neg/i15939.scala

@@ -0,0 +1,24 @@
+import scala.language.implicitConversions
+
+object Test {
+  class Foo {
+    class Bar {
+      override def toString() = "bar"
+    }
+    object Bar {
+      implicit def fromString(a: String): Bar = { println("convert bar") ; new Bar }
+    }
+
+    def andThen(b: Bar): Unit = { println("pre") ; println(s"use $b") ; println("post") }
+    def andThen_:(b: Bar) = { println("pre") ; println(s"use $b") ; println("post") }
+    def andThenByName_:(b: => Bar) = { println("pre") ; println(s"use $b") ; println(s"use $b") ; println("post") }
+  }
+
+  def mkFoo: Foo = ???
+  def mkBarString: String = ???
+  mkFoo.andThen(mkBarString)  // error
+  mkBarString andThen_: mkFoo  // error
+  mkFoo.andThen_:(mkBarString)  // error
+  mkBarString andThenByName_: mkFoo // error
+  mkFoo.andThenByName_:(mkBarString) // error
+}