Skip to content

Commit 46e3d77

Browse files
oderskyodersky
committed
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.
1 parent bf03086 commit 46e3d77

File tree

4 files changed

+119
-22
lines changed

4 files changed

+119
-22
lines changed

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

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ object TypeOps:
4545
val widenedAsf = new AsSeenFromMap(pre.info, cls)
4646
val ret = widenedAsf.apply(tp)
4747

48-
if (!widenedAsf.approximated)
48+
if widenedAsf.approxCount == 0 then
4949
return ret
5050

5151
Stats.record("asSeenFrom skolem prefix required")
@@ -58,7 +58,14 @@ object TypeOps:
5858
/** The TypeMap handling the asSeenFrom */
5959
class AsSeenFromMap(pre: Type, cls: Symbol)(using Context) extends ApproximatingTypeMap, IdempotentCaptRefMap {
6060
/** Set to true when the result of `apply` was approximated to avoid an unstable prefix. */
61-
var approximated: Boolean = false
61+
private var approximated: Boolean = false
62+
63+
/** The number of range approximations in invariant or contravariant positions
64+
* - Incremented each time we produce a range.
65+
* - Decremented each time we drop a prefix range by forwarding to a type alias
66+
* or singleton type.
67+
*/
68+
private[TypeOps] var approxCount: Int = 0
6269

6370
def apply(tp: Type): Type = {
6471

@@ -76,17 +83,22 @@ object TypeOps:
7683
case _ =>
7784
if (thiscls.derivesFrom(cls) && pre.baseType(thiscls).exists)
7885
if (variance <= 0 && !isLegalPrefix(pre))
79-
if (variance < 0) {
80-
approximated = true
81-
defn.NothingType
82-
}
83-
else
84-
// Don't set the `approximated` flag yet: if this is a prefix
85-
// of a path, we might be able to dealias the path instead
86-
// (this is handled in `ApproximatingTypeMap`). If dealiasing
87-
// is not possible, then `expandBounds` will end up being
88-
// called which we override to set the `approximated` flag.
86+
if true then
87+
approxCount += 1
8988
range(defn.NothingType, pre)
89+
else
90+
if (variance < 0) {
91+
approximated = true
92+
defn.NothingType
93+
}
94+
else
95+
// Don't set the `approximated` flag yet: if this is a prefix
96+
// of a path, we might be able to dealias the path instead
97+
// (this is handled in `ApproximatingTypeMap`). If dealiasing
98+
// is not possible, then `expandBounds` will end up being
99+
// called which we override to set the `approximated` flag.
100+
range(defn.NothingType, pre)
101+
90102
else pre
91103
else if (pre.termSymbol.is(Package) && !thiscls.is(Package))
92104
toPrefix(pre.select(nme.PACKAGE), cls, thiscls)
@@ -119,10 +131,15 @@ object TypeOps:
119131
// derived infos have already been subjected to asSeenFrom, hence to need to apply the map again.
120132
tp
121133

122-
override protected def expandBounds(tp: TypeBounds): Type = {
123-
approximated = true
124-
super.expandBounds(tp)
125-
}
134+
//override protected def expandBounds(tp: TypeBounds): Type = {
135+
// approximated = true
136+
// super.expandBounds(tp)
137+
//}
138+
139+
override protected def useAlternate(tp: Type): Type =
140+
assert(approxCount > 0)
141+
approxCount -= 1
142+
tp
126143
}
127144

128145
def isLegalPrefix(pre: Type)(using Context): Boolean =

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

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5766,6 +5766,13 @@ object Types {
57665766

57675767
private var expandingBounds: Boolean = false
57685768

5769+
/** Use an alterate type `tp` that replaces a range. This can happen if the
5770+
* prefix of a Select is a range and the selected symbol is an alias type
5771+
* or a value with a singleton type. In both cases we can forget the prefix
5772+
* and use the symbol's type.
5773+
*/
5774+
protected def useAlternate(tp: Type): Type = reapply(tp)
5775+
57695776
/** Whether it is currently expanding bounds
57705777
*
57715778
* It is used to avoid following LazyRef in F-Bounds
@@ -5789,15 +5796,15 @@ object Types {
57895796
case TypeAlias(alias) =>
57905797
// if H#T = U, then for any x in L..H, x.T =:= U,
57915798
// hence we can replace with U under all variances
5792-
reapply(alias.rewrapAnnots(tp1))
5799+
useAlternate(alias.rewrapAnnots(tp1))
57935800
case bounds: TypeBounds =>
57945801
// If H#T = ? >: S <: U, then for any x in L..H, S <: x.T <: U,
57955802
// hence we can replace with S..U under all variances
57965803
expandBounds(bounds)
57975804
case info: SingletonType =>
57985805
// if H#x: y.type, then for any x in L..H, x.type =:= y.type,
57995806
// hence we can replace with y.type under all variances
5800-
reapply(info)
5807+
useAlternate(info)
58015808
case _ =>
58025809
NoType
58035810
}
@@ -5812,11 +5819,15 @@ object Types {
58125819
tp.argForParam(pre) match {
58135820
case arg @ TypeRef(pre, _) if pre.isArgPrefixOf(arg.symbol) =>
58145821
arg.info match {
5815-
case argInfo: TypeBounds => expandBounds(argInfo)
5816-
case argInfo => reapply(arg)
5822+
case argInfo: TypeBounds =>
5823+
expandBounds(argInfo)
5824+
case argInfo =>
5825+
useAlternate(arg)
58175826
}
5818-
case arg: TypeBounds => expandBounds(arg)
5819-
case arg => reapply(arg)
5827+
case arg: TypeBounds =>
5828+
expandBounds(arg)
5829+
case arg =>
5830+
useAlternate(arg)
58205831
}
58215832

58225833
/** Derived selection.

‎tests/neg/i15939.check‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
-- [E007] Type Mismatch Error: tests/neg/i15939.scala:19:16 ------------------------------------------------------------
2+
19 | mkFoo.andThen(mkBarString) // error
3+
| ^^^^^^^^^^^
4+
| Found: String
5+
| Required: ?1.Bar
6+
|
7+
| where: ?1 is an unknown value of type Test.Foo
8+
|
9+
| longer explanation available when compiling with `-explain`
10+
-- [E007] Type Mismatch Error: tests/neg/i15939.scala:20:2 -------------------------------------------------------------
11+
20 | mkBarString andThen_: mkFoo // error
12+
| ^^^^^^^^^^^
13+
| Found: String
14+
| Required: ?2.Bar
15+
|
16+
| where: ?2 is an unknown value of type Test.Foo
17+
|
18+
| longer explanation available when compiling with `-explain`
19+
-- [E007] Type Mismatch Error: tests/neg/i15939.scala:21:18 ------------------------------------------------------------
20+
21 | mkFoo.andThen_:(mkBarString) // error
21+
| ^^^^^^^^^^^
22+
| Found: String
23+
| Required: ?3.Bar
24+
|
25+
| where: ?3 is an unknown value of type Test.Foo
26+
|
27+
| longer explanation available when compiling with `-explain`
28+
-- [E007] Type Mismatch Error: tests/neg/i15939.scala:22:2 -------------------------------------------------------------
29+
22 | mkBarString andThenByName_: mkFoo // error
30+
| ^^^^^^^^^^^
31+
| Found: String
32+
| Required: ?4.Bar
33+
|
34+
| where: ?4 is an unknown value of type Test.Foo
35+
|
36+
| longer explanation available when compiling with `-explain`
37+
-- [E007] Type Mismatch Error: tests/neg/i15939.scala:23:24 ------------------------------------------------------------
38+
23 | mkFoo.andThenByName_:(mkBarString) // error
39+
| ^^^^^^^^^^^
40+
| Found: String
41+
| Required: ?5.Bar
42+
|
43+
| where: ?5 is an unknown value of type Test.Foo
44+
|
45+
| longer explanation available when compiling with `-explain`

‎tests/neg/i15939.scala‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import scala.language.implicitConversions
2+
3+
object Test {
4+
class Foo {
5+
class Bar {
6+
override def toString() = "bar"
7+
}
8+
object Bar {
9+
implicit def fromString(a: String): Bar = { println("convert bar") ; new Bar }
10+
}
11+
12+
def andThen(b: Bar): Unit = { println("pre") ; println(s"use $b") ; println("post") }
13+
def andThen_:(b: Bar) = { println("pre") ; println(s"use $b") ; println("post") }
14+
def andThenByName_:(b: => Bar) = { println("pre") ; println(s"use $b") ; println(s"use $b") ; println("post") }
15+
}
16+
17+
def mkFoo: Foo = ???
18+
def mkBarString: String = ???
19+
mkFoo.andThen(mkBarString) // error
20+
mkBarString andThen_: mkFoo // error
21+
mkFoo.andThen_:(mkBarString) // error
22+
mkBarString andThenByName_: mkFoo // error
23+
mkFoo.andThenByName_:(mkBarString) // error
24+
}

0 commit comments

Comments
 (0)