Skip to content

Commit ad04a98

Browse files
committed
Address reviewers comments
- explain why checkRealizable better - add test case for bad bounds in basetypes - drop dead code - documentation fixes - fixes to lubArgs/glbArgs
1 parent 0de8041 commit ad04a98

File tree

9 files changed

+53
-23
lines changed

9 files changed

+53
-23
lines changed

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,9 @@ class CheckRealizable(implicit ctx: Context) {
101101
* - all type members have good bounds
102102
* - all base types are class types, and if their arguments are wildcards
103103
* they have good bounds.
104+
* - base types do not appear in multiple instances with different arguments.
105+
* (depending on the simplification scheme for AndTypes employed, this could
106+
* also lead to base types with bad bounds).
104107
*/
105108
private def boundsRealizability(tp: Type) = {
106109
val mbrProblems =
@@ -109,7 +112,7 @@ class CheckRealizable(implicit ctx: Context) {
109112
if !(mbr.info.loBound <:< mbr.info.hiBound)
110113
}
111114
yield new HasProblemBounds(mbr)
112-
115+
113116
def baseTypeProblems(base: Type) = base match {
114117
case AndType(base1, base2) =>
115118
new HasProblemBase(base1, base2) :: Nil
@@ -121,7 +124,7 @@ class CheckRealizable(implicit ctx: Context) {
121124
}
122125
val baseProblems =
123126
tp.baseClasses.map(_.baseTypeOf(tp)).flatMap(baseTypeProblems)
124-
127+
125128
(((Realizable: Realizability)
126129
/: mbrProblems)(_ andAlso _)
127130
/: baseProblems)(_ andAlso _)

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -194,13 +194,12 @@ trait ConstraintHandling {
194194
final def approximation(param: TypeParamRef, fromBelow: Boolean): Type = {
195195
val avoidParam = new TypeMap {
196196
override def stopAtStatic = true
197-
def avoidInArg(arg: Type, formal: TypeParamInfo): Type =
197+
def avoidInArg(arg: Type): Type =
198198
if (param.occursIn(arg)) TypeBounds.empty else arg
199199
def apply(tp: Type) = mapOver {
200200
tp match {
201201
case tp @ AppliedType(tycon, args) =>
202-
tp.derivedAppliedType(tycon,
203-
args.zipWithConserve(tycon.typeParams)(avoidInArg))
202+
tp.derivedAppliedType(tycon, args.mapConserve(avoidInArg))
204203
case tp: RefinedType if param occursIn tp.refinedInfo =>
205204
tp.parent
206205
case tp: WildcardType =>

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,6 @@ class Definitions {
7878
val cls = denot.asClass.classSymbol
7979
val paramDecls = newScope
8080
val typeParam = enterSyntheticTypeParam(cls, paramFlags, paramDecls)
81-
def instantiate(tpe: Type) =
82-
if (tpe.typeParams.nonEmpty) tpe.appliedTo(typeParam.typeRef)
83-
else tpe.dealias
8481
val parents = parentConstrs.toList
8582
denot.info = ClassInfo(ScalaPackageClass.thisType, cls, parents, paramDecls)
8683
}

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -644,12 +644,13 @@ object Denotations {
644644
// ------ Forming types -------------------------------------------
645645

646646
/** The TypeRef representing this type denotation at its original location. */
647-
def appliedRef(implicit ctx: Context): Type =
648-
typeRef.appliedTo(symbol.typeParams.map(_.typeRef))
649-
650647
def typeRef(implicit ctx: Context): TypeRef =
651648
TypeRef(symbol.owner.thisType, symbol.name.asTypeName, this)
652649

650+
/** The typeRef applied to its own type parameters */
651+
def appliedRef(implicit ctx: Context): Type =
652+
typeRef.appliedTo(symbol.typeParams.map(_.typeRef))
653+
653654
/** The TermRef representing this term denotation at its original location. */
654655
def termRef(implicit ctx: Context): TermRef =
655656
TermRef(symbol.owner.thisType, symbol.name.asTermName, this)

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

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,10 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
337337
case tp1 @ OrType(tp11, tp12) =>
338338
def joinOK = tp2.dealias match {
339339
case tp2: AppliedType if !tp2.tycon.typeSymbol.isClass =>
340+
// If we apply the default algorithm for `A[X] | B[Y] <: C[Z]` where `C` is a
341+
// type parameter, we will instantiate `C` to `A` and then fail when comparing
342+
// with `B[Y]`. To do the right thing, we need to instantiate `C` to the
343+
// common superclass of `A` and `B`.
340344
isSubType(tp1.join, tp2)
341345
case _ =>
342346
false
@@ -366,7 +370,9 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
366370
if (cls2.isClass) {
367371
val base = tp1.baseType(cls2)
368372
if (base.exists) {
369-
if (cls2.is(JavaDefined)) return base.typeSymbol == cls2
373+
if (cls2.is(JavaDefined))
374+
// If `cls2` is parameterized, we are seeing a raw type, so we need to compare only the symbol
375+
return base.typeSymbol == cls2
370376
if (base ne tp1) return isSubType(base, tp2)
371377
}
372378
if (cls2 == defn.SingletonClass && tp1.isStable) return true
@@ -689,14 +695,14 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
689695
*
690696
* Returns `true` iff `d >= 0` and `tycon2` can be instantiated to
691697
*
692-
* [tparams1_d, ... tparams1_n-1] -> tycon1a[args_1, ..., args_d-1, tparams_d, ... tparams_n-1]
698+
* [tparams1_d, ... tparams1_n-1] -> tycon1[args_1, ..., args_d-1, tparams_d, ... tparams_n-1]
693699
*
694700
* such that the resulting type application is a supertype of `tp1`.
695701
*/
696702
def appOK(tp1base: Type) = tp1base match {
697703
case tp1base: AppliedType =>
698704
var tycon1 = tp1base.tycon
699-
var args1 = tp1base.args
705+
val args1 = tp1base.args
700706
val tparams1all = tycon1.typeParams
701707
val lengthDiff = tparams1all.length - tparams.length
702708
lengthDiff >= 0 && {
@@ -1244,14 +1250,22 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
12441250
final def lub(tps: List[Type]): Type =
12451251
((defn.NothingType: Type) /: tps)(lub(_,_, canConstrain = false))
12461252

1253+
/** Try to produce joint arguments for a lub `A[T_1, ..., T_n] | A[T_1', ..., T_n']` using
1254+
* the following strategies:
1255+
*
1256+
* - if arguments are the same, that argument.
1257+
* - if corresponding parameter variance is co/contra-variant, the lub/glb.
1258+
* - otherwise a TypeBounds containing both arguments
1259+
*/
12471260
def lubArgs(args1: List[Type], args2: List[Type], tparams: List[TypeParamInfo], canConstrain: Boolean = false): List[Type] =
12481261
tparams match {
12491262
case tparam :: tparamsRest =>
12501263
val arg1 :: args1Rest = args1
12511264
val arg2 :: args2Rest = args2
12521265
val v = tparam.paramVariance
12531266
val lubArg =
1254-
if (v > 0) lub(arg1.hiBound, arg2.hiBound, canConstrain)
1267+
if (isSameTypeWhenFrozen(arg1, arg2)) arg1
1268+
else if (v > 0) lub(arg1.hiBound, arg2.hiBound, canConstrain)
12551269
else if (v < 0) glb(arg1.loBound, arg2.loBound)
12561270
else TypeBounds(glb(arg1.loBound, arg2.loBound),
12571271
lub(arg1.hiBound, arg2.hiBound, canConstrain))
@@ -1263,8 +1277,8 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
12631277
/** Try to produce joint arguments for a glb `A[T_1, ..., T_n] & A[T_1', ..., T_n']` using
12641278
* the following strategies:
12651279
*
1266-
* - if corresponding parameter variance is co/contra-variant, the glb/lub.
12671280
* - if arguments are the same, that argument.
1281+
* - if corresponding parameter variance is co/contra-variant, the glb/lub.
12681282
* - if at least one of the arguments if a TypeBounds, the union of
12691283
* the bounds.
12701284
* - if homogenizeArgs is set, and arguments can be unified by instantiating
@@ -1435,7 +1449,8 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
14351449
private def distributeAnd(tp1: Type, tp2: Type): Type = tp1 match {
14361450
case tp1 @ AppliedType(tycon1, args1) =>
14371451
tp2 match {
1438-
case AppliedType(tycon2, args2) if tycon1.typeSymbol == tycon2.typeSymbol =>
1452+
case AppliedType(tycon2, args2)
1453+
if tycon1.typeSymbol == tycon2.typeSymbol && tycon1 =:= tycon2 =>
14391454
val jointArgs = glbArgs(args1, args2, tycon1.typeParams)
14401455
if (jointArgs.forall(_.exists)) (tycon1 & tycon2).appliedTo(jointArgs)
14411456
else NoType

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1032,9 +1032,7 @@ object Types {
10321032
case tp: AnnotatedType =>
10331033
tp.underlying.underlyingClassRef(refinementOK)
10341034
case tp: RefinedType =>
1035-
def isParamName = tp.classSymbol.typeParams.exists(_.name == tp.refinedName)
1036-
if (refinementOK || isParamName) tp.underlying.underlyingClassRef(refinementOK)
1037-
else NoType
1035+
if (refinementOK) tp.underlying.underlyingClassRef(refinementOK) else NoType
10381036
case tp: RecType =>
10391037
tp.underlying.underlyingClassRef(refinementOK)
10401038
case _ =>
@@ -3498,8 +3496,8 @@ object Types {
34983496
/** Type bounds >: lo <: hi */
34993497
abstract case class TypeBounds(lo: Type, hi: Type) extends CachedProxyType with TypeType {
35003498

3501-
assert(lo.isInstanceOf[TermType])
3502-
assert(hi.isInstanceOf[TermType])
3499+
assert(lo.isInstanceOf[TermType], lo)
3500+
assert(hi.isInstanceOf[TermType], hi)
35033501

35043502
override def underlying(implicit ctx: Context): Type = hi
35053503

compiler/src/dotty/tools/dotc/typer/Checking.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -554,7 +554,7 @@ trait Checking {
554554
case _ =>
555555
ctx.error(ex"$tp is not a class type", pos)
556556
defn.ObjectType
557-
}
557+
}
558558

559559
/** Check that a non-implicit parameter making up the first parameter section of an
560560
* implicit conversion is not a singleton type.

tests/neg/wildbase.scala

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
class A[T]
2+
3+
class B extends A[_] // OK
4+
5+
class C extends A[_ >: Any <: Nothing] // error: conflicting bounds

tests/pos/andtypes.scala

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
class Outer {
2+
type Foo[X]
3+
}
4+
5+
object Test {
6+
def foo[T](x: T) = x
7+
8+
def bla[A <: Outer, B <: Outer](a: A, b: B) = {
9+
val x: a.Foo[Int] & b.Foo[Int] = ???
10+
foo(x)
11+
}
12+
}

0 commit comments

Comments
 (0)