Add failing test for proper termination

[timholy]

This is related to recent work on handling control-flow properly. In brief, this PR contains an example for which our current approach fails and which seems to require a breaking change to fix. @aviatesk, since JET is the only other (besides Revise) direct dependent of LoweredCodeUtils, I thought we should coordinate about this before I take any steps to implement this.

Longer version:

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.

[aviatesk]

since JET is the only other (besides Revise) direct dependent of LoweredCodeUtils, I thought we should coordinate about this before I take any steps to implement this.

Thank you for the reminder. JET uses LoweredCodeUtils, but it has its own implementation for CFG tracking (https://github.com/aviatesk/JET.jl/blob/25d95fb6687e30aa377b1eef6d5c9cf85e6b9768/src/toplevel/virtualprocess.jl#L1174-L1240). I haven't considered which is superior in general cases, but when updating to [email protected], simply using LCU's add_control_flow! did not pass JET's test cases, so I implemented it myself.
So even if there are breaking changes in control flow tracking on the LCU side, it shouldn't cause significant issues for JET. Making isrequired a three-valued variable might be a good idea, but if the issue can be addressed by simply improving the algorithm of LCU.add_control_flow!, that would be preferable.

[timholy]

Thanks for the pointer! I will test JET's version and see if it's usable in LCU.

[timholy]

JET's version (unless I've copied it incorrectly) marks too much. For example, in this test:

LoweredCodeUtils.jl/test/codeedges.jl

Lines 90 to 105 in dc7cc3a

	ex = quote
	flag2 = true
	z2 = 15
	if flag2
	x2 = 5
	a2 = 1
	else
	y2 = 7
	a2 = 2
	end
	a2
	end
	frame = Frame(ModSelective, ex)
	src = frame.framecode.src
	edges = CodeEdges(ModSelective, src)
	isrequired = lines_required(GlobalRef(ModSelective, :a2), src, edges)

the goal is to mark the lines needed for a2 but not x2 or y2, yet with the JET version I get

julia> LoweredCodeUtils.print_with_code(stdout, src, isrequired)
 1 t 1 ── %1  = Core.get_binding_type(Main.ModSelective, :flag2)
 2 t │          #s452 = true
 3 t │    %3  = #s452 isa %1
 4 t └───       goto #3 if not %3
 5 t 2 ──       goto #4
 6 t 3 ──       #s452 = Base.convert(%1, #s452)
 7 t 4 ┄─       flag2 = #s452
 8 t │    %8  = Core.get_binding_type(Main.ModSelective, :z2)
 9 t │          @_2 = 15
10 t │    %10 = @_2 isa %8
11 t └───       goto #6 if not %10
12 t 5 ──       goto #7
13 t 6 ──       @_2 = Base.convert(%8, @_2)
14 f 7 ┄─       z2 = @_2
15 t └───       goto #15 if not flag2
16 t 8 ── %16 = Core.get_binding_type(Main.ModSelective, :x2)
17 t │          @_3 = 5
18 t │    %18 = @_3 isa %16
19 t └───       goto #10 if not %18
20 t 9 ──       goto #11
21 t 10 ─       @_3 = Base.convert(%16, @_3)
22 f 11 ┄       x2 = @_3
23 t │    %23 = Core.get_binding_type(Main.ModSelective, :a2)
24 t │          @_4 = 1
25 t │    %25 = @_4 isa %23
26 t └───       goto #13 if not %25
27 t 12 ─       goto #14
28 t 13 ─       @_4 = Base.convert(%23, @_4)
29 t 14 ┄       a2 = @_4
30 f └───       goto #22
31 t 15 ─ %31 = Core.get_binding_type(Main.ModSelective, :y2)
32 t │          @_5 = 7
33 t │    %33 = @_5 isa %31
34 t └───       goto #17 if not %33
35 t 16 ─       goto #18
36 t 17 ─       @_5 = Base.convert(%31, @_5)
37 f 18 ┄       y2 = @_5
38 t │    %38 = Core.get_binding_type(Main.ModSelective, :a2)
39 t │          @_6 = 2
40 t │    %40 = @_6 isa %38
41 t └───       goto #20 if not %40
42 t 19 ─       goto #21
43 t 20 ─       @_6 = Base.convert(%38, @_6)
44 t 21 ┄       a2 = @_6
45 f 22 ┄       return a2

With the exception of the line evaluating z2 (which is the easiest one to avoid), basically everything is marked. I'll try the converse and see what's missing in JET, and at the same time try to fix this issue. (Probably fix this issue first, then take a look at JET.)

[timholy]

Dang, I thought I had it, but it turns out that it's not robust to differences in lowering across Julia versions. This is a difficult problem, and I'm even beginning to wonder if it's a well-defined problem.

[aviatesk]

With the exception of the line evaluating z2 (which is the easiest one to avoid), basically everything is marked. I'll try the converse and see what's missing in JET, and at the same time try to fix this issue.

This issue was resolved by JET's new algorithm. The algorithm was ported in #116.