From 7a340a071082c547096133a899eb7f2e3e4ef426 Mon Sep 17 00:00:00 2001 From: noti0na1 Date: Wed, 23 Mar 2022 02:49:47 -0400 Subject: [PATCH 1/3] Fix overriding check in mergeSingleDenot --- .../dotty/tools/dotc/core/Denotations.scala | 5 ++-- .../src/dotty/tools/dotc/core/Types.scala | 20 ++++++++------ .../dotc/transform/OverridingPairs.scala | 8 +++--- .../tools/dotc/transform/ResolveSuper.scala | 5 ++-- .../dotty/tools/dotc/typer/RefChecks.scala | 2 +- tests/explicit-nulls/pos-special/i14682.scala | 27 +++++++++++++++++++ 6 files changed, 48 insertions(+), 19 deletions(-) create mode 100644 tests/explicit-nulls/pos-special/i14682.scala diff --git a/compiler/src/dotty/tools/dotc/core/Denotations.scala b/compiler/src/dotty/tools/dotc/core/Denotations.scala index 8f0ae7a2da93..0109fabb1744 100644 --- a/compiler/src/dotty/tools/dotc/core/Denotations.scala +++ b/compiler/src/dotty/tools/dotc/core/Denotations.scala @@ -469,11 +469,12 @@ 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, matchLoosely, checkClassInfo = false) then + if symScore <= 0 && info2.overrides(info1, relaxedOverriding, matchLoosely, checkClassInfo = false) then denot2 - else if symScore >= 0 && info1.overrides(info2, matchLoosely, checkClassInfo = false) then + else if symScore >= 0 && info1.overrides(info2, relaxedOverriding, matchLoosely, checkClassInfo = false) then denot1 else val jointInfo = infoMeet(info1, info2, safeIntersection) diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 24acce54cc4b..6e16e73e6d63 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -1071,21 +1071,25 @@ object Types { /** Is this type a legal type for member `sym1` that overrides another * member `sym2` of type `that`? This is the same as `<:<`, except that + * @param relaxedCheck if true type `Null` becomes a subtype of non-primitive value types in TypeComparer. * @param matchLoosely if true the types `=> T` and `()T` are seen as overriding each other. * @param checkClassInfo if true we check that ClassInfos are within bounds of abstract types */ - final def overrides(that: Type, matchLoosely: => Boolean, checkClassInfo: Boolean = true)(using Context): Boolean = { + final def overrides(that: Type, relaxedCheck: Boolean, matchLoosely: => Boolean, checkClassInfo: Boolean = true)(using Context): Boolean = { def widenNullary(tp: Type) = tp match { case tp @ MethodType(Nil) => tp.resultType case _ => tp } - !checkClassInfo && this.isInstanceOf[ClassInfo] - || (this.widenExpr frozen_<:< that.widenExpr) - || matchLoosely && { - val this1 = widenNullary(this) - val that1 = widenNullary(that) - ((this1 `ne` this) || (that1 `ne` that)) && this1.overrides(that1, false, checkClassInfo) - } + val overrideCtx = if relaxedCheck && !ctx.mode.is(Mode.RelaxedOverriding) then ctx.relaxedOverrideContext else ctx + inContext(overrideCtx) { + !checkClassInfo && this.isInstanceOf[ClassInfo] + || (this.widenExpr frozen_<:< that.widenExpr) + || matchLoosely && { + val this1 = widenNullary(this) + val that1 = widenNullary(that) + ((this1 `ne` this) || (that1 `ne` that)) && this1.overrides(that1, relaxedCheck, false, checkClassInfo) + } + } } /** Is this type close enough to that type so that members diff --git a/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala b/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala index 4044034cb5ea..b820ae94be4d 100644 --- a/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala +++ b/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala @@ -217,11 +217,9 @@ 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 overrideCtx = if ctx.explicitNulls && (member.is(JavaDefined) || other.is(JavaDefined)) - then ctx.relaxedOverrideContext else ctx + val relaxedOverriding = ctx.explicitNulls && (member.is(JavaDefined) || other.is(JavaDefined)) member.name.is(DefaultGetterName) // default getters are not checked for compatibility - || memberTp.overrides(otherTp, - member.matchNullaryLoosely || other.matchNullaryLoosely || fallBack - )(using overrideCtx) + || memberTp.overrides(otherTp, relaxedOverriding, + member.matchNullaryLoosely || other.matchNullaryLoosely || fallBack) end OverridingPairs diff --git a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala index da439b0f4f84..47422512823e 100644 --- a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala +++ b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala @@ -113,9 +113,8 @@ object ResolveSuper { // 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 overrideCtx = if ctx.explicitNulls && (sym.is(JavaDefined) || acc.is(JavaDefined)) - then ctx.relaxedOverrideContext else ctx - if !otherTp.overrides(accTp, matchLoosely = true)(using overrideCtx) then + val relaxedOverriding = ctx.explicitNulls && (sym.is(JavaDefined) || acc.is(JavaDefined)) + if !otherTp.overrides(accTp, relaxedOverriding, 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 4a0db540ca2d..0a195409f9e8 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala @@ -767,7 +767,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, matchLoosely = true)) + if (!mbrType.overrides(mbrd.info, false, matchLoosely = true)) report.errorOrMigrationWarning( em"""${mbr.showLocated} is not a legal implementation of `$name` in $clazz | its type $mbrType diff --git a/tests/explicit-nulls/pos-special/i14682.scala b/tests/explicit-nulls/pos-special/i14682.scala new file mode 100644 index 000000000000..0c0619d8105b --- /dev/null +++ b/tests/explicit-nulls/pos-special/i14682.scala @@ -0,0 +1,27 @@ +class C1: + sealed abstract class Name { + type ThisName <: Name + def compareTo(that: ThisName): Int = ??? + } + + class LocalName extends Name with Comparable[LocalName] { + type ThisName = LocalName + } + + val localName = LocalName() + println(localName) + var count = 0 + +class C2: + sealed abstract class Name { + type ThisName <: Name + def compareTo(that: ThisName | Null): Int = ??? + } + + class LocalName extends Name with Comparable[LocalName] { + type ThisName = LocalName + } + + val localName = LocalName() + println(localName) + var count = 0 \ No newline at end of file From 654ec8e01626e4b1e00ffc1aee03dce562343dec Mon Sep 17 00:00:00 2001 From: noti0na1 Date: Wed, 23 Mar 2022 02:50:29 -0400 Subject: [PATCH 2/3] Move explicit nulls tests --- compiler/test/dotty/tools/dotc/CompilationTests.scala | 3 +-- .../neg}/byname-nullables.check | 8 ++++---- .../neg}/byname-nullables.scala | 0 .../neg}/byname-nullables1.check | 2 +- .../neg}/byname-nullables1.scala | 0 .../explicit-nulls => explicit-nulls/neg}/i7883.check | 6 +++--- .../explicit-nulls => explicit-nulls/neg}/i7883.scala | 0 tests/{pos-special => explicit-nulls/pos}/notNull.scala | 0 8 files changed, 9 insertions(+), 10 deletions(-) rename tests/{neg-custom-args/explicit-nulls => explicit-nulls/neg}/byname-nullables.check (78%) rename tests/{neg-custom-args/explicit-nulls => explicit-nulls/neg}/byname-nullables.scala (100%) rename tests/{neg-custom-args/explicit-nulls => explicit-nulls/neg}/byname-nullables1.check (77%) rename tests/{neg-custom-args/explicit-nulls => explicit-nulls/neg}/byname-nullables1.scala (100%) rename tests/{neg-custom-args/explicit-nulls => explicit-nulls/neg}/i7883.check (70%) rename tests/{neg-custom-args/explicit-nulls => explicit-nulls/neg}/i7883.scala (100%) rename tests/{pos-special => explicit-nulls/pos}/notNull.scala (100%) diff --git a/compiler/test/dotty/tools/dotc/CompilationTests.scala b/compiler/test/dotty/tools/dotc/CompilationTests.scala index f8089dfcabfa..bfaf10751fde 100644 --- a/compiler/test/dotty/tools/dotc/CompilationTests.scala +++ b/compiler/test/dotty/tools/dotc/CompilationTests.scala @@ -52,7 +52,6 @@ class CompilationTests { ), compileFile("tests/pos-special/typeclass-scaling.scala", defaultOptions.and("-Xmax-inlines", "40")), compileFile("tests/pos-special/i7296.scala", defaultOptions.and("-source", "future", "-deprecation", "-Xfatal-warnings")), - compileFile("tests/pos-special/notNull.scala", defaultOptions.and("-Yexplicit-nulls")), compileDir("tests/pos-special/adhoc-extension", defaultOptions.and("-source", "future", "-feature", "-Xfatal-warnings")), compileFile("tests/pos-special/i7575.scala", defaultOptions.andLanguageFeature("dynamics")), compileFile("tests/pos-special/kind-projector.scala", defaultOptions.and("-Ykind-projector")), @@ -137,7 +136,6 @@ class CompilationTests { compileFilesInDir("tests/neg-custom-args/erased", defaultOptions.and("-language:experimental.erasedDefinitions")), compileFilesInDir("tests/neg-custom-args/allow-double-bindings", allowDoubleBindings), compileFilesInDir("tests/neg-custom-args/allow-deep-subtypes", allowDeepSubtypes), - compileFilesInDir("tests/neg-custom-args/explicit-nulls", defaultOptions.and("-Yexplicit-nulls")), compileFilesInDir("tests/neg-custom-args/no-experimental", defaultOptions.and("-Yno-experimental")), compileDir("tests/neg-custom-args/impl-conv", defaultOptions.and("-Xfatal-warnings", "-feature")), compileDir("tests/neg-custom-args/i13946", defaultOptions.and("-Xfatal-warnings", "-feature")), @@ -249,6 +247,7 @@ class CompilationTests { compileFilesInDir("tests/explicit-nulls/pos-separate", explicitNullsOptions), compileFilesInDir("tests/explicit-nulls/pos-patmat", explicitNullsOptions and "-Xfatal-warnings"), compileFilesInDir("tests/explicit-nulls/unsafe-common", explicitNullsOptions and "-language:unsafeNulls"), + compileFile("tests/explicit-nulls/pos-special/i14682.scala", explicitNullsOptions and "-Ysafe-init"), ) }.checkCompile() diff --git a/tests/neg-custom-args/explicit-nulls/byname-nullables.check b/tests/explicit-nulls/neg/byname-nullables.check similarity index 78% rename from tests/neg-custom-args/explicit-nulls/byname-nullables.check rename to tests/explicit-nulls/neg/byname-nullables.check index ffd0a637563e..887e0267b6eb 100644 --- a/tests/neg-custom-args/explicit-nulls/byname-nullables.check +++ b/tests/explicit-nulls/neg/byname-nullables.check @@ -1,11 +1,11 @@ --- [E007] Type Mismatch Error: tests/neg-custom-args/explicit-nulls/byname-nullables.scala:19:24 ----------------------- +-- [E007] Type Mismatch Error: tests/explicit-nulls/neg/byname-nullables.scala:19:24 ----------------------------------- 19 | if x != null then f(x) // error: f is call-by-name | ^ | Found: (x : String | Null) | Required: String | | longer explanation available when compiling with `-explain` --- Error: tests/neg-custom-args/explicit-nulls/byname-nullables.scala:43:32 -------------------------------------------- +-- Error: tests/explicit-nulls/neg/byname-nullables.scala:43:32 -------------------------------------------------------- 43 | if x != null then f(identity(x), 1) // error: dropping not null check fails typing | ^^^^^^^^^^^ | This argument was typed using flow assumptions about mutable variables @@ -13,7 +13,7 @@ | Wrapping the argument in `byName(...)` fixes the problem by disabling the flow assumptions. | | `byName` needs to be imported from the `scala.compiletime` package. --- Error: tests/neg-custom-args/explicit-nulls/byname-nullables.scala:68:24 -------------------------------------------- +-- Error: tests/explicit-nulls/neg/byname-nullables.scala:68:24 -------------------------------------------------------- 68 | if x != null then f(x, 1) // error: dropping not null check typechecks OK, but gives incompatible result type | ^ | This argument was typed using flow assumptions about mutable variables @@ -21,7 +21,7 @@ | Wrapping the argument in `byName(...)` fixes the problem by disabling the flow assumptions. | | `byName` needs to be imported from the `scala.compiletime` package. --- [E134] Type Error: tests/neg-custom-args/explicit-nulls/byname-nullables.scala:81:22 -------------------------------- +-- [E134] Type Error: tests/explicit-nulls/neg/byname-nullables.scala:81:22 -------------------------------------------- 81 | if x != null then f(byName(x), 1) // error: none of the overloaded methods match argument types | ^ | None of the overloaded alternatives of method f in object Test7 with types diff --git a/tests/neg-custom-args/explicit-nulls/byname-nullables.scala b/tests/explicit-nulls/neg/byname-nullables.scala similarity index 100% rename from tests/neg-custom-args/explicit-nulls/byname-nullables.scala rename to tests/explicit-nulls/neg/byname-nullables.scala diff --git a/tests/neg-custom-args/explicit-nulls/byname-nullables1.check b/tests/explicit-nulls/neg/byname-nullables1.check similarity index 77% rename from tests/neg-custom-args/explicit-nulls/byname-nullables1.check rename to tests/explicit-nulls/neg/byname-nullables1.check index e4a46bf8322f..05ec1c811a9e 100644 --- a/tests/neg-custom-args/explicit-nulls/byname-nullables1.check +++ b/tests/explicit-nulls/neg/byname-nullables1.check @@ -1,4 +1,4 @@ --- Error: tests/neg-custom-args/explicit-nulls/byname-nullables1.scala:10:6 -------------------------------------------- +-- Error: tests/explicit-nulls/neg/byname-nullables1.scala:10:6 -------------------------------------------------------- 10 | f(x.fld != null) // error | ^^^^^^^^^^^^^ | This argument was typed using flow assumptions about mutable variables diff --git a/tests/neg-custom-args/explicit-nulls/byname-nullables1.scala b/tests/explicit-nulls/neg/byname-nullables1.scala similarity index 100% rename from tests/neg-custom-args/explicit-nulls/byname-nullables1.scala rename to tests/explicit-nulls/neg/byname-nullables1.scala diff --git a/tests/neg-custom-args/explicit-nulls/i7883.check b/tests/explicit-nulls/neg/i7883.check similarity index 70% rename from tests/neg-custom-args/explicit-nulls/i7883.check rename to tests/explicit-nulls/neg/i7883.check index 579ea0b4cd00..e37285332359 100644 --- a/tests/neg-custom-args/explicit-nulls/i7883.check +++ b/tests/explicit-nulls/neg/i7883.check @@ -1,4 +1,4 @@ --- [E134] Type Error: tests/neg-custom-args/explicit-nulls/i7883.scala:6:11 -------------------------------------------- +-- [E134] Type Error: tests/explicit-nulls/neg/i7883.scala:6:11 -------------------------------------------------------- 6 | case r(hd, tl) => Some((hd, tl)) // error // error // error | ^ | None of the overloaded alternatives of method unapplySeq in class Regex with types @@ -6,13 +6,13 @@ | (c: Char): Option[List[Char]] | (s: CharSequence): Option[List[String]] | match arguments (String | Null) --- [E006] Not Found Error: tests/neg-custom-args/explicit-nulls/i7883.scala:6:30 --------------------------------------- +-- [E006] Not Found Error: tests/explicit-nulls/neg/i7883.scala:6:30 --------------------------------------------------- 6 | case r(hd, tl) => Some((hd, tl)) // error // error // error | ^^ | Not found: hd | | longer explanation available when compiling with `-explain` --- [E006] Not Found Error: tests/neg-custom-args/explicit-nulls/i7883.scala:6:34 --------------------------------------- +-- [E006] Not Found Error: tests/explicit-nulls/neg/i7883.scala:6:34 --------------------------------------------------- 6 | case r(hd, tl) => Some((hd, tl)) // error // error // error | ^^ | Not found: tl diff --git a/tests/neg-custom-args/explicit-nulls/i7883.scala b/tests/explicit-nulls/neg/i7883.scala similarity index 100% rename from tests/neg-custom-args/explicit-nulls/i7883.scala rename to tests/explicit-nulls/neg/i7883.scala diff --git a/tests/pos-special/notNull.scala b/tests/explicit-nulls/pos/notNull.scala similarity index 100% rename from tests/pos-special/notNull.scala rename to tests/explicit-nulls/pos/notNull.scala From 6f4ac270757f072133acd6c32ab944ccf0a4da90 Mon Sep 17 00:00:00 2001 From: noti0na1 Date: Wed, 23 Mar 2022 12:27:19 -0400 Subject: [PATCH 3/3] Add name for the boolean argument --- compiler/src/dotty/tools/dotc/core/Types.scala | 2 +- compiler/src/dotty/tools/dotc/typer/RefChecks.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 6e16e73e6d63..47b83082bfdd 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -1080,7 +1080,7 @@ object Types { case tp @ MethodType(Nil) => tp.resultType case _ => tp } - val overrideCtx = if relaxedCheck && !ctx.mode.is(Mode.RelaxedOverriding) then ctx.relaxedOverrideContext else ctx + val overrideCtx = if relaxedCheck then ctx.relaxedOverrideContext else ctx inContext(overrideCtx) { !checkClassInfo && this.isInstanceOf[ClassInfo] || (this.widenExpr frozen_<:< that.widenExpr) diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala index 0a195409f9e8..0de3a03b167e 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala @@ -767,7 +767,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, false, matchLoosely = true)) + if (!mbrType.overrides(mbrd.info, relaxedCheck = false, matchLoosely = true)) report.errorOrMigrationWarning( em"""${mbr.showLocated} is not a legal implementation of `$name` in $clazz | its type $mbrType