From 9341bfaa9e019665564681cc5aa384dd94253a87 Mon Sep 17 00:00:00 2001
From: Tim Holy <tim.holy@gmail.com>
Date: Tue, 16 Jan 2024 20:21:00 -0600
Subject: [PATCH] Add failing test for proper termination

Currently we use a heuristic to avoid marking the exit of the last
basic block, expecting it to be a `return` and so that execution
will cease whether we evaluate it or not. However, it is possible
to write code for which the final statement is a `GotoNode` and
in this case we can get incorrect answers if we fail to evaluate it.

This example illustrates a tricky point: `return` both terminates
execution but may also return a value. If we don't require the
value, we still might need to terminate execution. This example
seems to illustrate that having `isrequired[i]` be either `true` or
`false` may be insufficiently expressive; we might need it to be
three states, `:no`, `:yes`, `:exit`. During marking, encountering
`:exit` would not force one to evaluate the returned SSAValue.
---
 test/codeedges.jl | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/test/codeedges.jl b/test/codeedges.jl
index 9b6da64..924991c 100644
--- a/test/codeedges.jl
+++ b/test/codeedges.jl
@@ -216,6 +216,23 @@ module ModSelective end
     @test ModSelective.k11 == 0
     @test 3 <= ModSelective.s11 <= 15
 
+    # Final block is not a `return`
+    ex = quote
+        x = 1
+        y = 7
+        @label loop
+        x += 1
+        x < 5 || return y
+        @goto loop
+    end
+    frame = Frame(ModSelective, ex)
+    src = frame.framecode.src
+    edges = CodeEdges(src)
+    isrequired = lines_required(:x, src, edges)
+    selective_eval_fromstart!(frame, isrequired, true)
+    @test ModSelective.x == 5
+    @test !isdefined(ModSelective, :y)
+
     # Control-flow in an abstract type definition
     ex = :(abstract type StructParent{T, N} <: AbstractArray{T, N} end)
     frame = Frame(ModSelective, ex)