-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Fix #21619: Refactor NotNullInfo to record every reference which is retracted once. #21624
Changes from 7 commits
585dda9
bcc9e68
05c630a
d44147b
f859afe
158af7d
00430c0
200c038
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1201,7 +1201,6 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer | |
untpd.unsplice(tree.expr).putAttachment(AscribedToUnit, ()) | ||
typed(tree.expr, underlyingTreeTpe.tpe.widenSkolem) | ||
assignType(cpy.Typed(tree)(expr1, tpt), underlyingTreeTpe) | ||
.withNotNullInfo(expr1.notNullInfo) | ||
} | ||
|
||
if (untpd.isWildcardStarArg(tree)) { | ||
|
@@ -1551,11 +1550,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer | |
|
||
def thenPathInfo = cond1.notNullInfoIf(true).seq(result.thenp.notNullInfo) | ||
def elsePathInfo = cond1.notNullInfoIf(false).seq(result.elsep.notNullInfo) | ||
result.withNotNullInfo( | ||
if result.thenp.tpe.isRef(defn.NothingClass) then elsePathInfo | ||
else if result.elsep.tpe.isRef(defn.NothingClass) then thenPathInfo | ||
else thenPathInfo.alt(elsePathInfo) | ||
) | ||
result.withNotNullInfo(thenPathInfo.alt(elsePathInfo)) | ||
end typedIf | ||
|
||
/** Decompose function prototype into a list of parameter prototypes and a result | ||
|
@@ -2139,20 +2134,23 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer | |
case1 | ||
} | ||
.asInstanceOf[List[CaseDef]] | ||
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) | ||
assignType(cpy.Match(tree)(sel, cases1), sel, cases1).cast(pt) | ||
.withNotNullInfo(notNullInfoFromCases(sel.notNullInfo, cases1)) | ||
} | ||
|
||
// 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]] | ||
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) | ||
assignType(cpy.Match(tree)(sel, cases1), sel, cases1) | ||
.withNotNullInfo(notNullInfoFromCases(sel.notNullInfo, cases1)) | ||
} | ||
|
||
private def notNullInfoFromCases(initInfo: NotNullInfo, cases: List[CaseDef])(using Context): NotNullInfo = | ||
if cases.nonEmpty then | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In what instances would the cases be empty? Depending on the answer, perhaps a comment would be useful. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think the cases can be empty. Just want to avoid the exception when calling There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, it seems the empty cases can happen after inlining. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK. A question is, if empty cases does happen, what does it mean in terms of runtime semantics? If it definitely fails to match, it would be more precise to return I think it's fine to leave it as it is, and you can merge this PR now. |
||
initInfo.seq(cases.map(_.notNullInfo).reduce(_.alt(_))) | ||
else initInfo | ||
|
||
def typedCases(cases: List[untpd.CaseDef], sel: Tree, wideSelType0: Type, pt: Type)(using Context): List[CaseDef] = | ||
var caseCtx = ctx | ||
var wideSelType = wideSelType0 | ||
|
@@ -2241,7 +2239,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer | |
def typedLabeled(tree: untpd.Labeled)(using Context): Labeled = { | ||
val bind1 = typedBind(tree.bind, WildcardType).asInstanceOf[Bind] | ||
val expr1 = typed(tree.expr, bind1.symbol.info) | ||
assignType(cpy.Labeled(tree)(bind1, expr1)) | ||
assignType(cpy.Labeled(tree)(bind1, expr1)).withNotNullInfo(expr1.notNullInfo.retractedInfo) | ||
} | ||
|
||
/** Type a case of a type match */ | ||
|
@@ -2291,7 +2289,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer | |
// Hence no adaptation is possible, and we assume WildcardType as prototype. | ||
(from, proto) | ||
val expr1 = typedExpr(tree.expr orElse untpd.syntheticUnitLiteral.withSpan(tree.span), proto) | ||
assignType(cpy.Return(tree)(expr1, from)) | ||
assignType(cpy.Return(tree)(expr1, from)).withNotNullInfo(expr1.notNullInfo.terminatedInfo) | ||
end typedReturn | ||
|
||
def typedWhileDo(tree: untpd.WhileDo)(using Context): Tree = | ||
|
@@ -2332,7 +2330,8 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer | |
val capabilityProof = caughtExceptions.reduce(OrType(_, _, true)) | ||
untpd.Block(makeCanThrow(capabilityProof), expr) | ||
|
||
def typedTry(tree: untpd.Try, pt: Type)(using Context): Try = { | ||
def typedTry(tree: untpd.Try, pt: Type)(using Context): Try = | ||
var nnInfo = NotNullInfo.empty | ||
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. | ||
|
@@ -2344,18 +2343,26 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer | |
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) | ||
|
||
// Since we don't know at which point the the exception is thrown in the body, | ||
// we have to collect any reference that is once retracted. | ||
nnInfo = expr1.notNullInfo.retractedInfo | ||
|
||
val casesCtx = ctx.addNotNullInfo(nnInfo) | ||
val cases1 = typedCases(tree.cases, EmptyTree, defn.ThrowableType, pt.dropIfProto)(using casesCtx) | ||
expr1 :: cases1 | ||
}: @unchecked | ||
val cases2 = cases2x.asInstanceOf[List[CaseDef]] | ||
|
||
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) | ||
} | ||
// It is possible to have non-exhaustive cases, and some exceptions are thrown and not caught. | ||
// Therefore, the code in the finalizer and after the try block can only rely on the retracted | ||
// info from the cases' body. | ||
if cases2.nonEmpty then | ||
nnInfo = nnInfo.seq(cases2.map(_.notNullInfo.retractedInfo).reduce(_.alt(_))) | ||
|
||
val finalizer1 = typed(tree.finalizer, defn.UnitType)(using ctx.addNotNullInfo(nnInfo)) | ||
nnInfo = nnInfo.seq(finalizer1.notNullInfo) | ||
assignType(cpy.Try(tree)(expr2, cases2, finalizer1), expr2, cases2).withNotNullInfo(nnInfo) | ||
|
||
def typedTry(tree: untpd.ParsedTry, pt: Type)(using Context): Try = | ||
val cases: List[untpd.CaseDef] = tree.handler match | ||
|
@@ -2369,15 +2376,15 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer | |
def typedThrow(tree: untpd.Throw)(using Context): Tree = | ||
val expr1 = typed(tree.expr, defn.ThrowableType) | ||
val cap = checkCanThrow(expr1.tpe.widen, tree.span) | ||
val res = Throw(expr1).withSpan(tree.span) | ||
var res = Throw(expr1).withSpan(tree.span) | ||
if Feature.ccEnabled && !cap.isEmpty && !ctx.isAfterTyper then | ||
// Record access to the CanThrow capabulity recovered in `cap` by wrapping | ||
// the type of the `throw` (i.e. Nothing) in a `@requiresCapability` annotation. | ||
Typed(res, | ||
res = Typed(res, | ||
TypeTree( | ||
AnnotatedType(res.tpe, | ||
Annotation(defn.RequiresCapabilityAnnot, cap, tree.span)))) | ||
else res | ||
res.withNotNullInfo(expr1.notNullInfo.terminatedInfo) | ||
|
||
def typedSeqLiteral(tree: untpd.SeqLiteral, pt: Type)(using Context): SeqLiteral = { | ||
val elemProto = pt.stripNull().elemType match { | ||
|
@@ -2842,6 +2849,8 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer | |
val vdef1 = assignType(cpy.ValDef(vdef)(name, tpt1, rhs1), sym) | ||
postProcessInfo(vdef1, sym) | ||
vdef1.setDefTree | ||
val nnInfo = rhs1.notNullInfo | ||
vdef1.withNotNullInfo(if sym.is(Lazy) then nnInfo.retractedInfo else nnInfo) | ||
} | ||
|
||
private def retractDefDef(sym: Symbol)(using Context): Tree = | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we lose knowledge about the loop being unreachable. The result always has
asserted == Set()
, even if it wasnull
before.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is fine for the NotNullInfo in the
ctx
. Suppose there is a terminated info in thectx
, adding a new non-terminated info will not change its behaviour: still treating all symbols as non-nullable.