From 090dbf45de81fb0f665658ea9932116900e6b702 Mon Sep 17 00:00:00 2001 From: HarrisL2 Date: Thu, 19 Dec 2024 00:00:24 -0500 Subject: [PATCH 1/4] Remove special overriding logic for explicit nulls --- .../src/dotty/tools/dotc/core/Contexts.scala | 3 --- .../dotty/tools/dotc/core/Denotations.scala | 5 ++--- compiler/src/dotty/tools/dotc/core/Mode.scala | 7 +------ .../dotty/tools/dotc/core/TypeComparer.scala | 9 +-------- .../src/dotty/tools/dotc/core/Types.scala | 20 ++++++++----------- .../dotc/transform/OverridingPairs.scala | 6 +----- .../tools/dotc/transform/ResolveSuper.scala | 7 ++----- .../dotty/tools/dotc/typer/RefChecks.scala | 2 +- 8 files changed, 16 insertions(+), 43 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Contexts.scala b/compiler/src/dotty/tools/dotc/core/Contexts.scala index 7f5779bb6127..0cdfc8791c00 100644 --- a/compiler/src/dotty/tools/dotc/core/Contexts.scala +++ b/compiler/src/dotty/tools/dotc/core/Contexts.scala @@ -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 = diff --git a/compiler/src/dotty/tools/dotc/core/Denotations.scala b/compiler/src/dotty/tools/dotc/core/Denotations.scala index 85ff51bc19de..c5047cb5d806 100644 --- a/compiler/src/dotty/tools/dotc/core/Denotations.scala +++ b/compiler/src/dotty/tools/dotc/core/Denotations.scala @@ -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) diff --git a/compiler/src/dotty/tools/dotc/core/Mode.scala b/compiler/src/dotty/tools/dotc/core/Mode.scala index 14d7827974c0..55e88eae4f6e 100644 --- a/compiler/src/dotty/tools/dotc/core/Mode.scala +++ b/compiler/src/dotty/tools/dotc/core/Mode.scala @@ -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") @@ -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") } diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index 8414c3795f49..ca2df327851e 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -967,17 +967,10 @@ 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.isPrimitiveValueClass || tpSym.isNullableClass case tp: TermRef => // https://scala-lang.org/files/archive/spec/2.13/03-types.html#singleton-types diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index c5937074f4bc..ab3b44847e18 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -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) } } @@ -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) } /** 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 6529eed77fa0..a9a17f6db464 100644 --- a/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala +++ b/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala @@ -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 diff --git a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala index dd3f41be5a8e..7b236f0e2b0e 100644 --- a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala +++ b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala @@ -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 } diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala index 7e53b38b5f98..00a6a0a8e060 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala @@ -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 From e8fbff18c648514bf919f16e1a2647657496bc47 Mon Sep 17 00:00:00 2001 From: HarrisL2 Date: Thu, 19 Dec 2024 19:41:16 -0500 Subject: [PATCH 2/4] Fix explicit null matching issue --- compiler/src/dotty/tools/dotc/core/TypeComparer.scala | 1 - compiler/src/dotty/tools/dotc/core/Types.scala | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index ca2df327851e..03f4e3e86a8e 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -970,7 +970,6 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling def isNullable(tp: Type): Boolean = tp.dealias match case tp: TypeRef => val tpSym = tp.symbol - !tpSym.isPrimitiveValueClass || tpSym.isNullableClass case tp: TermRef => // https://scala-lang.org/files/archive/spec/2.13/03-types.html#singleton-types diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index ab3b44847e18..884a71e23339 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -1193,7 +1193,8 @@ object Types extends TypeUtils { */ def matches(that: Type)(using Context): Boolean = { record("matches") - TypeComparer.matchesType(this, that, relaxed = !ctx.phase.erasedTypes) + withoutMode(Mode.SafeNulls)( + TypeComparer.matchesType(this, that, relaxed = !ctx.phase.erasedTypes)) } /** This is the same as `matches` except that it also matches => T with T and From ce9739d30457b9d446c82eab794e7c3e2f68b493 Mon Sep 17 00:00:00 2001 From: HarrisL2 Date: Sat, 21 Dec 2024 17:22:26 -0500 Subject: [PATCH 3/4] Change bootstrap compiler to use flexible types --- .../tools/dotc/core/JavaNullInterop.scala | 4 ++- project/Build.scala | 25 +++++++++---------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/JavaNullInterop.scala b/compiler/src/dotty/tools/dotc/core/JavaNullInterop.scala index 46ce0d2d7852..f3e62bc36e06 100644 --- a/compiler/src/dotty/tools/dotc/core/JavaNullInterop.scala +++ b/compiler/src/dotty/tools/dotc/core/JavaNullInterop.scala @@ -6,6 +6,8 @@ import Flags.JavaDefined import StdNames.nme import Symbols.* import Types.* +import dotty.tools.dotc.reporting.* +import dotty.tools.dotc.core.Decorators.i /** This module defines methods to interpret types of Java symbols, which are implicitly nullable in Java, * as Scala types, which are explicitly nullable. @@ -51,7 +53,7 @@ object JavaNullInterop { * * But the selection can throw an NPE if the returned value is `null`. */ - def nullifyMember(sym: Symbol, tp: Type, isEnumValueDef: Boolean)(using Context): Type = { + def nullifyMember(sym: Symbol, tp: Type, isEnumValueDef: Boolean)(using Context): Type = trace(i"nullifyMember ${sym}, ${tp}"){ assert(ctx.explicitNulls) assert(sym.is(JavaDefined), "can only nullify java-defined members") diff --git a/project/Build.scala b/project/Build.scala index 5aec4a4231a6..cdce133ab069 100644 --- a/project/Build.scala +++ b/project/Build.scala @@ -95,8 +95,8 @@ object Build { * Reference version should track the latest version pushed to Maven: * - In main branch it should be the last RC version * - In release branch it should be the last stable release - * - * Warning: Change of this variable needs to be consulted with `expectedTastyVersion` + * + * Warning: Change of this variable needs to be consulted with `expectedTastyVersion` */ val referenceVersion = "3.6.3-RC1" @@ -106,8 +106,8 @@ object Build { * * Should only be referred from `dottyVersion` or settings/tasks requiring simplified version string, * eg. `compatMode` or Windows native distribution version. - * - * Warning: Change of this variable might require updating `expectedTastyVersion` + * + * Warning: Change of this variable might require updating `expectedTastyVersion` */ val developedVersion = "3.6.4" @@ -119,13 +119,13 @@ object Build { * During final, stable release is set exactly to `developedVersion`. */ val baseVersion = s"$developedVersion-RC1" - - /** The version of TASTY that should be emitted, checked in runtime test - * For defails on how TASTY version should be set see related discussions: + + /** The version of TASTY that should be emitted, checked in runtime test + * For defails on how TASTY version should be set see related discussions: * - https://github.com/scala/scala3/issues/13447#issuecomment-912447107 * - https://github.com/scala/scala3/issues/14306#issuecomment-1069333516 * - https://github.com/scala/scala3/pull/19321 - * + * * Simplified rules, given 3.$minor.$patch = $developedVersion * - Major version is always 28 * - TASTY minor version: @@ -1058,7 +1058,6 @@ object Build { // compiler is updated. // Then, the next step is to enable flexible types by default and reduce the use of // `unsafeNulls`. - scalacOptions ++= Seq("-Yno-flexible-types"), packageAll := { (`scala3-compiler` / packageAll).value ++ Seq( "scala3-compiler" -> (Compile / packageBin).value.getAbsolutePath, @@ -2505,7 +2504,7 @@ object Build { case Bootstrapped => commonBootstrappedSettings }) } - + /* Tests TASTy version invariants during NIGHLY, RC or Stable releases */ def checkReleasedTastyVersion(): Unit = { lazy val (scalaMinor, scalaPatch, scalaIsRC) = baseVersion.split("\\.|-").take(4) match { @@ -2518,19 +2517,19 @@ object Build { case Array("28", minor, "experimental", _) => (minor.toInt, true) case other => sys.error(s"Invalid TASTy version string: $expectedTastyVersion") } - + if(isNightly) { assert(tastyIsExperimental, "TASTY needs to be experimental in nightly builds") val expectedTastyMinor = if(scalaPatch == 0) scalaMinor else scalaMinor + 1 assert(tastyMinor == expectedTastyMinor, "Invalid TASTy minor version") } - + if(isRelease) { assert(scalaMinor == tastyMinor, "Minor versions of TASTY vesion and Scala version should match in release builds") if (scalaIsRC && scalaPatch == 0) assert(tastyIsExperimental, "TASTy should be experimental when releasing a new minor version RC") else - assert(!tastyIsExperimental, "Stable version cannot use experimental TASTY") + assert(!tastyIsExperimental, "Stable version cannot use experimental TASTY") } } } From 52829cc51cea43c0fbcd1db79df32e4fd28c02ae Mon Sep 17 00:00:00 2001 From: HarrisL2 Date: Sat, 21 Dec 2024 17:29:54 -0500 Subject: [PATCH 4/4] Also enable flexible types on presentation compiler --- project/Build.scala | 4 ---- 1 file changed, 4 deletions(-) diff --git a/project/Build.scala b/project/Build.scala index cdce133ab069..b40d13abe2cf 100644 --- a/project/Build.scala +++ b/project/Build.scala @@ -1449,10 +1449,6 @@ object Build { .dependsOn(`scala3-compiler-bootstrapped`, `scala3-library-bootstrapped`, `scala3-presentation-compiler-testcases`) .settings(presentationCompilerSettings) .settings(scala3PresentationCompilerBuildInfo) - .settings( - // Add `-Yno-flexible-types` flag for bootstrap, see comments for `bootstrappedDottyCompilerSettings` - Compile / scalacOptions += "-Yno-flexible-types" - ) def scala3PresentationCompilerBuildInfo = Seq(