From 46e3d773ce1cae594196dc984356e2629d3fd74a Mon Sep 17 00:00:00 2001 From: odersky Date: Sat, 3 Sep 2022 12:29:48 +0200 Subject: [PATCH 1/2] Refine AsSeenFrom approximation scheme 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. --- .../src/dotty/tools/dotc/core/TypeOps.scala | 49 +++++++++++++------ .../src/dotty/tools/dotc/core/Types.scala | 23 ++++++--- tests/neg/i15939.check | 45 +++++++++++++++++ tests/neg/i15939.scala | 24 +++++++++ 4 files changed, 119 insertions(+), 22 deletions(-) create mode 100644 tests/neg/i15939.check create mode 100644 tests/neg/i15939.scala diff --git a/compiler/src/dotty/tools/dotc/core/TypeOps.scala b/compiler/src/dotty/tools/dotc/core/TypeOps.scala index 35afab770259..7e49e3586169 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeOps.scala +++ b/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") @@ -58,7 +58,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 + private var approximated: Boolean = false + + /** The number of range approximations in invariant or contravariant positions + * - 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 +83,22 @@ 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. + if true then + approxCount += 1 range(defn.NothingType, pre) + else + 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) + else pre else if (pre.termSymbol.is(Package) && !thiscls.is(Package)) toPrefix(pre.select(nme.PACKAGE), cls, thiscls) @@ -119,10 +131,15 @@ 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 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 = diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 0df3fa368d5a..c861920b08a6 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/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,7 +5796,7 @@ 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 @@ -5797,7 +5804,7 @@ object Types { 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 } @@ -5812,11 +5819,15 @@ object Types { tp.argForParam(pre) match { case arg @ TypeRef(pre, _) if pre.isArgPrefixOf(arg.symbol) => arg.info match { - case argInfo: TypeBounds => expandBounds(argInfo) - case argInfo => reapply(arg) + case argInfo: TypeBounds => + expandBounds(argInfo) + case argInfo => + useAlternate(arg) } - case arg: TypeBounds => expandBounds(arg) - case arg => reapply(arg) + case arg: TypeBounds => + expandBounds(arg) + case arg => + useAlternate(arg) } /** Derived selection. diff --git a/tests/neg/i15939.check b/tests/neg/i15939.check new file mode 100644 index 000000000000..bc1ce15b8c76 --- /dev/null +++ b/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` diff --git a/tests/neg/i15939.scala b/tests/neg/i15939.scala new file mode 100644 index 000000000000..4307d8107e16 --- /dev/null +++ b/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 +} \ No newline at end of file From 7649eafdca338bbb478fd265b3c48236dd246eb8 Mon Sep 17 00:00:00 2001 From: odersky Date: Sat, 3 Sep 2022 17:12:39 +0200 Subject: [PATCH 2/2] Cleanups --- .../src/dotty/tools/dotc/core/TypeOps.scala | 26 +++---------------- .../src/dotty/tools/dotc/core/Types.scala | 12 +++------ 2 files changed, 7 insertions(+), 31 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/TypeOps.scala b/compiler/src/dotty/tools/dotc/core/TypeOps.scala index 7e49e3586169..44d81a5a6724 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeOps.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeOps.scala @@ -57,10 +57,9 @@ 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. */ - private 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. @@ -83,22 +82,8 @@ object TypeOps: case _ => if (thiscls.derivesFrom(cls) && pre.baseType(thiscls).exists) if (variance <= 0 && !isLegalPrefix(pre)) - if true then - approxCount += 1 - range(defn.NothingType, pre) - else - 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) @@ -131,11 +116,6 @@ 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 diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index c861920b08a6..bc00897d7783 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -5819,15 +5819,11 @@ object Types { tp.argForParam(pre) match { case arg @ TypeRef(pre, _) if pre.isArgPrefixOf(arg.symbol) => arg.info match { - case argInfo: TypeBounds => - expandBounds(argInfo) - case argInfo => - useAlternate(arg) + case argInfo: TypeBounds => expandBounds(argInfo) + case argInfo => useAlternate(arg) } - case arg: TypeBounds => - expandBounds(arg) - case arg => - useAlternate(arg) + case arg: TypeBounds => expandBounds(arg) + case arg => useAlternate(arg) } /** Derived selection.