From b71e262a8561760e70e69ae8b8fcd844705daf61 Mon Sep 17 00:00:00 2001 From: noti0na1 Date: Thu, 18 Nov 2021 22:02:15 -0500 Subject: [PATCH 1/4] Fix space in unsafe nulls --- .../src/dotty/tools/dotc/core/SymDenotations.scala | 2 +- .../dotty/tools/dotc/transform/PatternMatcher.scala | 7 ++++++- .../dotty/tools/dotc/transform/patmat/Space.scala | 12 +++++------- compiler/src/dotty/tools/dotc/typer/Nullables.scala | 7 +++++++ compiler/src/dotty/tools/dotc/typer/Typer.scala | 5 ++++- .../unsafe-common/unsafe-match-null.scala | 12 ++++++++++++ 6 files changed, 35 insertions(+), 10 deletions(-) create mode 100644 tests/explicit-nulls/unsafe-common/unsafe-match-null.scala diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 2f7e3debfa6f..5c3545bb2f65 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -813,7 +813,7 @@ object SymDenotations { /** Is this symbol a class of which `null` is a value? */ final def isNullableClass(using Context): Boolean = if ctx.mode.is(Mode.SafeNulls) && !ctx.phase.erasedTypes - then symbol == defn.NullClass || symbol == defn.AnyClass + then symbol == defn.NullClass || symbol == defn.AnyClass || symbol == defn.MatchableClass else isNullableClassAfterErasure /** Is this symbol a class of which `null` is a value after erasure? diff --git a/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala b/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala index 5ba43253e8b3..fb4657f9d01a 100644 --- a/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala +++ b/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala @@ -13,6 +13,7 @@ import SymUtils._ import Flags._, Constants._ import Decorators._ import NameKinds.{PatMatStdBinderName, PatMatAltsName, PatMatResultName} +import typer.Nullables import config.Printers.patmatch import reporting._ import dotty.tools.dotc.ast._ @@ -43,8 +44,12 @@ class PatternMatcher extends MiniPhase { case rt => tree.tpe val translated = new Translator(matchType, this).translateMatch(tree) + val engineCtx = + if tree.hasAttachment(Nullables.UnsafeNullsMatch) + then ctx.retractMode(Mode.SafeNulls) else ctx + // check exhaustivity and unreachability - val engine = new patmat.SpaceEngine + val engine = new patmat.SpaceEngine()(using engineCtx) engine.checkExhaustivity(tree) engine.checkRedundancy(tree) diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index 13310299ed00..dcb946a45ec3 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -338,16 +338,13 @@ class SpaceEngine(using Context) extends SpaceLogic { override def intersectUnrelatedAtomicTypes(tp1: Type, tp2: Type): Space = trace(s"atomic intersection: ${AndType(tp1, tp2).show}", debug) { // Precondition: !isSubType(tp1, tp2) && !isSubType(tp2, tp1). - if (!ctx.explicitNulls && (tp1.isNullType || tp2.isNullType)) { + if !ctx.mode.is(Mode.SafeNulls) && (tp1.isNullType || tp2.isNullType) then // Since projections of types don't include null, intersection with null is empty. Empty - } - else { + else val res = TypeComparer.provablyDisjoint(tp1, tp2) - - if (res) Empty + if res then Empty else Typ(AndType(tp1, tp2), decomposed = true) - } } /** Return the space that represents the pattern `pat` */ @@ -549,7 +546,8 @@ class SpaceEngine(using Context) extends SpaceLogic { /** Is `tp1` a subtype of `tp2`? */ def isSubType(tp1: Type, tp2: Type): Boolean = trace(i"$tp1 <:< $tp2", debug, show = true) { - if tp1 == constantNullType && !ctx.explicitNulls then tp2 == constantNullType + if tp1 == constantNullType && !ctx.mode.is(Mode.SafeNulls) + then tp2 == constantNullType else adaptType(tp1, tp2) <:< tp2 } diff --git a/compiler/src/dotty/tools/dotc/typer/Nullables.scala b/compiler/src/dotty/tools/dotc/typer/Nullables.scala index 0ea7207c7b2b..dc3957e78510 100644 --- a/compiler/src/dotty/tools/dotc/typer/Nullables.scala +++ b/compiler/src/dotty/tools/dotc/typer/Nullables.scala @@ -20,6 +20,13 @@ import ast.Trees.mods object Nullables: import ast.tpd._ + /** An attachment that represents a match tree is created under Unsafe Nulls. + * This is used to pass Unsafe Nulls information to PatternMatcher Phase, + * so we don't get Match case Unreachable Warning when using `case null => ???` + * on non-nullable type. + */ + val UnsafeNullsMatch = Property.StickyKey[Unit] + inline def unsafeNullsEnabled(using Context): Boolean = ctx.explicitNulls && !ctx.mode.is(Mode.SafeNulls) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 02f3f6b4f164..c1a10c8f5846 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1499,7 +1499,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer } def typedMatch(tree: untpd.Match, pt: Type)(using Context): Tree = - tree.selector match { + val tree1 = tree.selector match { case EmptyTree => if (tree.isInline) { checkInInlineContext("summonFrom", tree.srcPos) @@ -1601,6 +1601,9 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer result } } + if Nullables.unsafeNullsEnabled then + tree1.putAttachment(Nullables.UnsafeNullsMatch, ()) + tree1 /** Special typing of Match tree when the expected type is a MatchType, * and the patterns of the Match tree and the MatchType correspond. diff --git a/tests/explicit-nulls/unsafe-common/unsafe-match-null.scala b/tests/explicit-nulls/unsafe-common/unsafe-match-null.scala new file mode 100644 index 000000000000..4aa85d5ba9ae --- /dev/null +++ b/tests/explicit-nulls/unsafe-common/unsafe-match-null.scala @@ -0,0 +1,12 @@ +def test1 = + val s: String = ??? + s match + case _: String => + // under unsafeNulls, we should not get Match case Unreachable Warning + case null => // error + +def test2 = + val s: String | Null = ??? + s match + case _: String => + case null => From 60d7781b5f35c3fd802e6dd34c4e9d6c859b5aef Mon Sep 17 00:00:00 2001 From: noti0na1 Date: Fri, 19 Nov 2021 01:04:44 -0500 Subject: [PATCH 2/4] Add patman test --- .../neg-patmat/{patmat1.scala => match-pat.scala} | 14 +++++++------- .../neg-patmat/unsafe-match-null-pat.scala | 14 ++++++++++++++ .../unsafe-common/unsafe-match-null.scala | 3 +-- 3 files changed, 22 insertions(+), 9 deletions(-) rename tests/explicit-nulls/neg-patmat/{patmat1.scala => match-pat.scala} (66%) create mode 100644 tests/explicit-nulls/neg-patmat/unsafe-match-null-pat.scala diff --git a/tests/explicit-nulls/neg-patmat/patmat1.scala b/tests/explicit-nulls/neg-patmat/match-pat.scala similarity index 66% rename from tests/explicit-nulls/neg-patmat/patmat1.scala rename to tests/explicit-nulls/neg-patmat/match-pat.scala index a73b1fca2e1e..fb7180d82e34 100644 --- a/tests/explicit-nulls/neg-patmat/patmat1.scala +++ b/tests/explicit-nulls/neg-patmat/match-pat.scala @@ -1,12 +1,9 @@ class Foo { + val s: String = ??? - s match { - case s: String => 100 // warning: type test will always succeed - case _ => 200 // error: unreachable - } s match { - case s: String => 100 // warning: type test will always succeed + case s: String => 100 case _ => 200 // error: unreachable } @@ -15,20 +12,23 @@ class Foo { case object Cat extends Animal val a: Animal = ??? + a match { case Dog(name) => 100 case Cat => 200 case _ => 300 // error: unreachable } - val a2: Animal|Null = ??? + val a2: Animal | Null = ??? + a2 match { case Dog(_) => 100 case Cat => 200 case _ => 300 } - val a3: Animal|Null = ??? + val a3: Animal | Null = ??? + a3 match { case Dog(_) => 100 case Cat => 200 diff --git a/tests/explicit-nulls/neg-patmat/unsafe-match-null-pat.scala b/tests/explicit-nulls/neg-patmat/unsafe-match-null-pat.scala new file mode 100644 index 000000000000..143a79d1e2f0 --- /dev/null +++ b/tests/explicit-nulls/neg-patmat/unsafe-match-null-pat.scala @@ -0,0 +1,14 @@ +import scala.language.unsafeNulls + +def test1 = + val s: String = ??? + s match + case _: String => + // under unsafeNulls, we should not get Match case Unreachable Warning + case null => // ok + +def test2 = + val s: String | Null = ??? + s match + case _: String => + case null => diff --git a/tests/explicit-nulls/unsafe-common/unsafe-match-null.scala b/tests/explicit-nulls/unsafe-common/unsafe-match-null.scala index 4aa85d5ba9ae..fc76de6ddd3a 100644 --- a/tests/explicit-nulls/unsafe-common/unsafe-match-null.scala +++ b/tests/explicit-nulls/unsafe-common/unsafe-match-null.scala @@ -2,8 +2,7 @@ def test1 = val s: String = ??? s match case _: String => - // under unsafeNulls, we should not get Match case Unreachable Warning - case null => // error + case null => // error: Values of types Null and String cannot be compared def test2 = val s: String | Null = ??? From 37dcadafe69cfdff17bf30dd37c16808295b25d8 Mon Sep 17 00:00:00 2001 From: noti0na1 Date: Fri, 19 Nov 2021 03:42:35 -0500 Subject: [PATCH 3/4] Fix tests in Ycheck --- compiler/src/dotty/tools/dotc/transform/patmat/Space.scala | 6 ------ compiler/src/dotty/tools/dotc/typer/Typer.scala | 2 +- compiler/test/dotty/tools/dotc/CompilationTests.scala | 1 + .../{neg-patmat => pos-patmat}/unsafe-match-null-pat.scala | 0 4 files changed, 2 insertions(+), 7 deletions(-) rename tests/explicit-nulls/{neg-patmat => pos-patmat}/unsafe-match-null-pat.scala (100%) diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index dcb946a45ec3..7fbd6ba22178 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -330,12 +330,6 @@ class SpaceEngine(using Context) extends SpaceLogic { private val constantNullType = ConstantType(Constant(null)) - /** Does the given tree stand for the literal `null`? */ - def isNullLit(tree: Tree): Boolean = tree match { - case Literal(Constant(null)) => true - case _ => false - } - override def intersectUnrelatedAtomicTypes(tp1: Type, tp2: Type): Space = trace(s"atomic intersection: ${AndType(tp1, tp2).show}", debug) { // Precondition: !isSubType(tp1, tp2) && !isSubType(tp2, tp1). if !ctx.mode.is(Mode.SafeNulls) && (tp1.isNullType || tp2.isNullType) then diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index c1a10c8f5846..112a5dd7fd18 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1601,7 +1601,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer result } } - if Nullables.unsafeNullsEnabled then + if Nullables.unsafeNullsEnabled && ctx.phase == Phases.typerPhase then tree1.putAttachment(Nullables.UnsafeNullsMatch, ()) tree1 diff --git a/compiler/test/dotty/tools/dotc/CompilationTests.scala b/compiler/test/dotty/tools/dotc/CompilationTests.scala index 88eab8d131e6..4ea8db553f54 100644 --- a/compiler/test/dotty/tools/dotc/CompilationTests.scala +++ b/compiler/test/dotty/tools/dotc/CompilationTests.scala @@ -247,6 +247,7 @@ class CompilationTests { aggregateTests( compileFilesInDir("tests/explicit-nulls/pos", explicitNullsOptions), 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"), ) }.checkCompile() diff --git a/tests/explicit-nulls/neg-patmat/unsafe-match-null-pat.scala b/tests/explicit-nulls/pos-patmat/unsafe-match-null-pat.scala similarity index 100% rename from tests/explicit-nulls/neg-patmat/unsafe-match-null-pat.scala rename to tests/explicit-nulls/pos-patmat/unsafe-match-null-pat.scala From d296dd4cd5b7072d9b53a46d338bc9d68ee23300 Mon Sep 17 00:00:00 2001 From: noti0na1 Date: Mon, 31 Jan 2022 13:26:43 -0500 Subject: [PATCH 4/4] Remove StickyKey --- .../src/dotty/tools/dotc/transform/PatternMatcher.scala | 7 +------ compiler/src/dotty/tools/dotc/typer/Nullables.scala | 7 ------- compiler/src/dotty/tools/dotc/typer/Typer.scala | 5 +---- 3 files changed, 2 insertions(+), 17 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala b/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala index fb4657f9d01a..5ba43253e8b3 100644 --- a/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala +++ b/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala @@ -13,7 +13,6 @@ import SymUtils._ import Flags._, Constants._ import Decorators._ import NameKinds.{PatMatStdBinderName, PatMatAltsName, PatMatResultName} -import typer.Nullables import config.Printers.patmatch import reporting._ import dotty.tools.dotc.ast._ @@ -44,12 +43,8 @@ class PatternMatcher extends MiniPhase { case rt => tree.tpe val translated = new Translator(matchType, this).translateMatch(tree) - val engineCtx = - if tree.hasAttachment(Nullables.UnsafeNullsMatch) - then ctx.retractMode(Mode.SafeNulls) else ctx - // check exhaustivity and unreachability - val engine = new patmat.SpaceEngine()(using engineCtx) + val engine = new patmat.SpaceEngine engine.checkExhaustivity(tree) engine.checkRedundancy(tree) diff --git a/compiler/src/dotty/tools/dotc/typer/Nullables.scala b/compiler/src/dotty/tools/dotc/typer/Nullables.scala index dc3957e78510..0ea7207c7b2b 100644 --- a/compiler/src/dotty/tools/dotc/typer/Nullables.scala +++ b/compiler/src/dotty/tools/dotc/typer/Nullables.scala @@ -20,13 +20,6 @@ import ast.Trees.mods object Nullables: import ast.tpd._ - /** An attachment that represents a match tree is created under Unsafe Nulls. - * This is used to pass Unsafe Nulls information to PatternMatcher Phase, - * so we don't get Match case Unreachable Warning when using `case null => ???` - * on non-nullable type. - */ - val UnsafeNullsMatch = Property.StickyKey[Unit] - inline def unsafeNullsEnabled(using Context): Boolean = ctx.explicitNulls && !ctx.mode.is(Mode.SafeNulls) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 112a5dd7fd18..02f3f6b4f164 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1499,7 +1499,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer } def typedMatch(tree: untpd.Match, pt: Type)(using Context): Tree = - val tree1 = tree.selector match { + tree.selector match { case EmptyTree => if (tree.isInline) { checkInInlineContext("summonFrom", tree.srcPos) @@ -1601,9 +1601,6 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer result } } - if Nullables.unsafeNullsEnabled && ctx.phase == Phases.typerPhase then - tree1.putAttachment(Nullables.UnsafeNullsMatch, ()) - tree1 /** Special typing of Match tree when the expected type is a MatchType, * and the patterns of the Match tree and the MatchType correspond.