Skip to content

Commit

Permalink
Drop phase.isTyper use in isLegalPrefix/asf (#21954)
Browse files Browse the repository at this point in the history
Note that "asSeenFrom" (aka asf) is used by SymDenotation#findMember,
which is used by TypeComparer's "hasMatchingMember", as a part of
"compareRefinedSlow".

Previously (using the minimisation in i17222.8.scala) `summonOne` is
inlined during the "inlining" phase, while "summonInline" is inlined
during typing.  The result is that during inlining we fail to find
`Reader[BC, Int]` because we incorrectly consider `A{type F = Int}` as
stable.

Fixes #17222
  • Loading branch information
noti0na1 authored Nov 22, 2024
2 parents 5d1d274 + d64766f commit 912b6f2
Show file tree
Hide file tree
Showing 18 changed files with 647 additions and 28 deletions.
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/core/CheckRealizable.scala
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,8 @@ class CheckRealizable(using Context) {
/** `Realizable` if `tp` has good bounds, a `HasProblem...` instance
* pointing to a bad bounds member otherwise. "Has good bounds" means:
*
* - all type members have good bounds (except for opaque helpers)
* - all refinements of the underlying type have good bounds (except for opaque companions)
* - all type members have good bounds
* - all refinements of the underlying type have good bounds
* - all base types are class types, and if their arguments are wildcards
* they have good bounds.
* - base types do not appear in multiple instances with different arguments.
Expand Down
12 changes: 9 additions & 3 deletions compiler/src/dotty/tools/dotc/core/TypeComparer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,8 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
}
compareWild
case tp2: LazyRef =>
isBottom(tp1) || !tp2.evaluating && recur(tp1, tp2.ref)
isBottom(tp1)
|| !tp2.evaluating && recur(tp1, tp2.ref)
case CapturingType(_, _) =>
secondTry
case tp2: AnnotatedType if !tp2.isRefining =>
Expand Down Expand Up @@ -489,7 +490,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
// If `tp1` is in train of being evaluated, don't force it
// because that would cause an assertionError. Return false instead.
// See i859.scala for an example where we hit this case.
tp2.isRef(AnyClass, skipRefined = false)
tp2.isAny
|| !tp1.evaluating && recur(tp1.ref, tp2)
case AndType(tp11, tp12) =>
if tp11.stripTypeVar eq tp12.stripTypeVar then recur(tp11, tp2)
Expand Down Expand Up @@ -2133,11 +2134,16 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
// resort to reflection to invoke the member. And Java reflection needs to know exact
// erased parameter types. See neg/i12211.scala. Other reflection algorithms could
// conceivably dispatch without knowing precise parameter signatures. One can signal
// this by inheriting from the `scala.reflect.SignatureCanBeImprecise` marker trait,
// this by inheriting from the `scala.Selectable.WithoutPreciseParameterTypes` marker trait,
// in which case the signature test is elided.
// We also relax signature checking when checking bounds,
// for instance in tests/pos/i17222.izumi.min.scala
// the `go` method info as seen from `Foo` is `>: (in: Any): Unit <: (Nothing): Unit`
// So the parameter types conform but their signatures don't match.
def sigsOK(symInfo: Type, info2: Type) =
tp2.underlyingClassRef(refinementOK = true).member(name).exists
|| tp2.derivesFrom(defn.WithoutPreciseParameterTypesClass)
|| ctx.mode.is(Mode.CheckBoundsOrSelfType)
|| symInfo.isInstanceOf[MethodType]
&& symInfo.signature.consistentParams(info2.signature)

Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/TypeOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ object TypeOps:
}

def isLegalPrefix(pre: Type)(using Context): Boolean =
pre.isStable || !ctx.phase.isTyper
pre.isStable

/** Implementation of Types#simplified */
def simplify(tp: Type, theMap: SimplifyMap | Null)(using Context): Type = {
Expand Down
29 changes: 13 additions & 16 deletions compiler/src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,8 @@ object Types extends TypeUtils {
// ----- Tests -----------------------------------------------------

// // debug only: a unique identifier for a type
// val uniqId = {
// nextId = nextId + 1
// if (nextId == 19555)
// println("foo")
// nextId
// }
// val uniqId = { nextId = nextId + 1; nextId }
// if uniqId == 19555 then trace.dumpStack()

/** A cache indicating whether the type was still provisional, last time we checked */
@sharable private var mightBeProvisional = true
Expand Down Expand Up @@ -5578,24 +5574,25 @@ object Types extends TypeUtils {
}

def & (that: TypeBounds)(using Context): TypeBounds =
val lo1 = this.lo.stripLazyRef
val lo2 = that.lo.stripLazyRef
val hi1 = this.hi.stripLazyRef
val hi2 = that.hi.stripLazyRef

// This will try to preserve the FromJavaObjects type in upper bounds.
// For example, (? <: FromJavaObjects | Null) & (? <: Any),
// we want to get (? <: FromJavaObjects | Null) intead of (? <: Any),
// because we may check the result <:< (? <: Object | Null) later.
if this.hi.containsFromJavaObject
&& (this.hi frozen_<:< that.hi)
&& (that.lo frozen_<:< this.lo) then
if hi1.containsFromJavaObject && (hi1 frozen_<:< hi2) && (lo2 frozen_<:< lo1) then
// FromJavaObject in tp1.hi guarantees tp2.hi <:< tp1.hi
// prefer tp1 if FromJavaObject is in its hi
this
else if that.hi.containsFromJavaObject
&& (that.hi frozen_<:< this.hi)
&& (this.lo frozen_<:< that.lo) then
else if hi2.containsFromJavaObject && (hi2 frozen_<:< hi1) && (lo1 frozen_<:< lo2) then
// Similarly, prefer tp2 if FromJavaObject is in its hi
that
else if (this.lo frozen_<:< that.lo) && (that.hi frozen_<:< this.hi) then that
else if (that.lo frozen_<:< this.lo) && (this.hi frozen_<:< that.hi) then this
else TypeBounds(this.lo | that.lo, this.hi & that.hi)
else if (lo1 frozen_<:< lo2) && (hi2 frozen_<:< hi1) then that
else if (lo2 frozen_<:< lo1) && (hi1 frozen_<:< hi2) then this
else TypeBounds(lo1 | lo2, hi1 & hi2)

def | (that: TypeBounds)(using Context): TypeBounds =
if ((this.lo frozen_<:< that.lo) && (that.hi frozen_<:< this.hi)) this
Expand All @@ -5604,7 +5601,7 @@ object Types extends TypeUtils {

override def & (that: Type)(using Context): Type = that match {
case that: TypeBounds => this & that
case _ => super.& (that)
case _ => super.&(that)
}

override def | (that: Type)(using Context): Type = that match {
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/transform/Recheck.scala
Original file line number Diff line number Diff line change
Expand Up @@ -217,10 +217,10 @@ abstract class Recheck extends Phase, SymTransformer:
sharpen: Denotation => Denotation)(using Context): Type =
if name.is(OuterSelectName) then tree.tpe
else
//val pre = ta.maybeSkolemizePrefix(qualType, name)
val pre = ta.maybeSkolemizePrefix(qualType, name)
val mbr =
sharpen(
qualType.findMember(name, qualType,
qualType.findMember(name, pre,
excluded = if tree.symbol.is(Private) then EmptyFlags else Private
)).suchThat(tree.symbol == _)
val newType = tree.tpe match
Expand Down
1 change: 1 addition & 0 deletions compiler/test/dotc/pos-test-pickling.blacklist
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ t5031_2.scala
i16997.scala
i7414.scala
i17588.scala
i8300.scala
i9804.scala
i13433.scala
i16649-irrefutable.scala
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,8 @@ trait ClassLikeSupport:


def unwrapMemberInfo(c: ClassDef, symbol: Symbol): MemberInfo =
val baseTypeRepr = typeForClass(c).memberType(symbol)
val qualTypeRepr = if c.symbol.isClassDef then This(c.symbol).tpe else typeForClass(c)
val baseTypeRepr = qualTypeRepr.memberType(symbol)

def isSyntheticEvidence(name: String) =
if !name.startsWith(NameKinds.ContextBoundParamName.separator) then false else
Expand Down
4 changes: 2 additions & 2 deletions tests/neg/6314-6.check
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@
|object creation impossible, since def apply(fa: String): Int in trait XX in object Test3 is not defined
|(Note that
| parameter String in def apply(fa: String): Int in trait XX in object Test3 does not match
| parameter Test3.Bar[X & Object with Test3.YY {...}#Foo] in def apply(fa: Test3.Bar[X & YY.this.Foo]): Test3.Bar[Y & YY.this.Foo] in trait YY in object Test3
| parameter Test3.Bar[X & (X & Y)] in def apply(fa: Test3.Bar[X & YY.this.Foo]): Test3.Bar[Y & YY.this.Foo] in trait YY in object Test3
| )
-- Error: tests/neg/6314-6.scala:52:3 ----------------------------------------------------------------------------------
52 | (new YY {}).boom // error: object creation impossible
| ^
|object creation impossible, since def apply(fa: String): Int in trait XX in object Test4 is not defined
|(Note that
| parameter String in def apply(fa: String): Int in trait XX in object Test4 does not match
| parameter Test4.Bar[X & Object with Test4.YY {...}#FooAlias] in def apply(fa: Test4.Bar[X & YY.this.FooAlias]): Test4.Bar[Y & YY.this.FooAlias] in trait YY in object Test4
| parameter Test4.Bar[X & (X & Y)] in def apply(fa: Test4.Bar[X & YY.this.FooAlias]): Test4.Bar[Y & YY.this.FooAlias] in trait YY in object Test4
| )
2 changes: 1 addition & 1 deletion tests/neg/i6225.scala
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
object O1 {
object O1 { // error: cannot be instantiated
type A[X] = X
opaque type T = A // error: opaque type alias must be fully applied
}
Expand Down
30 changes: 30 additions & 0 deletions tests/pos/i17222.2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import scala.compiletime.*

trait Reader[-In, Out]

trait A:
type T
type F[X]
type Q = F[T]

object Reader:

given [X]: Reader[A { type Q = X }, X] with {}

object Test:

trait B[X] extends A:
type T = X

trait C extends A:
type F[X] = X

trait D[X] extends B[X] with C

val d = new D[Int] {}
val bc = new B[Int] with C

summonAll[(Reader[d.type, Int], Reader[d.type, Int])] // works
summonAll[(Reader[bc.type, Int], Reader[bc.type, Int])] // error
summonInline[Reader[d.type, Int]] // works
summonInline[Reader[bc.type, Int]] // works??
39 changes: 39 additions & 0 deletions tests/pos/i17222.3.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import scala.compiletime.*

trait Reader[-In, Out]

trait A:
type T
type F[X]
type Q = F[T]

object Reader:

given [X]: Reader[A { type Q = X }, X] with {}

object Test:

trait B[X] extends A:
type T = X

trait C extends A:
type F[X] = X

trait D[X] extends B[X] with C

val d = new D[Int] {}
val bc = new B[Int] with C

case class Box[T](value: T)

/** compiletime.summonAll, but with one case */
inline def summonOne[T <: Box[?]]: T =
val res =
inline erasedValue[T] match
case _: Box[t] => summonInline[t]
end match
Box(res).asInstanceOf[T]
end summonOne

summonOne[Box[Reader[d.type, Int]]] // works
summonOne[Box[Reader[bc.type, Int]]] // errors
71 changes: 71 additions & 0 deletions tests/pos/i17222.4.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import scala.compiletime.*

trait Reader[-In, Out]

trait A:
type T
type F[X]
type Q = F[T]

given [X]: Reader[A { type Q = X }, X] with {}

case class Box[T](x: T)

/** compiletime.summonAll, but with one case */
inline def summonOne[T]: T =
val res =
inline erasedValue[T] match
case _: Box[t] => summonInline[t]
end match
Box(res).asInstanceOf[T]
end summonOne


@main def main =


trait B[X] extends A:
type T = X

trait C extends A:
type F[X] = X


val bc = new B[Int] with C

summonOne[Box[Reader[bc.type, Int]]] // errors


val bc2: A { type Q = Int } = new B[Int] with C

summonOne[Box[Reader[bc2.type, Int]]] // works


object BC extends B[Int] with C

summonOne[Box[Reader[BC.type, Int]]] // works


val a = new A:
type T = Int
type F[X] = X

summonOne[Box[Reader[a.type, Int]]] // works


val b = new B[Int]:
type F[X] = X

summonOne[Box[Reader[b.type, Int]]] // works


val ac = new A with C:
type T = Int

summonOne[Box[Reader[ac.type, Int]]] // works


trait D[X] extends B[X] with C
val d = new D[Int] {}

summonOne[Box[Reader[d.type, Int]]] // works
26 changes: 26 additions & 0 deletions tests/pos/i17222.5.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import scala.compiletime.*

trait Reader[-In, Out]

trait A:
type T
type F[X]
type Q = F[T]

given [X]: Reader[A { type Q = X }, X] with {}

case class Box[T](x: T)

inline def summonOne[T]: T =
summonInline[T]
end summonOne

@main def main =
trait B[X] extends A:
type T = X
trait C extends A:
type F[X] = X

val bc = new B[Int] with C
summonInline[Reader[bc.type, Int]] // (I) Works
summonOne[Reader[bc.type, Int]] // (II) Errors
18 changes: 18 additions & 0 deletions tests/pos/i17222.8.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import scala.compiletime.*

trait A:
type F
type Q = F

trait Reader[-In, Out]
object Reader:
given [X]: Reader[A { type Q = X }, X] with {}

class Test:
//type BC = A { type F = Int } & A // ok
type BC = A & A { type F = Int } // fail, also ok when manually de-aliased

inline def summonOne: Unit = summonInline[Reader[BC, Int]]

def t1(): Unit = summonInline[Reader[BC, Int]] // ok
def t2(): Unit = summonOne // error
7 changes: 7 additions & 0 deletions tests/pos/i17222.izumi.min.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
class Foo:
type In
type Bar = { def go(in: In): Unit }
type False = false

class Test:
def t1: Unit = valueOf[Foo#False]
Loading

0 comments on commit 912b6f2

Please sign in to comment.