From ac10324cf9f88bca0534b56053cc87de2120fba3 Mon Sep 17 00:00:00 2001 From: noti0na1 Date: Tue, 23 Nov 2021 14:14:07 -0500 Subject: [PATCH 1/7] Relax overriding by stripping nulls deeply --- .../tools/dotc/core/NullOpsDecorator.scala | 77 +++++++++++++------ .../src/dotty/tools/dotc/core/Types.scala | 4 +- .../dotc/transform/OverridingPairs.scala | 24 +++--- .../tools/dotc/transform/ResolveSuper.scala | 13 ++-- .../explicit-nulls/neg/opaque-nullable.scala | 47 +++++++++++ .../explicit-nulls/pos/opaque-nullable.scala | 26 ------- .../pos/override-type-params.scala | 18 +++++ 7 files changed, 143 insertions(+), 66 deletions(-) create mode 100644 tests/explicit-nulls/neg/opaque-nullable.scala delete mode 100644 tests/explicit-nulls/pos/opaque-nullable.scala create mode 100644 tests/explicit-nulls/pos/override-type-params.scala diff --git a/compiler/src/dotty/tools/dotc/core/NullOpsDecorator.scala b/compiler/src/dotty/tools/dotc/core/NullOpsDecorator.scala index d0799ca89d24..9d39186430ca 100644 --- a/compiler/src/dotty/tools/dotc/core/NullOpsDecorator.scala +++ b/compiler/src/dotty/tools/dotc/core/NullOpsDecorator.scala @@ -9,6 +9,41 @@ import Types._ /** Defines operations on nullable types and tree. */ object NullOpsDecorator: + private class StripNullsMap(isDeep: Boolean)(using Context) extends TypeMap: + def strip(tp: Type): Type = tp match + case tp @ OrType(lhs, rhs) => + val llhs = this(lhs) + val rrhs = this(rhs) + if rrhs.isNullType then llhs + else if llhs.isNullType then rrhs + else derivedOrType(tp, llhs, rrhs) + case tp @ AndType(tp1, tp2) => + // We cannot `tp.derivedAndType(strip(tp1), strip(tp2))` directly, + // since `stripNull((A | Null) & B)` would produce the wrong + // result `(A & B) | Null`. + val tp1s = this(tp1) + val tp2s = this(tp2) + if isDeep || (tp1s ne tp1) && (tp2s ne tp2) then + derivedAndType(tp, tp1s, tp2s) + else tp + case tp: TypeBounds => + mapOver(tp) + case _ => tp + + def stripOver(tp: Type): Type = tp match + case appTp @ AppliedType(tycon, targs) => + derivedAppliedType(appTp, tycon, targs.map(this)) + case ptp: PolyType => + derivedLambdaType(ptp)(ptp.paramInfos, this(ptp.resType)) + case mtp: MethodType => + mapOver(mtp) + case _ => strip(tp) + + override def apply(tp: Type): Type = + if isDeep then stripOver(tp) else strip(tp) + + end StripNullsMap + extension (self: Type) /** Syntactically strips the nullability from this type. * If the type is `T1 | ... | Tn`, and `Ti` references to `Null`, @@ -17,31 +52,11 @@ object NullOpsDecorator: * The type will not be changed if explicit-nulls is not enabled. */ def stripNull(using Context): Type = { - def strip(tp: Type): Type = - val tpWiden = tp.widenDealias - val tpStripped = tpWiden match { - case tp @ OrType(lhs, rhs) => - val llhs = strip(lhs) - val rrhs = strip(rhs) - if rrhs.isNullType then llhs - else if llhs.isNullType then rrhs - else tp.derivedOrType(llhs, rrhs) - case tp @ AndType(tp1, tp2) => - // We cannot `tp.derivedAndType(strip(tp1), strip(tp2))` directly, - // since `stripNull((A | Null) & B)` would produce the wrong - // result `(A & B) | Null`. - val tp1s = strip(tp1) - val tp2s = strip(tp2) - if (tp1s ne tp1) && (tp2s ne tp2) then - tp.derivedAndType(tp1s, tp2s) - else tp - case tp @ TypeBounds(lo, hi) => - tp.derivedTypeBounds(strip(lo), strip(hi)) - case tp => tp - } - if tpStripped ne tpWiden then tpStripped else tp - - if ctx.explicitNulls then strip(self) else self + if ctx.explicitNulls then + val selfw = self.widenDealias + val selfws = new StripNullsMap(false)(selfw) + if selfws ne selfw then selfws else self + else self } /** Is self (after widening and dealiasing) a type of the form `T | Null`? */ @@ -49,6 +64,18 @@ object NullOpsDecorator: val stripped = self.stripNull stripped ne self } + + /** Strips nulls from this type deeply. + * Compaired to `stripNull`, `stripNullsDeep` will apply `stripNull` to + * each member of function types as well. + */ + def stripNullsDeep(using Context): Type = + if ctx.explicitNulls then + val selfw = self.widenDealias + val selfws = new StripNullsMap(true)(selfw) + if selfws ne selfw then selfws else self + else self + end extension import ast.tpd._ diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index b4a6172a6659..05b48cc4fc3e 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -1112,8 +1112,10 @@ object Types { */ def matches(that: Type)(using Context): Boolean = { record("matches") + val thisTp1 = this.stripNullsDeep + val thatTp1 = that.stripNullsDeep withoutMode(Mode.SafeNulls)( - TypeComparer.matchesType(this, that, relaxed = !ctx.phase.erasedTypes)) + TypeComparer.matchesType(thisTp1, thatTp1, relaxed = !ctx.phase.erasedTypes)) } /** This is the same as `matches` except that it also matches => T with T and diff --git a/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala b/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala index 437dfea9f156..28ed037405b2 100644 --- a/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala +++ b/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala @@ -5,6 +5,7 @@ package transform import core._ import Flags._, Symbols._, Contexts._, Scopes._, Decorators._, Types.Type import NameKinds.DefaultGetterName +import NullOpsDecorator._ import collection.mutable import collection.immutable.BitSet import scala.annotation.tailrec @@ -215,15 +216,20 @@ object OverridingPairs: } ) else - // releaxed override check for explicit nulls if one of the symbols is Java defined, - // force `Null` being a subtype of reference types during override checking - val relaxedCtxForNulls = + def matchNullaryLoosely = member.matchNullaryLoosely || other.matchNullaryLoosely || fallBack + // default getters are not checked for compatibility + member.name.is(DefaultGetterName) || { if ctx.explicitNulls && (member.is(JavaDefined) || other.is(JavaDefined)) then - ctx.retractMode(Mode.SafeNulls) - else ctx - member.name.is(DefaultGetterName) // default getters are not checked for compatibility - || memberTp.overrides(otherTp, - member.matchNullaryLoosely || other.matchNullaryLoosely || fallBack - )(using relaxedCtxForNulls) + // releaxed override check for explicit nulls if one of the symbols is Java defined, + // force `Null` being a subtype of reference types during override checking. + // `stripNullsDeep` is used here because we may encounter type parameters + // (`T | Null` is not a subtype of `T` even if we retract Mode.SafeNulls). + val memberTp1 = memberTp.stripNullsDeep + val otherTp1 = otherTp.stripNullsDeep + withoutMode(Mode.SafeNulls)( + memberTp1.overrides(otherTp1, matchNullaryLoosely)) + else + memberTp.overrides(otherTp, matchNullaryLoosely) + } end OverridingPairs diff --git a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala index 2a4a775b834f..cc754f91f287 100644 --- a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala +++ b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala @@ -13,6 +13,7 @@ import Names._ import StdNames._ import NameOps._ import NameKinds._ +import NullOpsDecorator._ import ResolveSuper._ import reporting.IllegalSuperAccessor @@ -113,11 +114,13 @@ object ResolveSuper { // Since the super class can be Java defined, // we use releaxed overriding check for explicit nulls if one of the symbols is Java defined. // This forces `Null` being a subtype of reference types during override checking. - val relaxedCtxForNulls = - if ctx.explicitNulls && (sym.is(JavaDefined) || acc.is(JavaDefined)) then - ctx.retractMode(Mode.SafeNulls) - else ctx - if (!(otherTp.overrides(accTp, matchLoosely = true)(using relaxedCtxForNulls))) + val overridesSuper = if ctx.explicitNulls && (sym.is(JavaDefined) || acc.is(JavaDefined)) then + val otherTp1 = otherTp.stripNullsDeep + val accTp1 = accTp.stripNullsDeep + withoutMode(Mode.SafeNulls)(otherTp1.overrides(accTp1, matchLoosely = true)) + else + otherTp.overrides(accTp, matchLoosely = true) + if !overridesSuper then report.error(IllegalSuperAccessor(base, memberName, targetName, acc, accTp, other.symbol, otherTp), base.srcPos) bcs = bcs.tail diff --git a/tests/explicit-nulls/neg/opaque-nullable.scala b/tests/explicit-nulls/neg/opaque-nullable.scala new file mode 100644 index 000000000000..1d3a22249d29 --- /dev/null +++ b/tests/explicit-nulls/neg/opaque-nullable.scala @@ -0,0 +1,47 @@ +// Unboxed option type using unions + null + opaque. +// Relies on the fact that Null is not a subtype of AnyRef. +// Test suggested by Sébastien Doeraene. + +object Nullables { + opaque type Nullable[+A <: AnyRef] = A | Null // disjoint by construction! + + object Nullable: + def apply[A <: AnyRef](x: A | Null): Nullable[A] = x + + def some[A <: AnyRef](x: A): Nullable[A] = x + def none: Nullable[Nothing] = null + + extension [A <: AnyRef](x: Nullable[A]) + def isEmpty: Boolean = x == null + def get: A | Null = x + + extension [A <: AnyRef, B <: AnyRef](x: Nullable[A]) + def flatMap(f: A => Nullable[B]): Nullable[B] = + if (x == null) null + else f(x) + + def map(f: A => B): Nullable[B] = x.flatMap(f) + + def test1 = + val s1: Nullable[String] = Nullable("hello") + val s2: Nullable[String] = "world" + val s3: Nullable[String] = Nullable.none + val s4: Nullable[String] = null + + s1.isEmpty + s1.flatMap((x) => true) + + assert(s2 != null) +} + +def test2 = + import Nullables._ + + val s1: Nullable[String] = Nullable("hello") + val s2: Nullable[String] = Nullable.none + val s3: Nullable[String] = null // error: don't leak nullable union + + s1.isEmpty + s1.flatMap((x) => Nullable(true)) + + assert(s2 == null) // error diff --git a/tests/explicit-nulls/pos/opaque-nullable.scala b/tests/explicit-nulls/pos/opaque-nullable.scala deleted file mode 100644 index a7f626054ad3..000000000000 --- a/tests/explicit-nulls/pos/opaque-nullable.scala +++ /dev/null @@ -1,26 +0,0 @@ -// Unboxed option type using unions + null + opaque. -// Relies on the fact that Null is not a subtype of AnyRef. -// Test suggested by Sébastien Doeraene. - -opaque type Nullable[+A <: AnyRef] = A | Null // disjoint by construction! - -object Nullable { - def apply[A <: AnyRef](x: A | Null): Nullable[A] = x - - def some[A <: AnyRef](x: A): Nullable[A] = x - def none: Nullable[Nothing] = null - - extension [A <: AnyRef](x: Nullable[A]) - def isEmpty: Boolean = x == null - - extension [A <: AnyRef, B <: AnyRef](x: Nullable[A]) - def flatMap(f: A => Nullable[B]): Nullable[B] = - if (x == null) null - else f(x) - - val s1: Nullable[String] = "hello" - val s2: Nullable[String] = null - - s1.isEmpty - s1.flatMap((x) => true) -} diff --git a/tests/explicit-nulls/pos/override-type-params.scala b/tests/explicit-nulls/pos/override-type-params.scala new file mode 100644 index 000000000000..7f59409a4c3c --- /dev/null +++ b/tests/explicit-nulls/pos/override-type-params.scala @@ -0,0 +1,18 @@ +// Testing relaxed overriding check for explicit nulls. +// The relaxed check is only enabled if one of the members is Java defined. + +import java.util.Comparator + +class C1[T <: AnyRef] extends Ordering[T]: + override def compare(o1: T, o2: T): Int = 0 + +// The following overriding is not allowed, because `compare` +// has already been declared in Scala class `Ordering`. +// class C2[T <: AnyRef] extends Ordering[T]: +// override def compare(o1: T | Null, o2: T | Null): Int = 0 + +class D1[T <: AnyRef] extends Comparator[T]: + override def compare(o1: T, o2: T): Int = 0 + +class D2[T <: AnyRef] extends Comparator[T]: + override def compare(o1: T | Null, o2: T | Null): Int = 0 From 1aafe30ed0a4a8b3a89a512bddcf3d4793060f3d Mon Sep 17 00:00:00 2001 From: noti0na1 Date: Tue, 23 Nov 2021 14:17:51 -0500 Subject: [PATCH 2/7] move widenDealias --- .../dotty/tools/dotc/core/NullOpsDecorator.scala | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/NullOpsDecorator.scala b/compiler/src/dotty/tools/dotc/core/NullOpsDecorator.scala index 9d39186430ca..5c6d90fac2f8 100644 --- a/compiler/src/dotty/tools/dotc/core/NullOpsDecorator.scala +++ b/compiler/src/dotty/tools/dotc/core/NullOpsDecorator.scala @@ -40,7 +40,9 @@ object NullOpsDecorator: case _ => strip(tp) override def apply(tp: Type): Type = - if isDeep then stripOver(tp) else strip(tp) + val tpw = tp.widenDealias + val tpws = if isDeep then stripOver(tpw) else strip(tpw) + if tpws ne tpw then tpws else tp end StripNullsMap @@ -52,11 +54,7 @@ object NullOpsDecorator: * The type will not be changed if explicit-nulls is not enabled. */ def stripNull(using Context): Type = { - if ctx.explicitNulls then - val selfw = self.widenDealias - val selfws = new StripNullsMap(false)(selfw) - if selfws ne selfw then selfws else self - else self + if ctx.explicitNulls then new StripNullsMap(false)(self) else self } /** Is self (after widening and dealiasing) a type of the form `T | Null`? */ @@ -70,11 +68,7 @@ object NullOpsDecorator: * each member of function types as well. */ def stripNullsDeep(using Context): Type = - if ctx.explicitNulls then - val selfw = self.widenDealias - val selfws = new StripNullsMap(true)(selfw) - if selfws ne selfw then selfws else self - else self + if ctx.explicitNulls then new StripNullsMap(true)(self) else self end extension From 8a099ffd0567d9f48260e5a1138105e5b1dc2304 Mon Sep 17 00:00:00 2001 From: noti0na1 Date: Fri, 4 Feb 2022 02:22:53 -0500 Subject: [PATCH 3/7] Testing new approach --- compiler/src/dotty/tools/dotc/core/Mode.scala | 2 + .../tools/dotc/core/NullOpsDecorator.scala | 71 +++++++------------ .../dotty/tools/dotc/core/TypeComparer.scala | 16 +++-- .../src/dotty/tools/dotc/core/Types.scala | 8 +-- .../dotc/transform/OverridingPairs.scala | 10 +-- .../tools/dotc/transform/ResolveSuper.scala | 5 +- 6 files changed, 45 insertions(+), 67 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Mode.scala b/compiler/src/dotty/tools/dotc/core/Mode.scala index 9f5b8a9a1c05..2379cd804727 100644 --- a/compiler/src/dotty/tools/dotc/core/Mode.scala +++ b/compiler/src/dotty/tools/dotc/core/Mode.scala @@ -124,4 +124,6 @@ object Mode { * This mode forces expansion of inline calls in those positions even during typing. */ val ForceInline: Mode = newMode(29, "ForceInline") + + val RelaxedOverriding: Mode = newMode(30, "ForceInline") } diff --git a/compiler/src/dotty/tools/dotc/core/NullOpsDecorator.scala b/compiler/src/dotty/tools/dotc/core/NullOpsDecorator.scala index 5c6d90fac2f8..d0799ca89d24 100644 --- a/compiler/src/dotty/tools/dotc/core/NullOpsDecorator.scala +++ b/compiler/src/dotty/tools/dotc/core/NullOpsDecorator.scala @@ -9,43 +9,6 @@ import Types._ /** Defines operations on nullable types and tree. */ object NullOpsDecorator: - private class StripNullsMap(isDeep: Boolean)(using Context) extends TypeMap: - def strip(tp: Type): Type = tp match - case tp @ OrType(lhs, rhs) => - val llhs = this(lhs) - val rrhs = this(rhs) - if rrhs.isNullType then llhs - else if llhs.isNullType then rrhs - else derivedOrType(tp, llhs, rrhs) - case tp @ AndType(tp1, tp2) => - // We cannot `tp.derivedAndType(strip(tp1), strip(tp2))` directly, - // since `stripNull((A | Null) & B)` would produce the wrong - // result `(A & B) | Null`. - val tp1s = this(tp1) - val tp2s = this(tp2) - if isDeep || (tp1s ne tp1) && (tp2s ne tp2) then - derivedAndType(tp, tp1s, tp2s) - else tp - case tp: TypeBounds => - mapOver(tp) - case _ => tp - - def stripOver(tp: Type): Type = tp match - case appTp @ AppliedType(tycon, targs) => - derivedAppliedType(appTp, tycon, targs.map(this)) - case ptp: PolyType => - derivedLambdaType(ptp)(ptp.paramInfos, this(ptp.resType)) - case mtp: MethodType => - mapOver(mtp) - case _ => strip(tp) - - override def apply(tp: Type): Type = - val tpw = tp.widenDealias - val tpws = if isDeep then stripOver(tpw) else strip(tpw) - if tpws ne tpw then tpws else tp - - end StripNullsMap - extension (self: Type) /** Syntactically strips the nullability from this type. * If the type is `T1 | ... | Tn`, and `Ti` references to `Null`, @@ -54,7 +17,31 @@ object NullOpsDecorator: * The type will not be changed if explicit-nulls is not enabled. */ def stripNull(using Context): Type = { - if ctx.explicitNulls then new StripNullsMap(false)(self) else self + def strip(tp: Type): Type = + val tpWiden = tp.widenDealias + val tpStripped = tpWiden match { + case tp @ OrType(lhs, rhs) => + val llhs = strip(lhs) + val rrhs = strip(rhs) + if rrhs.isNullType then llhs + else if llhs.isNullType then rrhs + else tp.derivedOrType(llhs, rrhs) + case tp @ AndType(tp1, tp2) => + // We cannot `tp.derivedAndType(strip(tp1), strip(tp2))` directly, + // since `stripNull((A | Null) & B)` would produce the wrong + // result `(A & B) | Null`. + val tp1s = strip(tp1) + val tp2s = strip(tp2) + if (tp1s ne tp1) && (tp2s ne tp2) then + tp.derivedAndType(tp1s, tp2s) + else tp + case tp @ TypeBounds(lo, hi) => + tp.derivedTypeBounds(strip(lo), strip(hi)) + case tp => tp + } + if tpStripped ne tpWiden then tpStripped else tp + + if ctx.explicitNulls then strip(self) else self } /** Is self (after widening and dealiasing) a type of the form `T | Null`? */ @@ -62,14 +49,6 @@ object NullOpsDecorator: val stripped = self.stripNull stripped ne self } - - /** Strips nulls from this type deeply. - * Compaired to `stripNull`, `stripNullsDeep` will apply `stripNull` to - * each member of function types as well. - */ - def stripNullsDeep(using Context): Type = - if ctx.explicitNulls then new StripNullsMap(true)(self) else self - end extension import ast.tpd._ diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index 8b4eab685f2a..ddf7fe8268bf 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -766,13 +766,15 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling isSubType(hi1, tp2, approx.addLow) || compareGADT || tryLiftedToThis1 case _ => - def isNullable(tp: Type): Boolean = tp.widenDealias match { - case tp: TypeRef => tp.symbol.isNullableClass - case tp: RefinedOrRecType => isNullable(tp.parent) - case tp: AppliedType => isNullable(tp.tycon) - case AndType(tp1, tp2) => isNullable(tp1) && isNullable(tp2) - case OrType(tp1, tp2) => isNullable(tp1) || isNullable(tp2) - case _ => false + def isNullable(tp: Type): Boolean = ctx.mode.is(Mode.RelaxedOverriding) || { + tp.widenDealias match { + case tp: TypeRef => tp.symbol.isNullableClass + case tp: RefinedOrRecType => isNullable(tp.parent) + case tp: AppliedType => isNullable(tp.tycon) + case AndType(tp1, tp2) => isNullable(tp1) && isNullable(tp2) + case OrType(tp1, tp2) => isNullable(tp1) || isNullable(tp2) + case _ => false + } } val sym1 = tp1.symbol (sym1 eq NothingClass) && tp2.isValueTypeOrLambda || diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 05b48cc4fc3e..70d1ea29c1a8 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -1112,10 +1112,10 @@ object Types { */ def matches(that: Type)(using Context): Boolean = { record("matches") - val thisTp1 = this.stripNullsDeep - val thatTp1 = that.stripNullsDeep - withoutMode(Mode.SafeNulls)( - TypeComparer.matchesType(thisTp1, thatTp1, relaxed = !ctx.phase.erasedTypes)) + val overrideCtx = if ctx.explicitNulls + then ctx.retractMode(Mode.SafeNulls).addMode(Mode.RelaxedOverriding) + else ctx + TypeComparer.matchesType(this, that, relaxed = !ctx.phase.erasedTypes)(using overrideCtx) } /** This is the same as `matches` except that it also matches => T with T and diff --git a/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala b/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala index 28ed037405b2..bee66f2e2a17 100644 --- a/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala +++ b/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala @@ -221,13 +221,9 @@ object OverridingPairs: member.name.is(DefaultGetterName) || { if ctx.explicitNulls && (member.is(JavaDefined) || other.is(JavaDefined)) then // releaxed override check for explicit nulls if one of the symbols is Java defined, - // force `Null` being a subtype of reference types during override checking. - // `stripNullsDeep` is used here because we may encounter type parameters - // (`T | Null` is not a subtype of `T` even if we retract Mode.SafeNulls). - val memberTp1 = memberTp.stripNullsDeep - val otherTp1 = otherTp.stripNullsDeep - withoutMode(Mode.SafeNulls)( - memberTp1.overrides(otherTp1, matchNullaryLoosely)) + // force `Null` being a bottom types during override checking. + val overrideCtx = ctx.retractMode(Mode.SafeNulls).addMode(Mode.RelaxedOverriding) + memberTp.overrides(otherTp, matchNullaryLoosely)(using overrideCtx) else memberTp.overrides(otherTp, matchNullaryLoosely) } diff --git a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala index cc754f91f287..000d3ee846ff 100644 --- a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala +++ b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala @@ -115,9 +115,8 @@ object ResolveSuper { // we use releaxed overriding check for explicit nulls if one of the symbols is Java defined. // This forces `Null` being a subtype of reference types during override checking. val overridesSuper = if ctx.explicitNulls && (sym.is(JavaDefined) || acc.is(JavaDefined)) then - val otherTp1 = otherTp.stripNullsDeep - val accTp1 = accTp.stripNullsDeep - withoutMode(Mode.SafeNulls)(otherTp1.overrides(accTp1, matchLoosely = true)) + val overrideCtx = ctx.retractMode(Mode.SafeNulls).addMode(Mode.RelaxedOverriding) + otherTp.overrides(accTp, matchLoosely = true)(using overrideCtx) else otherTp.overrides(accTp, matchLoosely = true) if !overridesSuper then From 9b10efd97590ed61f709fe5b7fac152c979f1d7e Mon Sep 17 00:00:00 2001 From: noti0na1 Date: Wed, 9 Feb 2022 19:47:10 -0500 Subject: [PATCH 4/7] Add comments --- compiler/src/dotty/tools/dotc/core/Mode.scala | 5 ++++- .../dotty/tools/dotc/core/TypeComparer.scala | 9 +++++++-- .../dotc/transform/OverridingPairs.scala | 19 ++++++++----------- .../tools/dotc/transform/ResolveSuper.scala | 12 ++++-------- 4 files changed, 23 insertions(+), 22 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Mode.scala b/compiler/src/dotty/tools/dotc/core/Mode.scala index 2379cd804727..7a7d50ac0d33 100644 --- a/compiler/src/dotty/tools/dotc/core/Mode.scala +++ b/compiler/src/dotty/tools/dotc/core/Mode.scala @@ -125,5 +125,8 @@ object Mode { */ val ForceInline: Mode = newMode(29, "ForceInline") - val RelaxedOverriding: Mode = newMode(30, "ForceInline") + /** This mode is enabled when we check Java overriding in explicit nulls. + * Type `Null` becomes a bottom type in TypeComparer. + */ + val RelaxedOverriding: Mode = newMode(30, "RelaxedOverriding") } diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index ddf7fe8268bf..c5c25cafc085 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -766,15 +766,20 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling isSubType(hi1, tp2, approx.addLow) || compareGADT || tryLiftedToThis1 case _ => + // `Mode.RelaxedOverriding` is only enabled when checking Java overriding + // in explicit nulls, and `Null` becomes a bottom type, which allows + // `T | Null` being a subtype of `T`. + // A type varibale `T` from Java is translated to `T >: Nothing <: Any`. + // However, `null` can always be a value of `T` for Java side. + // So the best solution here is to let `Null` be bottom type temporarily. def isNullable(tp: Type): Boolean = ctx.mode.is(Mode.RelaxedOverriding) || { - tp.widenDealias match { + tp.widenDealias match case tp: TypeRef => tp.symbol.isNullableClass case tp: RefinedOrRecType => isNullable(tp.parent) case tp: AppliedType => isNullable(tp.tycon) case AndType(tp1, tp2) => isNullable(tp1) && isNullable(tp2) case OrType(tp1, tp2) => isNullable(tp1) || isNullable(tp2) case _ => false - } } val sym1 = tp1.symbol (sym1 eq NothingClass) && tp2.isValueTypeOrLambda || diff --git a/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala b/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala index bee66f2e2a17..f67f11cbf920 100644 --- a/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala +++ b/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala @@ -216,16 +216,13 @@ object OverridingPairs: } ) else - def matchNullaryLoosely = member.matchNullaryLoosely || other.matchNullaryLoosely || fallBack - // default getters are not checked for compatibility - member.name.is(DefaultGetterName) || { - if ctx.explicitNulls && (member.is(JavaDefined) || other.is(JavaDefined)) then - // releaxed override check for explicit nulls if one of the symbols is Java defined, - // force `Null` being a bottom types during override checking. - val overrideCtx = ctx.retractMode(Mode.SafeNulls).addMode(Mode.RelaxedOverriding) - memberTp.overrides(otherTp, matchNullaryLoosely)(using overrideCtx) - else - memberTp.overrides(otherTp, matchNullaryLoosely) - } + // releaxed override check for explicit nulls if one of the symbols is Java defined, + // force `Null` being a bottom types during override checking. + val overrideCtx = if ctx.explicitNulls && (member.is(JavaDefined) || other.is(JavaDefined)) + then ctx.retractMode(Mode.SafeNulls).addMode(Mode.RelaxedOverriding) else ctx + member.name.is(DefaultGetterName) // default getters are not checked for compatibility + || memberTp.overrides(otherTp, + member.matchNullaryLoosely || other.matchNullaryLoosely || fallBack + )(using overrideCtx) end OverridingPairs diff --git a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala index 000d3ee846ff..1f5aa25535d7 100644 --- a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala +++ b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala @@ -113,15 +113,11 @@ object ResolveSuper { val accTp = acc.asSeenFrom(base.typeRef).info // Since the super class can be Java defined, // we use releaxed overriding check for explicit nulls if one of the symbols is Java defined. - // This forces `Null` being a subtype of reference types during override checking. - val overridesSuper = if ctx.explicitNulls && (sym.is(JavaDefined) || acc.is(JavaDefined)) then - val overrideCtx = ctx.retractMode(Mode.SafeNulls).addMode(Mode.RelaxedOverriding) - otherTp.overrides(accTp, matchLoosely = true)(using overrideCtx) - else - otherTp.overrides(accTp, matchLoosely = true) - if !overridesSuper then + // This forces `Null` being a bottom type during override checking. + val overrideCtx = if ctx.explicitNulls && (sym.is(JavaDefined) || acc.is(JavaDefined)) + then ctx.retractMode(Mode.SafeNulls).addMode(Mode.RelaxedOverriding) else ctx + if !otherTp.overrides(accTp, matchLoosely = true)(using overrideCtx) then report.error(IllegalSuperAccessor(base, memberName, targetName, acc, accTp, other.symbol, otherTp), base.srcPos) - bcs = bcs.tail } assert(sym.exists, i"cannot rebind $acc, ${acc.targetName} $memberName") From 4472e6166780bc3f0617ce819a1106905b954c84 Mon Sep 17 00:00:00 2001 From: noti0na1 Date: Thu, 10 Feb 2022 12:15:51 -0500 Subject: [PATCH 5/7] Fix typos --- compiler/src/dotty/tools/dotc/core/TypeComparer.scala | 2 +- compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala | 2 +- compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index c5c25cafc085..18d469f8e2f8 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -769,7 +769,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling // `Mode.RelaxedOverriding` is only enabled when checking Java overriding // in explicit nulls, and `Null` becomes a bottom type, which allows // `T | Null` being a subtype of `T`. - // A type varibale `T` from Java is translated to `T >: Nothing <: Any`. + // A type variable `T` from Java is translated to `T >: Nothing <: Any`. // However, `null` can always be a value of `T` for Java side. // So the best solution here is to let `Null` be bottom type temporarily. def isNullable(tp: Type): Boolean = ctx.mode.is(Mode.RelaxedOverriding) || { diff --git a/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala b/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala index f67f11cbf920..a40ef61ddebd 100644 --- a/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala +++ b/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala @@ -217,7 +217,7 @@ object OverridingPairs: ) else // releaxed override check for explicit nulls if one of the symbols is Java defined, - // force `Null` being a bottom types during override checking. + // force `Null` to be a bottom type during override checking. val overrideCtx = if ctx.explicitNulls && (member.is(JavaDefined) || other.is(JavaDefined)) then ctx.retractMode(Mode.SafeNulls).addMode(Mode.RelaxedOverriding) else ctx member.name.is(DefaultGetterName) // default getters are not checked for compatibility diff --git a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala index 1f5aa25535d7..1ee7ee09d843 100644 --- a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala +++ b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala @@ -112,8 +112,8 @@ object ResolveSuper { val otherTp = other.asSeenFrom(base.typeRef).info val accTp = acc.asSeenFrom(base.typeRef).info // Since the super class can be Java defined, - // we use releaxed overriding check for explicit nulls if one of the symbols is Java defined. - // This forces `Null` being a bottom type during override checking. + // we use relaxed overriding check for explicit nulls if one of the symbols is Java defined. + // This forces `Null` to be a bottom type during override checking. val overrideCtx = if ctx.explicitNulls && (sym.is(JavaDefined) || acc.is(JavaDefined)) then ctx.retractMode(Mode.SafeNulls).addMode(Mode.RelaxedOverriding) else ctx if !otherTp.overrides(accTp, matchLoosely = true)(using overrideCtx) then From 939ec218347b275c0c97f425b7dd786792faedfd Mon Sep 17 00:00:00 2001 From: noti0na1 Date: Fri, 18 Feb 2022 13:01:27 -0500 Subject: [PATCH 6/7] Add restriction --- compiler/src/dotty/tools/dotc/core/Mode.scala | 2 +- .../dotty/tools/dotc/core/TypeComparer.scala | 23 +++++++++++-------- .../dotc/transform/OverridingPairs.scala | 2 +- .../tools/dotc/transform/ResolveSuper.scala | 2 +- 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Mode.scala b/compiler/src/dotty/tools/dotc/core/Mode.scala index 7a7d50ac0d33..ec8ab0dbca15 100644 --- a/compiler/src/dotty/tools/dotc/core/Mode.scala +++ b/compiler/src/dotty/tools/dotc/core/Mode.scala @@ -126,7 +126,7 @@ object Mode { val ForceInline: Mode = newMode(29, "ForceInline") /** This mode is enabled when we check Java overriding in explicit nulls. - * Type `Null` becomes a bottom type in TypeComparer. + * Type `Null` becomes a subtype of non-primitive value types in TypeComparer. */ val RelaxedOverriding: Mode = newMode(30, "RelaxedOverriding") } diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index 18d469f8e2f8..9e71e362a345 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -771,16 +771,19 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling // `T | Null` being a subtype of `T`. // A type variable `T` from Java is translated to `T >: Nothing <: Any`. // However, `null` can always be a value of `T` for Java side. - // So the best solution here is to let `Null` be bottom type temporarily. - def isNullable(tp: Type): Boolean = ctx.mode.is(Mode.RelaxedOverriding) || { - tp.widenDealias match - case tp: TypeRef => tp.symbol.isNullableClass - case tp: RefinedOrRecType => isNullable(tp.parent) - case tp: AppliedType => isNullable(tp.tycon) - case AndType(tp1, tp2) => isNullable(tp1) && isNullable(tp2) - case OrType(tp1, tp2) => isNullable(tp1) || isNullable(tp2) - case _ => false - } + // So the best solution here is to let `Null` be a subtype of non-primitive + // value types temporarily. + def isNullable(tp: Type): Boolean = tp.widenDealias match + case tp: TypeRef => + val tpSym = tp.symbol + ctx.mode.is(Mode.RelaxedOverriding) && !tpSym.isPrimitiveValueClass || + tpSym.isNullableClass + case tp: RefinedOrRecType => isNullable(tp.parent) + case tp: AppliedType => isNullable(tp.tycon) + case AndType(tp1, tp2) => isNullable(tp1) && isNullable(tp2) + case OrType(tp1, tp2) => isNullable(tp1) || isNullable(tp2) + case _ => false + val sym1 = tp1.symbol (sym1 eq NothingClass) && tp2.isValueTypeOrLambda || (sym1 eq NullClass) && isNullable(tp2) diff --git a/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala b/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala index a40ef61ddebd..0490f8c513e3 100644 --- a/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala +++ b/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala @@ -217,7 +217,7 @@ object OverridingPairs: ) else // releaxed override check for explicit nulls if one of the symbols is Java defined, - // force `Null` to be a bottom type during override checking. + // force `Null` to be a subtype of non-primitive value types during override checking. val overrideCtx = if ctx.explicitNulls && (member.is(JavaDefined) || other.is(JavaDefined)) then ctx.retractMode(Mode.SafeNulls).addMode(Mode.RelaxedOverriding) else ctx member.name.is(DefaultGetterName) // default getters are not checked for compatibility diff --git a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala index 1ee7ee09d843..aa5ac62d6c9d 100644 --- a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala +++ b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala @@ -113,7 +113,7 @@ object ResolveSuper { val accTp = acc.asSeenFrom(base.typeRef).info // Since the super class can be Java defined, // we use relaxed overriding check for explicit nulls if one of the symbols is Java defined. - // This forces `Null` to be a bottom type during override checking. + // This forces `Null` to be a subtype of non-primitive value types during override checking. val overrideCtx = if ctx.explicitNulls && (sym.is(JavaDefined) || acc.is(JavaDefined)) then ctx.retractMode(Mode.SafeNulls).addMode(Mode.RelaxedOverriding) else ctx if !otherTp.overrides(accTp, matchLoosely = true)(using overrideCtx) then From e78f44969999497a373a20ba922896ae0503a445 Mon Sep 17 00:00:00 2001 From: noti0na1 Date: Sat, 19 Feb 2022 13:52:19 -0500 Subject: [PATCH 7/7] Move relaxed context to a function --- compiler/src/dotty/tools/dotc/core/Contexts.scala | 3 +++ compiler/src/dotty/tools/dotc/core/Types.scala | 4 +--- compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala | 2 +- compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Contexts.scala b/compiler/src/dotty/tools/dotc/core/Contexts.scala index e9fbd6065261..27859abaae52 100644 --- a/compiler/src/dotty/tools/dotc/core/Contexts.scala +++ b/compiler/src/dotty/tools/dotc/core/Contexts.scala @@ -712,6 +712,9 @@ object Contexts { def withNotNullInfos(infos: List[NotNullInfo]): Context = if c.notNullInfos eq infos then c else c.fresh.setNotNullInfos(infos) + + def relaxedOverrideContext: Context = + c.withModeBits(c.mode &~ Mode.SafeNulls | Mode.RelaxedOverriding) end ops // TODO: Fix issue when converting ModeChanges and FreshModeChanges to extension givens diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 70d1ea29c1a8..e9eb8ed78d02 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -1112,9 +1112,7 @@ object Types { */ def matches(that: Type)(using Context): Boolean = { record("matches") - val overrideCtx = if ctx.explicitNulls - then ctx.retractMode(Mode.SafeNulls).addMode(Mode.RelaxedOverriding) - else ctx + val overrideCtx = if ctx.explicitNulls then ctx.relaxedOverrideContext else ctx TypeComparer.matchesType(this, that, relaxed = !ctx.phase.erasedTypes)(using overrideCtx) } diff --git a/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala b/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala index 0490f8c513e3..7a0516c38991 100644 --- a/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala +++ b/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala @@ -219,7 +219,7 @@ object OverridingPairs: // releaxed override check for explicit nulls if one of the symbols is Java defined, // force `Null` to be a subtype of non-primitive value types during override checking. val overrideCtx = if ctx.explicitNulls && (member.is(JavaDefined) || other.is(JavaDefined)) - then ctx.retractMode(Mode.SafeNulls).addMode(Mode.RelaxedOverriding) else ctx + then ctx.relaxedOverrideContext else ctx member.name.is(DefaultGetterName) // default getters are not checked for compatibility || memberTp.overrides(otherTp, member.matchNullaryLoosely || other.matchNullaryLoosely || fallBack diff --git a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala index aa5ac62d6c9d..3c03fa012464 100644 --- a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala +++ b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala @@ -115,7 +115,7 @@ object ResolveSuper { // we use relaxed overriding check for explicit nulls if one of the symbols is Java defined. // This forces `Null` to be a subtype of non-primitive value types during override checking. val overrideCtx = if ctx.explicitNulls && (sym.is(JavaDefined) || acc.is(JavaDefined)) - then ctx.retractMode(Mode.SafeNulls).addMode(Mode.RelaxedOverriding) else ctx + then ctx.relaxedOverrideContext else ctx if !otherTp.overrides(accTp, matchLoosely = true)(using overrideCtx) then report.error(IllegalSuperAccessor(base, memberName, targetName, acc, accTp, other.symbol, otherTp), base.srcPos) bcs = bcs.tail