Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refine AsSeenFrom approximation scheme #15957

Merged
merged 2 commits into from
Sep 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 15 additions & 18 deletions compiler/src/dotty/tools/dotc/core/TypeOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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 = {

Expand All @@ -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)
Expand Down Expand Up @@ -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 =
Expand Down
15 changes: 11 additions & 4 deletions compiler/src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
Expand All @@ -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.
Expand Down
45 changes: 45 additions & 0 deletions tests/neg/i15939.check
Original file line number Diff line number Diff line change
@@ -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`
24 changes: 24 additions & 0 deletions tests/neg/i15939.scala
Original file line number Diff line number Diff line change
@@ -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
}