From f7b08b647d8b062f26734280c15e01d4c1eaf673 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 5 Dec 2018 09:21:38 +0100 Subject: [PATCH 1/6] Harden RefChecks I had a TypeError crash in refchecks after screwing up a typeclass encoding in a particularly bad way. This commit reports an error instead. --- compiler/src/dotty/tools/dotc/typer/RefChecks.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala index e36a6817fb91..1d84c3478d33 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala @@ -979,7 +979,7 @@ class RefChecks extends MiniPhase { thisPhase => checkAllOverrides(cls) tree } catch { - case ex: MergeError => + case ex: TypeError => ctx.error(ex.getMessage, tree.pos) tree } From b3dd07cc068d4ad6f74ba4321b325170653d3cb6 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 5 Dec 2018 10:28:11 +0100 Subject: [PATCH 2/6] Special case comparison with Any Add the rule T <: Any for any *-Type T. This was not include fully before. We did have the rule that T <: Any, if Any is in the base types of T. However, it could be that the base type wrt Any does not exist. Example: Any#L <: Any yielded false before, now yields true. This error manifested itself in i4031.scala. With the new rule, we can drop again the special case in derivedSelect. --- .../src/dotty/tools/dotc/core/TypeComparer.scala | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index 60ad1963ee29..c24ecdfaa667 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -319,7 +319,7 @@ class TypeComparer(initctx: Context) extends ConstraintHandling { thirdTry case tp1: TypeParamRef => def flagNothingBound = { - if (!frozenConstraint && tp2.isRef(defn.NothingClass) && state.isGlobalCommittable) { + if (!frozenConstraint && tp2.isRef(NothingClass) && state.isGlobalCommittable) { def msg = s"!!! instantiated to Nothing: $tp1, constraint = ${constraint.show}" if (Config.failOnInstantiationToNothing) assert(false, msg) else ctx.log(msg) @@ -404,8 +404,9 @@ class TypeComparer(initctx: Context) extends ConstraintHandling { if (cls2.isClass) { if (cls2.typeParams.isEmpty) { if (cls2 eq AnyKindClass) return true - if (tp1.isRef(defn.NothingClass)) return true + if (tp1.isRef(NothingClass)) return true if (tp1.isLambdaSub) return false + if (cls2 eq AnyClass) return true // Note: We would like to replace this by `if (tp1.hasHigherKind)` // but right now we cannot since some parts of the standard library rely on the // idiom that e.g. `List <: Any`. We have to bootstrap without scalac first. @@ -417,7 +418,7 @@ class TypeComparer(initctx: Context) extends ConstraintHandling { val base = tp1.baseType(cls2) if (base.typeSymbol == cls2) return true } - else if (tp1.isLambdaSub && !tp1.isRef(defn.AnyKindClass)) + else if (tp1.isLambdaSub && !tp1.isRef(AnyKindClass)) return recur(tp1, EtaExpansion(cls2.typeRef)) } fourthTry @@ -1382,7 +1383,7 @@ class TypeComparer(initctx: Context) extends ConstraintHandling { // at run time. It would not work to replace that with `Nothing`. // However, maybe we can still apply the replacement to // types which are not explicitly written. - defn.NothingType + NothingType case _ => andType(tp1, tp2) } case _ => andType(tp1, tp2) @@ -1393,8 +1394,7 @@ class TypeComparer(initctx: Context) extends ConstraintHandling { } /** The greatest lower bound of a list types */ - final def glb(tps: List[Type]): Type = - ((defn.AnyType: Type) /: tps)(glb) + final def glb(tps: List[Type]): Type = ((AnyType: Type) /: tps)(glb) /** The least upper bound of two types * @param canConstrain If true, new constraints might be added to simplify the lub. @@ -1424,7 +1424,7 @@ class TypeComparer(initctx: Context) extends ConstraintHandling { /** The least upper bound of a list of types */ final def lub(tps: List[Type]): Type = - ((defn.NothingType: Type) /: tps)(lub(_,_, canConstrain = false)) + ((NothingType: Type) /: tps)(lub(_,_, canConstrain = false)) /** Try to produce joint arguments for a lub `A[T_1, ..., T_n] | A[T_1', ..., T_n']` using * the following strategies: From 0c18756c52c234fb97592ab53730d56c69032f0c Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 5 Dec 2018 10:29:25 +0100 Subject: [PATCH 3/6] Clean up logic for isStable --- compiler/src/dotty/tools/dotc/core/SymDenotations.scala | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 4b6261901d5f..f87d5cf1ddc0 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -606,8 +606,10 @@ object SymDenotations { ) /** Is this a denotation of a stable term (or an arbitrary type)? */ - final def isStable(implicit ctx: Context): Boolean = - isType || is(StableOrErased) || !is(UnstableValue) && !info.isInstanceOf[ExprType] + final def isStable(implicit ctx: Context): Boolean = { + def isUnstableValue = is(UnstableValue) || info.isInstanceOf[ExprType] + isType || is(Stable) || !isUnstableValue + } /** Is this a denotation of a class that does not have - either direct or inherited - * initaliazion code? From 82a1721c1dede822fe5b6c9c44485549f7ba70a2 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 5 Dec 2018 10:33:29 +0100 Subject: [PATCH 4/6] Refinements to realizability Unstable types can still be realizable (i.e. if their underlying type is concrete with no bad bounds). --- .../tools/dotc/core/CheckRealizable.scala | 14 ++++++++---- .../src/dotty/tools/dotc/core/Types.scala | 6 +++-- .../dotty/tools/dotc/typer/TypeAssigner.scala | 2 +- tests/neg/i50-volatile.scala | 17 +++++++++----- tests/neg/realizability.scala | 22 +++++++++++++++++++ tests/pos/i4031.scala | 8 +++++++ 6 files changed, 57 insertions(+), 12 deletions(-) create mode 100644 tests/neg/realizability.scala create mode 100644 tests/pos/i4031.scala diff --git a/compiler/src/dotty/tools/dotc/core/CheckRealizable.scala b/compiler/src/dotty/tools/dotc/core/CheckRealizable.scala index 788b6d474999..0d41806ef1bd 100644 --- a/compiler/src/dotty/tools/dotc/core/CheckRealizable.scala +++ b/compiler/src/dotty/tools/dotc/core/CheckRealizable.scala @@ -73,10 +73,16 @@ class CheckRealizable(implicit ctx: Context) { if (sym.is(Stable)) realizability(tp.prefix) else { val r = - if (!sym.isStable) NotStable - else if (!isLateInitialized(sym)) Realizable - else if (!sym.isEffectivelyFinal) new NotFinal(sym) - else realizability(tp.info).mapError(r => new ProblemInUnderlying(tp.info, r)) + if (sym.isStable && !isLateInitialized(sym)) + // it's realizable because we know that a value of type `tp` has been created at run-time + Realizable + else if (!sym.isEffectivelyFinal) + // it's potentially not realizable since it might be overridden with a member of nonrealizable type + new NotFinal(sym) + else + // otherwise we need to look at the info to determine realizability + // roughly: it's realizable if the info does not have bad bounds + realizability(tp.info).mapError(r => new ProblemInUnderlying(tp, r)) r andAlso { sym.setFlag(Stable) realizability(tp.prefix) diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 576e69e790cc..afc60348bfc9 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -149,7 +149,7 @@ object Types { /** Does this type denote a stable reference (i.e. singleton type)? */ final def isStable(implicit ctx: Context): Boolean = stripTypeVar match { - case tp: TermRef => tp.termSymbol.isStable && tp.prefix.isStable || tp.info.isStable + case tp: TermRef => tp.symbol.isStable && tp.prefix.isStable || tp.info.isStable case _: SingletonType | NoPrefix => true case tp: RefinedOrRecType => tp.parent.isStable case tp: ExprType => tp.resultType.isStable @@ -4506,6 +4506,8 @@ object Types { else if (lo `eq` hi) lo else Range(lower(lo), upper(hi)) + protected def emptyRange = range(defn.NothingType, defn.AnyType) + protected def isRange(tp: Type): Boolean = tp.isInstanceOf[Range] protected def lower(tp: Type): Type = tp match { @@ -4630,7 +4632,7 @@ object Types { else tp.derivedTypeBounds(lo, hi) override protected def derivedSuperType(tp: SuperType, thistp: Type, supertp: Type): Type = - if (isRange(thistp) || isRange(supertp)) range(defn.NothingType, defn.AnyType) + if (isRange(thistp) || isRange(supertp)) emptyRange else tp.derivedSuperType(thistp, supertp) override protected def derivedAppliedType(tp: AppliedType, tycon: Type, args: List[Type]): Type = diff --git a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala index 4ca25185d907..5335a0acdd9b 100644 --- a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala +++ b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala @@ -105,7 +105,7 @@ trait TypeAssigner { case info: ClassInfo => range(defn.NothingType, apply(classBound(info))) case _ => - range(defn.NothingType, defn.AnyType) // should happen only in error cases + emptyRange // should happen only in error cases } case tp: ThisType if toAvoid(tp.cls) => range(defn.NothingType, apply(classBound(tp.cls.classInfo))) diff --git a/tests/neg/i50-volatile.scala b/tests/neg/i50-volatile.scala index fcfc9592b9a6..e67b354b5a57 100644 --- a/tests/neg/i50-volatile.scala +++ b/tests/neg/i50-volatile.scala @@ -10,16 +10,23 @@ class Test { } lazy val o: A & B = ??? - class Client extends o.Inner // old-error // old-error + class Client extends o.Inner // error - def xToString(x: o.X): String = x // old-error + def xToString(x: o.X): String = x // error def intToString(i: Int): String = xToString(i) } -object Test2 { - import Test.o._ // error +object Test2 { + trait A { + type X = String + } + trait B { + type X = Int + } + lazy val o: A & B = ??? - def xToString(x: X): String = x + def xToString(x: o.X): String = x // error + def intToString(i: Int): String = xToString(i) } diff --git a/tests/neg/realizability.scala b/tests/neg/realizability.scala new file mode 100644 index 000000000000..1730873e7692 --- /dev/null +++ b/tests/neg/realizability.scala @@ -0,0 +1,22 @@ +class C { type T } + +class Test { + + type D <: C + + lazy val a: C = ??? + final lazy val b: C = ??? + val c: D = ??? + final lazy val d: D = ??? + + val x1: a.T = ??? // error: not a legal path, since a is lazy & non-final + val x2: b.T = ??? // OK, b is lazy but concrete + val x3: c.T = ??? // OK, c is abstract but strict + val x4: d.T = ??? // error: not a legal path since d is abstract and lazy + + val y1: Singleton = a + val y2: Singleton = a + val y3: Singleton = a + val y4: Singleton = a + +} diff --git a/tests/pos/i4031.scala b/tests/pos/i4031.scala new file mode 100644 index 000000000000..faf8f9d7f256 --- /dev/null +++ b/tests/pos/i4031.scala @@ -0,0 +1,8 @@ +object App { + trait A { type L >: Any} + def upcast(a: A, x: Any): a.L = x + lazy val p: A { type L <: Nothing } = p + val q = new A { type L = Any } + def coerce1(x: Any): Any = upcast(q, x) // ok + def coerce3(x: Any): Any = upcast(p, x) // ok, since dependent result type is not needed +} From 6bf171fd6f0da9789ecfd9128ef8dd0f90fd05d4 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 5 Dec 2018 16:02:08 +0100 Subject: [PATCH 5/6] Some polishings --- compiler/src/dotty/tools/dotc/core/CheckRealizable.scala | 2 -- compiler/src/dotty/tools/dotc/core/TypeComparer.scala | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/CheckRealizable.scala b/compiler/src/dotty/tools/dotc/core/CheckRealizable.scala index 0d41806ef1bd..294ff489bd65 100644 --- a/compiler/src/dotty/tools/dotc/core/CheckRealizable.scala +++ b/compiler/src/dotty/tools/dotc/core/CheckRealizable.scala @@ -21,8 +21,6 @@ object CheckRealizable { object NotConcrete extends Realizability(" is not a concrete type") - object NotStable extends Realizability(" is not a stable reference") - class NotFinal(sym: Symbol)(implicit ctx: Context) extends Realizability(i" refers to nonfinal $sym") diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index c24ecdfaa667..bdc192bc6bd7 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -406,10 +406,10 @@ class TypeComparer(initctx: Context) extends ConstraintHandling { if (cls2 eq AnyKindClass) return true if (tp1.isRef(NothingClass)) return true if (tp1.isLambdaSub) return false - if (cls2 eq AnyClass) return true // Note: We would like to replace this by `if (tp1.hasHigherKind)` // but right now we cannot since some parts of the standard library rely on the // idiom that e.g. `List <: Any`. We have to bootstrap without scalac first. + if (cls2 eq AnyClass) return true if (cls2 == defn.SingletonClass && tp1.isStable) return true return tryBaseType(cls2) } From 483a595e25c4f77a4bd60dfb302f3ea720e595a5 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 6 Dec 2018 13:25:14 +0100 Subject: [PATCH 6/6] Be more careful setting the Stable flag --- compiler/src/dotty/tools/dotc/core/CheckRealizable.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/core/CheckRealizable.scala b/compiler/src/dotty/tools/dotc/core/CheckRealizable.scala index 294ff489bd65..57dc1bcb5af9 100644 --- a/compiler/src/dotty/tools/dotc/core/CheckRealizable.scala +++ b/compiler/src/dotty/tools/dotc/core/CheckRealizable.scala @@ -82,7 +82,7 @@ class CheckRealizable(implicit ctx: Context) { // roughly: it's realizable if the info does not have bad bounds realizability(tp.info).mapError(r => new ProblemInUnderlying(tp, r)) r andAlso { - sym.setFlag(Stable) + if (sym.isStable) sym.setFlag(Stable) // it's known to be stable and realizable realizability(tp.prefix) } }