From 51dc1af94ff77d9cb3e2b23930cd322dcc79ee1f Mon Sep 17 00:00:00 2001 From: Ondrej Lhotak Date: Wed, 14 Aug 2024 16:27:06 -0400 Subject: [PATCH 1/4] add tracking of NotNullInfo for Match, Case, Try trees (fix #21380) [Cherry-picked 10497c9a9bafdb36a84250584b0eaa2d7b1b6b07] --- .../src/dotty/tools/dotc/typer/Typer.scala | 20 +++++++++++++++---- tests/explicit-nulls/neg/i21380.scala | 19 ++++++++++++++++++ tests/explicit-nulls/neg/i21380b.scala | 8 ++++++++ 3 files changed, 43 insertions(+), 4 deletions(-) create mode 100644 tests/explicit-nulls/neg/i21380.scala create mode 100644 tests/explicit-nulls/neg/i21380b.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index f2d6cf0715ad..07dc1514b9b8 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1856,14 +1856,18 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer case1 } .asInstanceOf[List[CaseDef]] - assignType(cpy.Match(tree)(sel, cases1), sel, cases1).cast(pt) + var nni = sel.notNullInfo + if(cases1.nonEmpty) nni = nni.seq(cases1.map(_.notNullInfo).reduce(_.alt(_))) + assignType(cpy.Match(tree)(sel, cases1), sel, cases1).cast(pt).withNotNullInfo(nni) } // Overridden in InlineTyper for inline matches def typedMatchFinish(tree: untpd.Match, sel: Tree, wideSelType: Type, cases: List[untpd.CaseDef], pt: Type)(using Context): Tree = { val cases1 = harmonic(harmonize, pt)(typedCases(cases, sel, wideSelType, pt.dropIfProto)) .asInstanceOf[List[CaseDef]] - assignType(cpy.Match(tree)(sel, cases1), sel, cases1) + var nni = sel.notNullInfo + if(cases1.nonEmpty) nni = nni.seq(cases1.map(_.notNullInfo).reduce(_.alt(_))) + assignType(cpy.Match(tree)(sel, cases1), sel, cases1).withNotNullInfo(nni) } def typedCases(cases: List[untpd.CaseDef], sel: Tree, wideSelType: Type, pt: Type)(using Context): List[CaseDef] = @@ -1935,7 +1939,12 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer pat1.putAttachment(InferredGadtConstraints, ctx.gadt) if (pt1.isValueType) // insert a cast if body does not conform to expected type if we disregard gadt bounds body1 = body1.ensureConforms(pt1)(using originalCtx) - assignType(cpy.CaseDef(tree)(pat1, guard1, body1), pat1, body1) + val nni = pat1.notNullInfo.seq( + guard1.notNullInfoIf(false).alt( + guard1.notNullInfoIf(true).seq(body1.notNullInfo) + ) + ) + assignType(cpy.CaseDef(tree)(pat1, guard1, body1), pat1, body1).withNotNullInfo(nni) } val pat1 = typedPattern(tree.pat, wideSelType)(using gadtCtx) @@ -2046,7 +2055,10 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer }: @unchecked val finalizer1 = typed(tree.finalizer, defn.UnitType) val cases2 = cases2x.asInstanceOf[List[CaseDef]] - assignType(cpy.Try(tree)(expr2, cases2, finalizer1), expr2, cases2) + val nni = expr2.notNullInfo.retractedInfo.seq( + cases2.map(_.notNullInfo.retractedInfo).fold(NotNullInfo.empty)(_.alt(_)) + ).seq(finalizer1.notNullInfo) + assignType(cpy.Try(tree)(expr2, cases2, finalizer1), expr2, cases2).withNotNullInfo(nni) } def typedTry(tree: untpd.ParsedTry, pt: Type)(using Context): Try = diff --git a/tests/explicit-nulls/neg/i21380.scala b/tests/explicit-nulls/neg/i21380.scala new file mode 100644 index 000000000000..685aa09ef818 --- /dev/null +++ b/tests/explicit-nulls/neg/i21380.scala @@ -0,0 +1,19 @@ +@main def test() = { + var x: String | Null = null + if (false) { + x = "" + + } else { + x = "" + } + try { + x = "" + throw new Exception() + } + catch { + case e: Exception => { + x = null + } + } + x.replace("", "") // error +} diff --git a/tests/explicit-nulls/neg/i21380b.scala b/tests/explicit-nulls/neg/i21380b.scala new file mode 100644 index 000000000000..b371dfcd743f --- /dev/null +++ b/tests/explicit-nulls/neg/i21380b.scala @@ -0,0 +1,8 @@ +@main def test() = { + var x: String | Null = null + x = "" + 1 match { + case 1 => x = null + } + x.replace("", "") // error +} From 6ff6b10d26fc02d8851e121b8091040bb5ae1e75 Mon Sep 17 00:00:00 2001 From: noti0na1 Date: Thu, 15 Aug 2024 16:19:35 +0200 Subject: [PATCH 2/4] Fix NotNullInfo for Case and Try; add more tests [Cherry-picked 7674fefae3e9c9c9dffe880c9b2b799049e90a10] --- .../src/dotty/tools/dotc/typer/Typer.scala | 35 +++++++++++-------- tests/explicit-nulls/neg/i21380b.scala | 14 +++++--- tests/explicit-nulls/neg/i21380c.scala | 34 ++++++++++++++++++ 3 files changed, 65 insertions(+), 18 deletions(-) create mode 100644 tests/explicit-nulls/neg/i21380c.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 07dc1514b9b8..546e3fcd0901 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1857,7 +1857,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer } .asInstanceOf[List[CaseDef]] var nni = sel.notNullInfo - if(cases1.nonEmpty) nni = nni.seq(cases1.map(_.notNullInfo).reduce(_.alt(_))) + if cases1.nonEmpty then nni = nni.seq(cases1.map(_.notNullInfo).reduce(_.alt(_))) assignType(cpy.Match(tree)(sel, cases1), sel, cases1).cast(pt).withNotNullInfo(nni) } @@ -1866,7 +1866,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer val cases1 = harmonic(harmonize, pt)(typedCases(cases, sel, wideSelType, pt.dropIfProto)) .asInstanceOf[List[CaseDef]] var nni = sel.notNullInfo - if(cases1.nonEmpty) nni = nni.seq(cases1.map(_.notNullInfo).reduce(_.alt(_))) + if cases1.nonEmpty then nni = nni.seq(cases1.map(_.notNullInfo).reduce(_.alt(_))) assignType(cpy.Match(tree)(sel, cases1), sel, cases1).withNotNullInfo(nni) } @@ -1937,13 +1937,11 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer // will end up taking too much memory. If it does, we should just limit // how much GADT constraints we infer - it's always sound to infer less. pat1.putAttachment(InferredGadtConstraints, ctx.gadt) - if (pt1.isValueType) // insert a cast if body does not conform to expected type if we disregard gadt bounds + if pt1.isValueType then // insert a cast if body does not conform to expected type if we disregard gadt bounds body1 = body1.ensureConforms(pt1)(using originalCtx) - val nni = pat1.notNullInfo.seq( - guard1.notNullInfoIf(false).alt( - guard1.notNullInfoIf(true).seq(body1.notNullInfo) - ) - ) + val nni = pat1.notNullInfo + .seq(guard1.notNullInfoIf(false).alt(guard1.notNullInfoIf(true))) + .seq(body1.notNullInfo) assignType(cpy.CaseDef(tree)(pat1, guard1, body1), pat1, body1).withNotNullInfo(nni) } @@ -2048,16 +2046,25 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer untpd.Block(makeCanThrow(capabilityProof), expr) def typedTry(tree: untpd.Try, pt: Type)(using Context): Try = { + // We want to type check tree.expr first to comput NotNullInfo, but `addCanThrowCapabilities` + // uses the types of patterns in `tree.cases` to determine the capabilities. + // Hence, we create a copy of cases with empty body and type check that first, then type check + // the rest of the tree in order. + val casesEmptyBody1 = tree.cases.mapconserve(cpy.CaseDef(_)(body = EmptyTree)) + val casesEmptyBody2 = typedCases(casesEmptyBody1, EmptyTree, defn.ThrowableType, WildcardType) + val expr2 :: cases2x = harmonic(harmonize, pt) { - val cases1 = typedCases(tree.cases, EmptyTree, defn.ThrowableType, pt.dropIfProto) - val expr1 = typed(addCanThrowCapabilities(tree.expr, cases1), pt.dropIfProto) + val expr1 = typed(addCanThrowCapabilities(tree.expr, casesEmptyBody2), pt.dropIfProto) + val casesCtx = ctx.addNotNullInfo(expr1.notNullInfo.retractedInfo) + val cases1 = typedCases(tree.cases, EmptyTree, defn.ThrowableType, pt.dropIfProto)(using casesCtx) expr1 :: cases1 }: @unchecked - val finalizer1 = typed(tree.finalizer, defn.UnitType) val cases2 = cases2x.asInstanceOf[List[CaseDef]] - val nni = expr2.notNullInfo.retractedInfo.seq( - cases2.map(_.notNullInfo.retractedInfo).fold(NotNullInfo.empty)(_.alt(_)) - ).seq(finalizer1.notNullInfo) + + var nni = expr2.notNullInfo.retractedInfo + if cases2.nonEmpty then nni = nni.seq(cases2.map(_.notNullInfo).reduce(_.alt(_))) + val finalizer1 = typed(tree.finalizer, defn.UnitType)(using ctx.addNotNullInfo(nni)) + nni = nni.seq(finalizer1.notNullInfo) assignType(cpy.Try(tree)(expr2, cases2, finalizer1), expr2, cases2).withNotNullInfo(nni) } diff --git a/tests/explicit-nulls/neg/i21380b.scala b/tests/explicit-nulls/neg/i21380b.scala index b371dfcd743f..55a5fcf5bb60 100644 --- a/tests/explicit-nulls/neg/i21380b.scala +++ b/tests/explicit-nulls/neg/i21380b.scala @@ -1,8 +1,14 @@ -@main def test() = { +def test1 = var x: String | Null = null x = "" - 1 match { + 1 match case 1 => x = null - } + case _ => x = x.trim() // ok x.replace("", "") // error -} + +def test2(i: Int) = + var x: String | Null = null + i match + case 1 => x = "1" + case _ => x = " " + x.replace("", "") // ok \ No newline at end of file diff --git a/tests/explicit-nulls/neg/i21380c.scala b/tests/explicit-nulls/neg/i21380c.scala new file mode 100644 index 000000000000..4fea14f2f124 --- /dev/null +++ b/tests/explicit-nulls/neg/i21380c.scala @@ -0,0 +1,34 @@ +def test1(i: Int): Int = + var x: String | Null = null + if i == 0 then x = "" + else x = "" + try + x = x.replace(" ", "") // ok + throw new Exception() + catch + case e: Exception => + x = x.replaceAll(" ", "") // error + x = null + x.length // error + +def test2: Int = + var x: String | Null = null + try throw new Exception() + finally x = "" + x.length // ok + +def test3 = + var x: String | Null = "" + try throw new Exception() + catch case e: Exception => + x = (??? : String | Null) + finally + val l = x.length // error + +def test4: Int = + var x: String | Null = null + try throw new Exception() + catch + case npe: NullPointerException => x = "" + case _ => x = "" + x.length // ok \ No newline at end of file From d2c9b3fb1d97eba4b2f887db96770f56ebc6aaad Mon Sep 17 00:00:00 2001 From: noti0na1 Date: Wed, 28 Aug 2024 15:56:12 +0200 Subject: [PATCH 3/4] Fix typing cases [Cherry-picked 7f92b53fa5670c64c8419e680a0b0d85d493e959] --- compiler/src/dotty/tools/dotc/typer/Typer.scala | 8 +++++--- tests/explicit-nulls/neg/i21380b.scala | 9 ++++++++- tests/explicit-nulls/neg/i21380c.scala | 13 ++++++++++++- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 546e3fcd0901..527e5d4057ef 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1929,7 +1929,9 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer } val pat1 = indexPattern(tree).transform(pat) val guard1 = typedExpr(tree.guard, defn.BooleanType) - var body1 = ensureNoLocalRefs(typedExpr(tree.body, pt1), pt1, ctx.scope.toList) + var body1 = ensureNoLocalRefs( + typedExpr(tree.body, pt1)(using ctx.addNotNullInfo(guard1.notNullInfoIf(true))), + pt1, ctx.scope.toList) if ctx.gadt.isNarrowing then // Store GADT constraint to later retrieve it (in PostTyper, for now). // GADT constraints are necessary to correctly check bounds of type app, @@ -1940,7 +1942,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer if pt1.isValueType then // insert a cast if body does not conform to expected type if we disregard gadt bounds body1 = body1.ensureConforms(pt1)(using originalCtx) val nni = pat1.notNullInfo - .seq(guard1.notNullInfoIf(false).alt(guard1.notNullInfoIf(true))) + .seq(guard1.notNullInfoIf(true)) .seq(body1.notNullInfo) assignType(cpy.CaseDef(tree)(pat1, guard1, body1), pat1, body1).withNotNullInfo(nni) } @@ -2062,7 +2064,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer val cases2 = cases2x.asInstanceOf[List[CaseDef]] var nni = expr2.notNullInfo.retractedInfo - if cases2.nonEmpty then nni = nni.seq(cases2.map(_.notNullInfo).reduce(_.alt(_))) + if cases2.nonEmpty then nni = nni.seq(cases2.map(_.notNullInfo.retractedInfo).reduce(_.alt(_))) val finalizer1 = typed(tree.finalizer, defn.UnitType)(using ctx.addNotNullInfo(nni)) nni = nni.seq(finalizer1.notNullInfo) assignType(cpy.Try(tree)(expr2, cases2, finalizer1), expr2, cases2).withNotNullInfo(nni) diff --git a/tests/explicit-nulls/neg/i21380b.scala b/tests/explicit-nulls/neg/i21380b.scala index 55a5fcf5bb60..83e23053547c 100644 --- a/tests/explicit-nulls/neg/i21380b.scala +++ b/tests/explicit-nulls/neg/i21380b.scala @@ -11,4 +11,11 @@ def test2(i: Int) = i match case 1 => x = "1" case _ => x = " " - x.replace("", "") // ok \ No newline at end of file + x.replace("", "") // ok + +def test3(i: Int) = + var x: String | Null = null + i match + case 1 if x != null => () + case _ => x = " " + x.trim() // ok \ No newline at end of file diff --git a/tests/explicit-nulls/neg/i21380c.scala b/tests/explicit-nulls/neg/i21380c.scala index 4fea14f2f124..f86a5638e4c8 100644 --- a/tests/explicit-nulls/neg/i21380c.scala +++ b/tests/explicit-nulls/neg/i21380c.scala @@ -31,4 +31,15 @@ def test4: Int = catch case npe: NullPointerException => x = "" case _ => x = "" - x.length // ok \ No newline at end of file + x.length // error + // Although the catch block here is exhaustive, + // it is possible that the exception is thrown and not caught. + // Therefore, the code after the try block can only rely on the retracted info. + +def test5: Int = + var x: String | Null = null + try + x = "" + throw new Exception() + catch + case npe: NullPointerException => val i: Int = x.length // error \ No newline at end of file From fdbae420ac2f3bf26dcb1fd0ec0d473f8906e49c Mon Sep 17 00:00:00 2001 From: Wojciech Mazur Date: Tue, 3 Dec 2024 22:18:48 +0100 Subject: [PATCH 4/4] Put i13864 in the blacklist of best-effort test. [Cherry-picked fe0bdad123ea0daef818be993f9b7127c9242482][modified] --- compiler/src/dotty/tools/dotc/typer/Typer.scala | 16 +++++++++------- tests/explicit-nulls/neg/i21380b.scala | 6 +++--- tests/explicit-nulls/neg/i21380c.scala | 4 ++-- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 527e5d4057ef..90210e12dd04 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2048,14 +2048,16 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer untpd.Block(makeCanThrow(capabilityProof), expr) def typedTry(tree: untpd.Try, pt: Type)(using Context): Try = { - // We want to type check tree.expr first to comput NotNullInfo, but `addCanThrowCapabilities` - // uses the types of patterns in `tree.cases` to determine the capabilities. - // Hence, we create a copy of cases with empty body and type check that first, then type check - // the rest of the tree in order. - val casesEmptyBody1 = tree.cases.mapconserve(cpy.CaseDef(_)(body = EmptyTree)) - val casesEmptyBody2 = typedCases(casesEmptyBody1, EmptyTree, defn.ThrowableType, WildcardType) - val expr2 :: cases2x = harmonic(harmonize, pt) { + // We want to type check tree.expr first to comput NotNullInfo, but `addCanThrowCapabilities` + // uses the types of patterns in `tree.cases` to determine the capabilities. + // Hence, we create a copy of cases with empty body and type check that first, then type check + // the rest of the tree in order. + // It may seem that invalid references can be created if the type of the pattern contains + // type binds, but this is not a valid `CanThrow` capability (checked by `addCanThrowCapabilities`), + // so it is not a problem. + val casesEmptyBody1 = tree.cases.mapconserve(cpy.CaseDef(_)(body = EmptyTree)) + val casesEmptyBody2 = typedCases(casesEmptyBody1, EmptyTree, defn.ThrowableType, WildcardType) val expr1 = typed(addCanThrowCapabilities(tree.expr, casesEmptyBody2), pt.dropIfProto) val casesCtx = ctx.addNotNullInfo(expr1.notNullInfo.retractedInfo) val cases1 = typedCases(tree.cases, EmptyTree, defn.ThrowableType, pt.dropIfProto)(using casesCtx) diff --git a/tests/explicit-nulls/neg/i21380b.scala b/tests/explicit-nulls/neg/i21380b.scala index 83e23053547c..bd28bc63bd26 100644 --- a/tests/explicit-nulls/neg/i21380b.scala +++ b/tests/explicit-nulls/neg/i21380b.scala @@ -3,7 +3,7 @@ def test1 = x = "" 1 match case 1 => x = null - case _ => x = x.trim() // ok + case _ => x = x.trim() // error // LTS specific x.replace("", "") // error def test2(i: Int) = @@ -11,11 +11,11 @@ def test2(i: Int) = i match case 1 => x = "1" case _ => x = " " - x.replace("", "") // ok + x.replace("", "") // error // LTS specific def test3(i: Int) = var x: String | Null = null i match case 1 if x != null => () case _ => x = " " - x.trim() // ok \ No newline at end of file + x.trim() // error // LTS specific \ No newline at end of file diff --git a/tests/explicit-nulls/neg/i21380c.scala b/tests/explicit-nulls/neg/i21380c.scala index f86a5638e4c8..c5758743d784 100644 --- a/tests/explicit-nulls/neg/i21380c.scala +++ b/tests/explicit-nulls/neg/i21380c.scala @@ -3,7 +3,7 @@ def test1(i: Int): Int = if i == 0 then x = "" else x = "" try - x = x.replace(" ", "") // ok + x = x.replace(" ", "") // error // LTS specific throw new Exception() catch case e: Exception => @@ -15,7 +15,7 @@ def test2: Int = var x: String | Null = null try throw new Exception() finally x = "" - x.length // ok + x.length // error // LTS specific def test3 = var x: String | Null = ""