From 10497c9a9bafdb36a84250584b0eaa2d7b1b6b07 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) --- .../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 1125e09539b6..71bf3c3bc9a4 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2135,14 +2135,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, wideSelType0: Type, pt: Type)(using Context): List[CaseDef] = @@ -2216,7 +2220,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) @@ -2327,7 +2336,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 7674fefae3e9c9c9dffe880c9b2b799049e90a10 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 --- .../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 71bf3c3bc9a4..e4932045e91a 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2136,7 +2136,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) } @@ -2145,7 +2145,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) } @@ -2218,13 +2218,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) } @@ -2329,16 +2327,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 7f92b53fa5670c64c8419e680a0b0d85d493e959 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 --- 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 e4932045e91a..85dc13f7853a 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2210,7 +2210,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, @@ -2221,7 +2223,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) } @@ -2343,7 +2345,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 fe0bdad123ea0daef818be993f9b7127c9242482 Mon Sep 17 00:00:00 2001 From: noti0na1 Date: Mon, 9 Sep 2024 14:59:25 +0200 Subject: [PATCH 4/4] Put i13864 in the blacklist of best-effort test. --- compiler/src/dotty/tools/dotc/typer/Typer.scala | 16 +++++++++------- .../test/dotc/neg-best-effort-pickling.blacklist | 1 + 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 85dc13f7853a..ce5743f69d0c 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2329,14 +2329,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/compiler/test/dotc/neg-best-effort-pickling.blacklist b/compiler/test/dotc/neg-best-effort-pickling.blacklist index a582f085dd30..99a83a467f08 100644 --- a/compiler/test/dotc/neg-best-effort-pickling.blacklist +++ b/compiler/test/dotc/neg-best-effort-pickling.blacklist @@ -16,6 +16,7 @@ i13780-1.scala i20317a.scala i11226.scala i974.scala +i13864.scala # semantic db generation fails in the first compilation i1642.scala