Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add tracking of NotNullInfo for Match, Case, Try trees (fix #21380) #21389

Merged
merged 4 commits into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 32 additions & 9 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 then 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 then 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] =
Expand Down Expand Up @@ -2206,17 +2210,22 @@ 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,
// see tests/pos/i12226 and issue #12226. It might be possible that this
// 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)
assignType(cpy.CaseDef(tree)(pat1, guard1, body1), pat1, body1)
val nni = pat1.notNullInfo
.seq(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)
Expand Down Expand Up @@ -2321,13 +2330,27 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer

def typedTry(tree: untpd.Try, pt: Type)(using Context): Try = {
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)
// 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)
expr1 :: cases1
}: @unchecked
val finalizer1 = typed(tree.finalizer, defn.UnitType)
val cases2 = cases2x.asInstanceOf[List[CaseDef]]
assignType(cpy.Try(tree)(expr2, cases2, finalizer1), expr2, cases2)

var nni = expr2.notNullInfo.retractedInfo
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)
}

def typedTry(tree: untpd.ParsedTry, pt: Type)(using Context): Try =
Expand Down
1 change: 1 addition & 0 deletions compiler/test/dotc/neg-best-effort-pickling.blacklist
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 19 additions & 0 deletions tests/explicit-nulls/neg/i21380.scala
Original file line number Diff line number Diff line change
@@ -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
}
21 changes: 21 additions & 0 deletions tests/explicit-nulls/neg/i21380b.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
def test1 =
var x: String | Null = null
x = ""
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

def test3(i: Int) =
var x: String | Null = null
i match
case 1 if x != null => ()
case _ => x = " "
x.trim() // ok
45 changes: 45 additions & 0 deletions tests/explicit-nulls/neg/i21380c.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
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 // 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
Loading