Fix tail resumptive analysis for vars #1154
Conversation
| case Stmt.Match(scrutinee, clauses, default) => !freeInExpr(scrutinee) && clauses.forall { | ||
| case (_, BlockLit(tparams, cparams, vparams, bparams, body)) => tailResumptive(k, body) | ||
| } && default.forall { stmt => tailResumptive(k, stmt) } | ||
| case Stmt.Region(BlockLit(tparams, cparams, vparams, bparams, body)) => tailResumptive(k, body) |
There was a problem hiding this comment.
Shouldn't this also be false then?
There was a problem hiding this comment.
If/when we change this, could we add an example where this fires / doesn't fire?
|
Nice that you apparently didn't have to change the VMTests! |
b-studios
left a comment
b-studios
left a comment
There was a problem hiding this comment.
I pre-approve :) Once you fixed / commented out the Scheme tests this is ready to go.
| case Stmt.Var(ref, init, capture, body) => tailResumptive(k, body) && !freeInExpr(init) | ||
| // Conceptually, a mutable variable definition can be seen as a handler for get and put operations. | ||
| // Treating this as tail-resumptive leads to a failure of semantics preservation | ||
| case Stmt.Var(ref, init, capture, body) => false |
There was a problem hiding this comment.
Not too relevant for the current PR, but would a use-mention distinction help here?
If we have variables/prompts that are merely mentioned, can we still consider the function to be tail-res?
As a very trivial example, is it safe to do something similar to the example, but not use x at all after declaring it? 🤔
There was a problem hiding this comment.
Is there an example that isn't just dead code (and would be handled by dead code elimination)?
Fixes #1153.