Skip to content

Commit fb7f900

Browse files
olhotakolhotak
authored
Merge pull request #14755 from dotty-staging/fix-14682
Fix 14682: fix overriding check in mergeSingleDenot
2 parents fdd43d7 + 6f4ac27 commit fb7f900

File tree

14 files changed

+57
-29
lines changed

14 files changed

+57
-29
lines changed

‎compiler/src/dotty/tools/dotc/core/Denotations.scala‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -469,11 +469,12 @@ object Denotations {
469469
else if sym1.is(Method) && !sym2.is(Method) then 1
470470
else 0
471471

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

474-
if symScore <= 0 && info2.overrides(info1, matchLoosely, checkClassInfo = false) then
475+
if symScore <= 0 && info2.overrides(info1, relaxedOverriding, matchLoosely, checkClassInfo = false) then
475476
denot2
476-
else if symScore >= 0 && info1.overrides(info2, matchLoosely, checkClassInfo = false) then
477+
else if symScore >= 0 && info1.overrides(info2, relaxedOverriding, matchLoosely, checkClassInfo = false) then
477478
denot1
478479
else
479480
val jointInfo = infoMeet(info1, info2, safeIntersection)

‎compiler/src/dotty/tools/dotc/core/Types.scala‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1071,21 +1071,25 @@ object Types {
10711071

10721072
/** Is this type a legal type for member `sym1` that overrides another
10731073
* member `sym2` of type `that`? This is the same as `<:<`, except that
1074+
* @param relaxedCheck if true type `Null` becomes a subtype of non-primitive value types in TypeComparer.
10741075
* @param matchLoosely if true the types `=> T` and `()T` are seen as overriding each other.
10751076
* @param checkClassInfo if true we check that ClassInfos are within bounds of abstract types
10761077
*/
1077-
final def overrides(that: Type, matchLoosely: => Boolean, checkClassInfo: Boolean = true)(using Context): Boolean = {
1078+
final def overrides(that: Type, relaxedCheck: Boolean, matchLoosely: => Boolean, checkClassInfo: Boolean = true)(using Context): Boolean = {
10781079
def widenNullary(tp: Type) = tp match {
10791080
case tp @ MethodType(Nil) => tp.resultType
10801081
case _ => tp
10811082
}
1082-
!checkClassInfo && this.isInstanceOf[ClassInfo]
1083-
|| (this.widenExpr frozen_<:< that.widenExpr)
1084-
|| matchLoosely && {
1085-
val this1 = widenNullary(this)
1086-
val that1 = widenNullary(that)
1087-
((this1 `ne` this) || (that1 `ne` that)) && this1.overrides(that1, false, checkClassInfo)
1088-
}
1083+
val overrideCtx = if relaxedCheck then ctx.relaxedOverrideContext else ctx
1084+
inContext(overrideCtx) {
1085+
!checkClassInfo && this.isInstanceOf[ClassInfo]
1086+
|| (this.widenExpr frozen_<:< that.widenExpr)
1087+
|| matchLoosely && {
1088+
val this1 = widenNullary(this)
1089+
val that1 = widenNullary(that)
1090+
((this1 `ne` this) || (that1 `ne` that)) && this1.overrides(that1, relaxedCheck, false, checkClassInfo)
1091+
}
1092+
}
10891093
}
10901094

10911095
/** Is this type close enough to that type so that members

‎compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -217,11 +217,9 @@ object OverridingPairs:
217217
else
218218
// releaxed override check for explicit nulls if one of the symbols is Java defined,
219219
// force `Null` to be a subtype of non-primitive value types during override checking.
220-
val overrideCtx = if ctx.explicitNulls && (member.is(JavaDefined) || other.is(JavaDefined))
221-
then ctx.relaxedOverrideContext else ctx
220+
val relaxedOverriding = ctx.explicitNulls && (member.is(JavaDefined) || other.is(JavaDefined))
222221
member.name.is(DefaultGetterName) // default getters are not checked for compatibility
223-
|| memberTp.overrides(otherTp,
224-
member.matchNullaryLoosely || other.matchNullaryLoosely || fallBack
225-
)(using overrideCtx)
222+
|| memberTp.overrides(otherTp, relaxedOverriding,
223+
member.matchNullaryLoosely || other.matchNullaryLoosely || fallBack)
226224

227225
end OverridingPairs

‎compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,9 +113,8 @@ object ResolveSuper {
113113
// Since the super class can be Java defined,
114114
// we use relaxed overriding check for explicit nulls if one of the symbols is Java defined.
115115
// This forces `Null` to be a subtype of non-primitive value types during override checking.
116-
val overrideCtx = if ctx.explicitNulls && (sym.is(JavaDefined) || acc.is(JavaDefined))
117-
then ctx.relaxedOverrideContext else ctx
118-
if !otherTp.overrides(accTp, matchLoosely = true)(using overrideCtx) then
116+
val relaxedOverriding = ctx.explicitNulls && (sym.is(JavaDefined) || acc.is(JavaDefined))
117+
if !otherTp.overrides(accTp, relaxedOverriding, matchLoosely = true) then
119118
report.error(IllegalSuperAccessor(base, memberName, targetName, acc, accTp, other.symbol, otherTp), base.srcPos)
120119
bcs = bcs.tail
121120
}

‎compiler/src/dotty/tools/dotc/typer/RefChecks.scala‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -767,7 +767,7 @@ object RefChecks {
767767
for (mbrd <- self.member(name).alternatives) {
768768
val mbr = mbrd.symbol
769769
val mbrType = mbr.info.asSeenFrom(self, mbr.owner)
770-
if (!mbrType.overrides(mbrd.info, matchLoosely = true))
770+
if (!mbrType.overrides(mbrd.info, relaxedCheck = false, matchLoosely = true))
771771
report.errorOrMigrationWarning(
772772
em"""${mbr.showLocated} is not a legal implementation of `$name` in $clazz
773773
| its type $mbrType

‎compiler/test/dotty/tools/dotc/CompilationTests.scala‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ class CompilationTests {
5252
),
5353
compileFile("tests/pos-special/typeclass-scaling.scala", defaultOptions.and("-Xmax-inlines", "40")),
5454
compileFile("tests/pos-special/i7296.scala", defaultOptions.and("-source", "future", "-deprecation", "-Xfatal-warnings")),
55-
compileFile("tests/pos-special/notNull.scala", defaultOptions.and("-Yexplicit-nulls")),
5655
compileDir("tests/pos-special/adhoc-extension", defaultOptions.and("-source", "future", "-feature", "-Xfatal-warnings")),
5756
compileFile("tests/pos-special/i7575.scala", defaultOptions.andLanguageFeature("dynamics")),
5857
compileFile("tests/pos-special/kind-projector.scala", defaultOptions.and("-Ykind-projector")),
@@ -137,7 +136,6 @@ class CompilationTests {
137136
compileFilesInDir("tests/neg-custom-args/erased", defaultOptions.and("-language:experimental.erasedDefinitions")),
138137
compileFilesInDir("tests/neg-custom-args/allow-double-bindings", allowDoubleBindings),
139138
compileFilesInDir("tests/neg-custom-args/allow-deep-subtypes", allowDeepSubtypes),
140-
compileFilesInDir("tests/neg-custom-args/explicit-nulls", defaultOptions.and("-Yexplicit-nulls")),
141139
compileFilesInDir("tests/neg-custom-args/no-experimental", defaultOptions.and("-Yno-experimental")),
142140
compileDir("tests/neg-custom-args/impl-conv", defaultOptions.and("-Xfatal-warnings", "-feature")),
143141
compileDir("tests/neg-custom-args/i13946", defaultOptions.and("-Xfatal-warnings", "-feature")),
@@ -249,6 +247,7 @@ class CompilationTests {
249247
compileFilesInDir("tests/explicit-nulls/pos-separate", explicitNullsOptions),
250248
compileFilesInDir("tests/explicit-nulls/pos-patmat", explicitNullsOptions and "-Xfatal-warnings"),
251249
compileFilesInDir("tests/explicit-nulls/unsafe-common", explicitNullsOptions and "-language:unsafeNulls"),
250+
compileFile("tests/explicit-nulls/pos-special/i14682.scala", explicitNullsOptions and "-Ysafe-init"),
252251
)
253252
}.checkCompile()
254253

‎tests/neg-custom-args/explicit-nulls/byname-nullables.check‎ renamed to ‎tests/explicit-nulls/neg/byname-nullables.check‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,27 @@
1-
-- [E007] Type Mismatch Error: tests/neg-custom-args/explicit-nulls/byname-nullables.scala:19:24 -----------------------
1+
-- [E007] Type Mismatch Error: tests/explicit-nulls/neg/byname-nullables.scala:19:24 -----------------------------------
22
19 | if x != null then f(x) // error: f is call-by-name
33
| ^
44
| Found: (x : String | Null)
55
| Required: String
66
|
77
| longer explanation available when compiling with `-explain`
8-
-- Error: tests/neg-custom-args/explicit-nulls/byname-nullables.scala:43:32 --------------------------------------------
8+
-- Error: tests/explicit-nulls/neg/byname-nullables.scala:43:32 --------------------------------------------------------
99
43 | if x != null then f(identity(x), 1) // error: dropping not null check fails typing
1010
| ^^^^^^^^^^^
1111
| This argument was typed using flow assumptions about mutable variables
1212
| but it is passed to a by-name parameter where such flow assumptions are unsound.
1313
| Wrapping the argument in `byName(...)` fixes the problem by disabling the flow assumptions.
1414
|
1515
| `byName` needs to be imported from the `scala.compiletime` package.
16-
-- Error: tests/neg-custom-args/explicit-nulls/byname-nullables.scala:68:24 --------------------------------------------
16+
-- Error: tests/explicit-nulls/neg/byname-nullables.scala:68:24 --------------------------------------------------------
1717
68 | if x != null then f(x, 1) // error: dropping not null check typechecks OK, but gives incompatible result type
1818
| ^
1919
| This argument was typed using flow assumptions about mutable variables
2020
| but it is passed to a by-name parameter where such flow assumptions are unsound.
2121
| Wrapping the argument in `byName(...)` fixes the problem by disabling the flow assumptions.
2222
|
2323
| `byName` needs to be imported from the `scala.compiletime` package.
24-
-- [E134] Type Error: tests/neg-custom-args/explicit-nulls/byname-nullables.scala:81:22 --------------------------------
24+
-- [E134] Type Error: tests/explicit-nulls/neg/byname-nullables.scala:81:22 --------------------------------------------
2525
81 | if x != null then f(byName(x), 1) // error: none of the overloaded methods match argument types
2626
| ^
2727
| None of the overloaded alternatives of method f in object Test7 with types

‎tests/neg-custom-args/explicit-nulls/byname-nullables.scala‎ renamed to ‎tests/explicit-nulls/neg/byname-nullables.scala‎

File renamed without changes.

‎tests/neg-custom-args/explicit-nulls/byname-nullables1.check‎ renamed to ‎tests/explicit-nulls/neg/byname-nullables1.check‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
-- Error: tests/neg-custom-args/explicit-nulls/byname-nullables1.scala:10:6 --------------------------------------------
1+
-- Error: tests/explicit-nulls/neg/byname-nullables1.scala:10:6 --------------------------------------------------------
22
10 | f(x.fld != null) // error
33
| ^^^^^^^^^^^^^
44
| This argument was typed using flow assumptions about mutable variables

‎tests/neg-custom-args/explicit-nulls/byname-nullables1.scala‎ renamed to ‎tests/explicit-nulls/neg/byname-nullables1.scala‎

File renamed without changes.

0 commit comments

Comments
 (0)