Skip to content

Commit

Permalink
Remove special overriding logic for explicit nulls
Browse files Browse the repository at this point in the history
  • Loading branch information
HarrisL2 committed Dec 19, 2024
1 parent bd1f004 commit c6fcaf7
Show file tree
Hide file tree
Showing 8 changed files with 16 additions and 45 deletions.
3 changes: 0 additions & 3 deletions compiler/src/dotty/tools/dotc/core/Contexts.scala
Original file line number Diff line number Diff line change
Expand Up @@ -785,9 +785,6 @@ object Contexts {
def withNotNullInfos(infos: List[NotNullInfo]): Context =
if !c.explicitNulls || (c.notNullInfos eq infos) then c else c.fresh.setNotNullInfos(infos)

def relaxedOverrideContext: Context =
c.withModeBits(c.mode &~ Mode.SafeNulls | Mode.RelaxedOverriding)

// TODO: Fix issue when converting ModeChanges and FreshModeChanges to extension givens
extension (c: Context) {
final def withModeBits(mode: Mode): Context =
Expand Down
5 changes: 2 additions & 3 deletions compiler/src/dotty/tools/dotc/core/Denotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -478,12 +478,11 @@ object Denotations {
else if sym1.is(Method) && !sym2.is(Method) then 1
else 0

val relaxedOverriding = ctx.explicitNulls && (sym1.is(JavaDefined) || sym2.is(JavaDefined))
val matchLoosely = sym1.matchNullaryLoosely || sym2.matchNullaryLoosely

if symScore <= 0 && info2.overrides(info1, relaxedOverriding, matchLoosely, checkClassInfo = false) then
if symScore <= 0 && info2.overrides(info1, matchLoosely, checkClassInfo = false) then
denot2
else if symScore >= 0 && info1.overrides(info2, relaxedOverriding, matchLoosely, checkClassInfo = false) then
else if symScore >= 0 && info1.overrides(info2, matchLoosely, checkClassInfo = false) then
denot1
else
val jointInfo = infoMeet(info1, info2, safeIntersection)
Expand Down
7 changes: 1 addition & 6 deletions compiler/src/dotty/tools/dotc/core/Mode.scala
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ object Mode {

/** Use previous Scheme for implicit resolution. Currently significant
* in 3.0-migration where we use Scala-2's scheme instead and in 3.5 and 3.6-migration
* where we use the previous scheme up to 3.4 for comparison with the new scheme.
* where we use the previous scheme up to 3.4 for comparison with the new scheme.
*/
val OldImplicitResolution: Mode = newMode(15, "OldImplicitResolution")

Expand Down Expand Up @@ -163,11 +163,6 @@ object Mode {
*/
val ForceInline: Mode = newMode(29, "ForceInline")

/** This mode is enabled when we check Java overriding in explicit nulls.
* Type `Null` becomes a subtype of non-primitive value types in TypeComparer.
*/
val RelaxedOverriding: Mode = newMode(30, "RelaxedOverriding")

/** Skip inlining of methods. */
val NoInline: Mode = newMode(31, "NoInline")
}
11 changes: 1 addition & 10 deletions compiler/src/dotty/tools/dotc/core/TypeComparer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -967,18 +967,9 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
|| 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 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 a subtype of non-primitive
// value types temporarily.
def isNullable(tp: Type): Boolean = tp.dealias match
case tp: TypeRef =>
val tpSym = tp.symbol
ctx.mode.is(Mode.RelaxedOverriding) && !tpSym.isPrimitiveValueClass ||
tpSym.isNullableClass
tp.symbol.isNullableClass
case tp: TermRef =>
// https://scala-lang.org/files/archive/spec/2.13/03-types.html#singleton-types
// A singleton type is of the form p.type. Where p is a path pointing to a value which conforms to
Expand Down
20 changes: 8 additions & 12 deletions compiler/src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1160,17 +1160,14 @@ object Types extends TypeUtils {
*
* @param isSubType a function used for checking subtype relationships.
*/
final def overrides(that: Type, relaxedCheck: Boolean, matchLoosely: => Boolean, checkClassInfo: Boolean = true,
final def overrides(that: Type, matchLoosely: => Boolean, checkClassInfo: Boolean = true,
isSubType: (Type, Type) => Context ?=> Boolean = (tp1, tp2) => tp1 frozen_<:< tp2)(using Context): Boolean = {
val overrideCtx = if relaxedCheck then ctx.relaxedOverrideContext else ctx
inContext(overrideCtx) {
!checkClassInfo && this.isInstanceOf[ClassInfo]
|| isSubType(this.widenExpr, that.widenExpr)
|| matchLoosely && {
val this1 = this.widenNullaryMethod
val that1 = that.widenNullaryMethod
((this1 `ne` this) || (that1 `ne` that)) && this1.overrides(that1, relaxedCheck, false, checkClassInfo)
}
!checkClassInfo && this.isInstanceOf[ClassInfo]
|| isSubType(this.widenExpr, that.widenExpr)
|| matchLoosely && {
val this1 = this.widenNullaryMethod
val that1 = that.widenNullaryMethod
((this1 `ne` this) || (that1 `ne` that)) && this1.overrides(that1, false, checkClassInfo)
}
}

Expand All @@ -1196,8 +1193,7 @@ object Types extends TypeUtils {
*/
def matches(that: Type)(using Context): Boolean = {
record("matches")
val overrideCtx = if ctx.explicitNulls then ctx.relaxedOverrideContext else ctx
TypeComparer.matchesType(this, that, relaxed = !ctx.phase.erasedTypes)(using overrideCtx)
TypeComparer.matchesType(this, that, relaxed = !ctx.phase.erasedTypes)(using ctx)
}

/** This is the same as `matches` except that it also matches => T with T and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,11 +225,7 @@ object OverridingPairs:
}
)
else
// 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 relaxedOverriding = ctx.explicitNulls && (member.is(JavaDefined) || other.is(JavaDefined))
member.name.is(DefaultGetterName) // default getters are not checked for compatibility
|| memberTp.overrides(otherTp, relaxedOverriding,
member.matchNullaryLoosely || other.matchNullaryLoosely || fallBack, isSubType = isSubType)
|| memberTp.overrides(otherTp, member.matchNullaryLoosely || other.matchNullaryLoosely || fallBack, isSubType = isSubType)

end OverridingPairs
7 changes: 2 additions & 5 deletions compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,8 @@ object ResolveSuper {
// of the superaccessor's type, see i5433.scala for an example where this matters
val otherTp = other.asSeenFrom(base.typeRef).info
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 subtype of non-primitive value types during override checking.
val relaxedOverriding = ctx.explicitNulls && (sym.is(JavaDefined) || acc.is(JavaDefined))
if !otherTp.overrides(accTp, relaxedOverriding, matchLoosely = true) then

if !otherTp.overrides(accTp, matchLoosely = true) then
report.error(IllegalSuperAccessor(base, memberName, targetName, acc, accTp, other.symbol, otherTp), base.srcPos)
bcs = bcs.tail
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/typer/RefChecks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -906,7 +906,7 @@ object RefChecks {
for (mbrd <- self.member(name).alternatives) {
val mbr = mbrd.symbol
val mbrType = mbr.info.asSeenFrom(self, mbr.owner)
if (!mbrType.overrides(mbrd.info, relaxedCheck = false, matchLoosely = true))
if (!mbrType.overrides(mbrd.info, matchLoosely = true))
report.errorOrMigrationWarning(
em"""${mbr.showLocated} is not a legal implementation of `$name` in $clazz
| its type $mbrType
Expand Down

0 comments on commit c6fcaf7

Please sign in to comment.