Add failing test for proper termination

src/codeedges.jl

@@ -369,8 +369,9 @@ struct CodeEdges
     preds::Vector{Vector{Int}}
     succs::Vector{Vector{Int}}
     byname::Dict{GlobalRef,Variable}
+    slotassigns::Vector{Vector{Int}}
 end
-CodeEdges(n::Integer) = CodeEdges([Int[] for i = 1:n], [Int[] for i = 1:n], Dict{GlobalRef,Variable}())
+CodeEdges(nstmts::Integer, nslots::Integer) = CodeEdges([Int[] for i = 1:nstmts], [Int[] for i = 1:nstmts], Dict{GlobalRef,Variable}(), [Int[] for i = 1:nslots])
 
 function Base.show(io::IO, edges::CodeEdges)
     println(io, "CodeEdges:")
@@ -389,7 +390,7 @@ end
 
 
 """
-    edges = CodeEdges(src::CodeInfo)
+    edges = CodeEdges(mod::Module, src::CodeInfo)
 
 Analyze `src` and determine the chain of dependencies.
 
@@ -410,7 +411,10 @@ function CodeEdges(src::CodeInfo, cl::CodeLinks)
     # Replace/add named intermediates (slot & named-variable references) with statement numbers
     nstmts, nslots = length(src.code), length(src.slotnames)
     marked, slothandled = BitSet(), fill(false, nslots)  # working storage during resolution
-    edges = CodeEdges(nstmts)
+    edges = CodeEdges(nstmts, nslots)
+    for (edge_s, link_s) in zip(edges.slotassigns, cl.slotassigns)
+        append!(edge_s, link_s)
+    end
     emptylink = Links()
     emptylist = Int[]
     for (i, stmt) in enumerate(src.code)
@@ -561,6 +565,8 @@ function terminal_preds(i::Int, edges::CodeEdges)
     return s
 end
 
+initialize_isrequired(n) = fill!(Vector{Union{Bool,Symbol}}(undef, n), false)
+
 """
     isrequired = lines_required(obj::GlobalRef, src::CodeInfo, edges::CodeEdges)
     isrequired = lines_required(idx::Int,                     src::CodeInfo, edges::CodeEdges)
@@ -573,20 +579,20 @@ will end up skipping a subset of such statements, perhaps while repeating others
 See also [`lines_required!`](@ref) and [`selective_eval!`](@ref).
 """
 function lines_required(obj::GlobalRef, src::CodeInfo, edges::CodeEdges; kwargs...)
-    isrequired = falses(length(edges.preds))
+    isrequired = initialize_isrequired(length(src.code))
     objs = Set{GlobalRef}([obj])
     return lines_required!(isrequired, objs, src, edges; kwargs...)
 end
 
 function lines_required(idx::Int, src::CodeInfo, edges::CodeEdges; kwargs...)
-    isrequired = falses(length(edges.preds))
+    isrequired = initialize_isrequired(length(edges.preds))
     isrequired[idx] = true
     objs = Set{GlobalRef}()
     return lines_required!(isrequired, objs, src, edges; kwargs...)
 end
 
 """
-    lines_required!(isrequired::AbstractVector{Bool}, src::CodeInfo, edges::CodeEdges;
+    lines_required!(isrequired::AbstractVector{Union{Bool,Symbol}}, src::CodeInfo, edges::CodeEdges;
                     norequire = ())
 
 Like `lines_required`, but where `isrequired[idx]` has already been set to `true` for all statements
@@ -598,7 +604,7 @@ should _not_ be marked as a requirement.
 For example, use `norequire = LoweredCodeUtils.exclude_named_typedefs(src, edges)` if you're
 extracting method signatures and not evaluating new definitions.
 """
-function lines_required!(isrequired::AbstractVector{Bool}, src::CodeInfo, edges::CodeEdges; kwargs...)
+function lines_required!(isrequired::AbstractVector{Union{Bool,Symbol}}, src::CodeInfo, edges::CodeEdges; kwargs...)
     objs = Set{GlobalRef}()
     return lines_required!(isrequired, objs, src, edges; kwargs...)
 end
@@ -620,7 +626,7 @@ function exclude_named_typedefs(src::CodeInfo, edges::CodeEdges)
     return norequire
 end
 
-function lines_required!(isrequired::AbstractVector{Bool}, objs, src::CodeInfo, edges::CodeEdges; norequire = ())
+function lines_required!(isrequired::AbstractVector{Union{Bool,Symbol}}, objs, src::CodeInfo, edges::CodeEdges; norequire = ())
     # Mark any requested objects (their lines of assignment)
     objs = add_requests!(isrequired, objs, edges, norequire)
 
@@ -638,6 +644,9 @@ function lines_required!(isrequired::AbstractVector{Bool}, objs, src::CodeInfo,
     iter = 0
     while changed
         changed = false
+        # @show iter
+        # print_with_code(stdout, src, isrequired)
+        # println()
 
         # Handle ssa predecessors
         changed |= add_ssa_preds!(isrequired, src, edges, norequire)
@@ -646,8 +655,7 @@ function lines_required!(isrequired::AbstractVector{Bool}, objs, src::CodeInfo,
         changed |= add_named_dependencies!(isrequired, edges, objs, norequire)
 
         # Add control-flow
-        changed |= add_loops!(isrequired, cfg)
-        changed |= add_control_flow!(isrequired, cfg, domtree, postdomtree)
+        changed |= add_control_flow!(isrequired, src, cfg, domtree, postdomtree)
 
         # So far, everything is generic graph traversal. Now we add some domain-specific information
         changed |= add_typedefs!(isrequired, src, edges, typedefs, norequire)
@@ -669,7 +677,7 @@ end
 function add_ssa_preds!(isrequired, src::CodeInfo, edges::CodeEdges, norequire)
     changed = false
     for idx = 1:length(src.code)
-        if isrequired[idx]
+        if isrequired[idx] == true
             changed |= add_preds!(isrequired, idx, edges, norequire)
         end
     end
@@ -680,7 +688,7 @@ function add_named_dependencies!(isrequired, edges::CodeEdges, objs, norequire)
     changed = false
     for (obj, uses) in edges.byname
         obj ∈ objs && continue
-        if any(view(isrequired, uses.succs))
+        if any(==(true), view(isrequired, uses.succs))
             changed |= add_obj!(isrequired, objs, obj, edges, norequire)
         end
     end
@@ -691,7 +699,7 @@ function add_preds!(isrequired, idx, edges::CodeEdges, norequire)
     chngd = false
     preds = edges.preds[idx]
     for p in preds
-        isrequired[p] && continue
+        isrequired[p] == true && continue
         p ∈ norequire && continue
         isrequired[p] = true
         chngd = true
@@ -702,7 +710,7 @@ end
 function add_succs!(isrequired, idx, edges::CodeEdges, succs, norequire)
     chngd = false
     for p in succs
-        isrequired[p] && continue
+        isrequired[p] == true && continue
         p ∈ norequire && continue
         isrequired[p] = true
         chngd = true
@@ -713,8 +721,9 @@ end
 function add_obj!(isrequired, objs, obj, edges::CodeEdges, norequire)
     chngd = false
     for d in edges.byname[obj].assigned
+        isrequired[d] == true && continue
         d ∈ norequire && continue
-        isrequired[d] || add_preds!(isrequired, d, edges, norequire)
+        add_preds!(isrequired, d, edges, norequire)
         isrequired[d] = true
         chngd = true
     end
@@ -724,71 +733,200 @@ end
 
 ## Add control-flow
 
-# Mark loops that contain evaluated statements
-function add_loops!(isrequired, cfg)
-    changed = false
-    for (ibb, bb) in enumerate(cfg.blocks)
-        needed = false
-        for ibbp in bb.preds
-            # Is there a backwards-pointing predecessor, and if so are there any required statements between the two?
-            ibbp > ibb || continue   # not a loop-block predecessor
-            r, rp = rng(bb), rng(cfg.blocks[ibbp])
-            r = first(r):first(rp)-1
-            needed |= any(view(isrequired, r))
-        end
-        if needed
-            # Mark the final statement of all predecessors
-            for ibbp in bb.preds
-                rp = rng(cfg.blocks[ibbp])
-                changed |= !isrequired[last(rp)]
-                isrequired[last(rp)] = true
-            end
+iscf(stmt) = isa(stmt, Core.GotoNode) || isa(stmt, Core.GotoIfNot) || isa(stmt, Core.ReturnNode)
+function jumps_back(src, i)
+    stmt = src.code[i]
+    (isa(stmt, Core.GotoNode) && stmt.label < i ||
+        isa(stmt, Core.GotoIfNot) && stmt.dest < i) && return true
+    if isa(stmt, Core.GotoIfNot) && i < length(src.code)
+        stmt = src.code[i+1]
+        isa(stmt, Core.GotoNode) && return stmt.label < i
+    end
+    return false
+end
+
+function markcf!(isrequired, src, i)
+    stmt = src.code[i]
+    @assert iscf(stmt)
+    isrequired[i] ∈ (true, :exit) && return false
+    isrequired[i] = isa(stmt, Core.ReturnNode) ? :exit : true
+    return true
+end
+
+"""
+    ispredecessor(blocks, i, j)
+
+Determine whether block `i` is a predecessor of block `j` in the control-flow graph `blocks`.
+"""
+function ispredecessor(blocks, i, j, cache=Set{Int}())
+    for p in blocks[j].preds  # avoid putting `j` in the cache unless it loops back
+        getpreds!(cache, blocks, p)
+    end
+    return i ∈ cache
+end
+function getpreds!(cache, blocks, j)
+    if j ∈ cache
+        return cache
+    end
+    push!(cache, j)
+    for p in blocks[j].preds
+        getpreds!(cache, blocks, p)
+    end
+    return cache
+end
+
+function block_internals_needed(isrequired, src, r)
+    needed = false
+    for i in r
+        if isrequired[i] == true
+            iscf(src.code[i]) && continue
+            needed = true
+            break
         end
     end
-    return changed
+    return needed
 end
 
-function add_control_flow!(isrequired, cfg, domtree, postdomtree)
+function add_control_flow!(isrequired, src, cfg, domtree, postdomtree)
     changed, _changed = false, true
     blocks = cfg.blocks
-    nblocks = length(blocks)
+    needed = falses(length(blocks))
+    cache = Set{Int}()
     while _changed
         _changed = false
         for (ibb, bb) in enumerate(blocks)
             r = rng(bb)
-            if any(view(isrequired, r))
-                # Walk up the dominators
+            if block_internals_needed(isrequired, src, r)
+                needed[ibb] = true
+                # Check this block's precessors and mark any that are just control-flow
+                for p in bb.preds
+                    r = rng(blocks[p])
+                    if length(r) == 1 && iscf(src.code[r[1]])
+                        _changed |= markcf!(isrequired, src, r[1])
+                    end
+                end
+                # Check control flow that's needed to reach this block by walking up the dominators
                 jbb = ibb
                 while jbb != 1
-                    jdbb = domtree.idoms_bb[jbb]
+                    jdbb = domtree.idoms_bb[jbb]   # immediate dominator of jbb
                     dbb = blocks[jdbb]
-                    # Check the successors; if jbb doesn't post-dominate, mark the last statement
-                    for s in dbb.succs
-                        if !postdominates(postdomtree, jbb, s)
-                            idxlast = rng(dbb)[end]
-                            _changed |= !isrequired[idxlast]
-                            isrequired[idxlast] = true
-                            break
+                    idxlast = rng(dbb)[end]
+                    if iscf(src.code[idxlast])
+                        # Check the idom's successors; if jbb doesn't post-dominate, mark the last statement
+                        for s in dbb.succs
+                            if !postdominates(postdomtree, jbb, s)
+                                if markcf!(isrequired, src, idxlast)
+                                    _changed = true
+                                    break
+                                end
+                            end
                         end
                     end
                     jbb = jdbb
                 end
-                # Walk down the post-dominators, including self
+                # Walk down the post-dominators, starting with self
                 jbb = ibb
-                while jbb != 0 && jbb < nblocks
-                    pdbb = blocks[jbb]
-                    # Check if the exit of this block is a GotoNode or `return`
-                    if length(pdbb.succs) < 2
-                        idxlast = rng(pdbb)[end]
-                        _changed |= !isrequired[idxlast]
-                        isrequired[idxlast] = true
+                while jbb != 0
+                    if ispredecessor(blocks, jbb, ibb, empty!(cache))  # is post-dominator jbb also a predecessor of ibb? If so we have a loop.
+                        pdbb = blocks[jbb]
+                        r = rng(pdbb)
+                        # if block_internals_needed(isrequired, src, r)
+                            idxlast = rng(pdbb)[end]
+                            stmt = src.code[idxlast]
+                            if iscf(stmt) && jumps_back(src, idxlast)
+                                if isrequired[idxlast] != true
+                                    _changed = true
+                                    if isa(stmt, Core.ReturnNode) && isrequired[idxlast] != :exit
+                                        isrequired[idxlast] = :exit
+                                    else
+                                        isrequired[idxlast] = true
+                                        if isa(stmt, Core.GotoIfNot) && idxlast < length(isrequired) && isrequired[idxlast+1] != true && iscf(src.code[idxlast+1])
+                                            isrequired[idxlast+1] = true
+                                        end
+                                    end
+                                    break
+                                end
+                            end
+                        # end
                     end
                     jbb = postdomtree.idoms_bb[jbb]
                 end
             end
         end
         changed |= _changed
     end
+    # Now handle "exclusions": in code that would inappropriately fall through
+    # during selective evaluation, find a post-dominator between the two that is
+    # marked, or mark one if absent.
+    marked = push!(findall(needed), length(blocks)+1)
+    for k in Iterators.drop(eachindex(marked), 1)
+        ibb, jbb = marked[k-1], marked[k]
+        if jbb <= length(blocks)
+            # are ibb and jbb exclusive?
+            ispredecessor(blocks, ibb, jbb, empty!(cache)) && continue
+        end
+        # Is there a required control-flow statement between ibb and jbb?
+        ok = false
+        ipbb = ibb
+        while ipbb < jbb
+            ipbb = postdomtree.idoms_bb[ipbb]
+            ipbb == 0 && break
+            idxlast = rng(blocks[ipbb])[end]
+            if iscf(src.code[idxlast]) && isrequired[idxlast] != false
+                ok = true
+                break
+            end
+        end
+        if !ok
+            # Mark a control-flow statement between ibb and jbb
+            ipbb = ibb
+            while ipbb < jbb
+                ipbb = postdomtree.idoms_bb[ipbb]
+                ipbb == 0 && break
+                # Mark the ipostdom's predecessors...
+                for k in blocks[ipbb].preds
+                    idxlast = rng(blocks[k])[end]
+                    stmt = src.code[idxlast]
+                    if iscf(stmt)
+                        if markcf!(isrequired, src, idxlast)
+                            changed = true
+                            ok = true
+                            if isa(stmt, Core.GotoIfNot) && idxlast < length(isrequired) && isrequired[idxlast+1] != true && iscf(src.code[idxlast+1])
+                                isrequired[idxlast+1] = true
+                            end
+                        end
+                    end
+                end
+                # ...or the ipostdom itself
+                if !ok
+                    idxlast = rng(blocks[ipbb])[end]
+                    stmt = src.code[idxlast]
+                    if isa(stmt, Core.GotoNode) || isa(stmt, Core.ReturnNode)  # unconditional jump
+                        if markcf!(isrequired, src, idxlast)
+                            changed = true
+                            ok = true
+                        end
+                    end
+                    # r = rng(blocks[ipbb])
+                    # if length(r) == 1 && iscf(src.code[r[1]])
+                    #     if markcf!(isrequired, src, r[1])
+                    #         changed = true
+                    #         ok = true
+                    #     end
+                    # end
+                end
+                # idxlast = rng(blocks[ipbb])[end]
+                # if iscf(src.code[idxlast])    # ideally we might find an unconditional jump to prevent unnecessary evaluation of the conditional
+                #     if markcf!(isrequired, src, idxlast)
+                #         changed = true
+                #         ok = true
+                #         break
+                #     end
+                # end
+            end
+        end
+        jbb <= length(blocks) && @assert ok
+    end
     return changed
 end
 
@@ -825,11 +963,11 @@ function add_typedefs!(isrequired, src::CodeInfo, edges::CodeEdges, (typedef_blo
     idx = 1
     while idx < length(stmts)
         stmt = stmts[idx]
-        isrequired[idx] || (idx += 1; continue)
+        isrequired[idx] == true || (idx += 1; continue)
         for (typedefr, typedefn) in zip(typedef_blocks, typedef_names)
-            if idx ∈ typedefr
+            if idx ∈ typedefr && !iscf(stmt)  # exclude control-flow nodes since they may be needed for other purposes
                 ireq = view(isrequired, typedefr)
-                if !all(ireq)
+                if !all(==(true), ireq)
                     changed = true
                     ireq .= true
                     # Also mark any by-type constructor(s) associated with this typedef
@@ -857,8 +995,10 @@ function add_typedefs!(isrequired, src::CodeInfo, edges::CodeEdges, (typedef_blo
             if i <= length(stmts) && (stmts[i]::Expr).args[1] == false
                 tpreds = terminal_preds(i, edges)
                 if minimum(tpreds) == idx && i ∉ norequire
-                    changed |= !isrequired[i]
-                    isrequired[i] = true
+                    if isrequired[i] != true
+                        changed = true
+                        isrequired[i] = true
+                    end
                 end
             end
         end
@@ -877,15 +1017,17 @@ function add_inplace!(isrequired, src, edges, norequire)
             callee_matches(fname, Base, :pop!) ||
             callee_matches(fname, Base, :empty!) ||
             callee_matches(fname, Base, :setindex!))
-            _changed = !isrequired[j]
-            isrequired[j] = true
+            if isrequired[j] != true
+                _changed = true
+               isrequired[j] = true
+            end
         end
         return _changed
     end
 
     changed = false
     for (i, isreq) in pairs(isrequired)
-        isreq || continue
+        isreq == true || continue
         for j in edges.succs[i]
             j ∈ norequire && continue
             stmt = src.code[j]
@@ -930,14 +1072,16 @@ This will return either a `BreakpointRef`, the value obtained from the last exec
 (if stored to `frame.framedata.ssavlues`), or `nothing`.
 Typically, assignment to a variable binding does not result in an ssa store by JuliaInterpreter.
 """
-function selective_eval!(@nospecialize(recurse), frame::Frame, isrequired::AbstractVector{Bool}, istoplevel::Bool=false)
+function selective_eval!(@nospecialize(recurse), frame::Frame, isrequired, istoplevel::Bool=false)
     pc = pcexec = pclast = frame.pc
     while isa(pc, Int)
         frame.pc = pc
         te = isrequired[pc]
         pclast = pcexec::Int
-        if te
+        if te == true
             pcexec = pc = step_expr!(recurse, frame, istoplevel)
+        elseif te == :exit
+            pc = nothing
         else
             pc = next_or_nothing!(frame)
         end
@@ -946,11 +1090,11 @@ function selective_eval!(@nospecialize(recurse), frame::Frame, isrequired::Abstr
     pcexec = (pcexec === nothing ? pclast : pcexec)::Int
     frame.pc = pcexec
     node = pc_expr(frame)
-    is_return(node) && return isrequired[pcexec] ? lookup_return(frame, node) : nothing
+    is_return(node) && return isrequired[pcexec] == true ? lookup_return(frame, node) : nothing
     isassigned(frame.framedata.ssavalues, pcexec) && return frame.framedata.ssavalues[pcexec]
     return nothing
 end
-function selective_eval!(frame::Frame, isrequired::AbstractVector{Bool}, istoplevel::Bool=false)
+function selective_eval!(frame::Frame, isrequired, istoplevel::Bool=false)
     selective_eval!(finish_and_return!, frame, isrequired, istoplevel)
 end
 
@@ -959,29 +1103,32 @@ end
 
 Like [`selective_eval!`](@ref), except it sets `frame.pc` to the first `true` statement in `isrequired`.
 """
-function selective_eval_fromstart!(@nospecialize(recurse), frame, isrequired, istoplevel::Bool=false)
+function selective_eval_fromstart!(@nospecialize(recurse), frame::Frame, isrequired, istoplevel::Bool=false)
     pc = findfirst(isrequired)
     pc === nothing && return nothing
     frame.pc = pc
     return selective_eval!(recurse, frame, isrequired, istoplevel)
 end
-function selective_eval_fromstart!(frame::Frame, isrequired::AbstractVector{Bool}, istoplevel::Bool=false)
+function selective_eval_fromstart!(frame::Frame, isrequired, istoplevel::Bool=false)
     selective_eval_fromstart!(finish_and_return!, frame, isrequired, istoplevel)
 end
 
 """
-    print_with_code(io, src::CodeInfo, isrequired::AbstractVector{Bool})
+    print_with_code(io, src::CodeInfo, isrequired::AbstractVector{Union{Bool,Symbol}})
 
 Mark each line of code with its requirement status.
 
 !!! compat "Julia 1.6"
     This function produces dummy output if suitable support is missing in your version of Julia.
 
 """
-function print_with_code(io::IO, src::CodeInfo, isrequired::AbstractVector{Bool})
+function print_with_code(io::IO, src::CodeInfo, isrequired::AbstractVector{Union{Bool,Symbol}})
+    function markchar(c)
+        return c === true ? 't' : (c === false ? 'f' : (c === :exit ? 'e' : 'x'))
+    end
     nd = ndigits(length(isrequired))
     preprint(::IO) = nothing
-    preprint(io::IO, idx::Int) = (c = isrequired[idx]; printstyled(io, lpad(idx, nd), ' ', c ? "t " : "f "; color = c ? :cyan : :plain))
+    preprint(io::IO, idx::Int) = (c = markchar(isrequired[idx]); printstyled(io, lpad(idx, nd), ' ', c; color = c ∈ ('t', 'e') ? :cyan : :plain))
     postprint(::IO) = nothing
     postprint(io::IO, idx::Int, bbchanged::Bool) = nothing
 

src/packagedef.jl

@@ -21,6 +21,7 @@ if Base.VERSION < v"1.10"
 else
     const construct_domtree = Core.Compiler.construct_domtree
     const construct_postdomtree = Core.Compiler.construct_postdomtree
+    const dominates = Core.Compiler.dominates
     const postdominates = Core.Compiler.postdominates
 end
 
@@ -46,10 +47,8 @@ if ccall(:jl_generating_output, Cint, ()) == 1
     isrequired = lines_required(GlobalRef(@__MODULE__, :s), src, edges)
     lines_required(GlobalRef(@__MODULE__, :s), src, edges; norequire=())
     lines_required(GlobalRef(@__MODULE__, :s), src, edges; norequire=exclude_named_typedefs(src, edges))
-    for isreq in (isrequired, convert(Vector{Bool}, isrequired))
-        lines_required!(isreq, src, edges; norequire=())
-        lines_required!(isreq, src, edges; norequire=exclude_named_typedefs(src, edges))
-    end
+    lines_required!(isrequired, src, edges; norequire=())
+    lines_required!(isrequired, src, edges; norequire=exclude_named_typedefs(src, edges))
     frame = Frame(@__MODULE__, src)
     # selective_eval_fromstart!(frame, isrequired, true)
     precompile(selective_eval_fromstart!, (typeof(frame), typeof(isrequired), Bool))  # can't @eval during precompilation

test/codeedges.jl

@@ -25,7 +25,7 @@ function hastrackedexpr(stmt; heads=LoweredCodeUtils.trackedheads)
 end
 
 function minimal_evaluation(predicate, src::Core.CodeInfo, edges::CodeEdges; kwargs...)
-    isrequired = fill(false, length(src.code))
+    isrequired = LoweredCodeUtils.initialize_isrequired(length(src.code))
     for (i, stmt) in enumerate(src.code)
         if !isrequired[i]
             isrequired[i], haseval = predicate(stmt)
@@ -65,7 +65,7 @@ module ModSelective end
     # Check that the result of direct evaluation agrees with selective evaluation
     Core.eval(ModEval, ex)
     isrequired = lines_required(GlobalRef(ModSelective, :x), src, edges)
-    # theere is too much diversity in lowering across Julia versions to make it useful to test `sum(isrequired)`
+    # there is too much diversity in lowering across Julia versions to make it useful to test `sum(isrequired)`
     selective_eval_fromstart!(frame, isrequired)
     @test ModSelective.x === ModEval.x
     @test allmissing(ModSelective, (:y, :z, :a, :b, :k))
@@ -160,7 +160,7 @@ module ModSelective end
     src = frame.framecode.src
     edges = CodeEdges(ModSelective, src)
     isrequired = lines_required(GlobalRef(ModSelective, :c_os), src, edges)
-    @test sum(isrequired) >= length(isrequired) - 3
+    @test sum(∈((true, :exit)), isrequired) >= length(isrequired) - 3
     selective_eval_fromstart!(frame, isrequired)
     Core.eval(ModEval, ex)
     @test ModSelective.c_os === ModEval.c_os == Sys.iswindows()
@@ -183,7 +183,7 @@ module ModSelective end
     # Mark just the load of Core.eval
     haseval(stmt) = (isa(stmt, Expr) && JuliaInterpreter.hasarg(isequal(:eval), stmt.args)) ||
                     (isa(stmt, Expr) && stmt.head === :call && is_quotenode(stmt.args[1], Core.eval))
-    isrequired = map(haseval, src.code)
+    isrequired = Union{Bool,Symbol}[haseval(stmt) for stmt in src.code]
     @test sum(isrequired) == 1
     isrequired[edges.succs[findfirst(isrequired)]] .= true   # add lines that use Core.eval
     lines_required!(isrequired, src, edges)
@@ -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
+        yy = 7
+        @label loop
+        x += 1
+        x < 5 || return yy
+        @goto loop
+    end
+    frame = Frame(ModSelective, ex)
+    src = frame.framecode.src
+    edges = CodeEdges(ModSelective, src)
+    isrequired = lines_required(GlobalRef(ModSelective, :x), src, edges)
+    selective_eval_fromstart!(frame, isrequired, true)
+    @test ModSelective.x == 5
+    @test !isdefined(ModSelective, :yy)
+
     # Control-flow in an abstract type definition
     ex = :(abstract type StructParent{T, N} <: AbstractArray{T, N} end)
     frame = Frame(ModSelective, ex)
@@ -280,7 +297,7 @@ module ModSelective end
     frame = Frame(ModSelective, ex)
     src = frame.framecode.src
     edges = CodeEdges(ModSelective, src)
-    isrequired = fill(false, length(src.code))
+    isrequired = LoweredCodeUtils.initialize_isrequired(length(src.code))
     j = length(src.code) - 1
     if !Meta.isexpr(src.code[end-1], :method, 3)
         j -= 1
@@ -307,9 +324,9 @@ module ModSelective end
     for (iblock, block) in enumerate(bbs.blocks)
         r = LoweredCodeUtils.rng(block)
         if iblock == length(bbs.blocks)
-            @test any(idx->isrequired[idx], r)
+            @test any(idx->isrequired[idx]==true, r)
         else
-            @test !any(idx->isrequired[idx], r)
+            @test !any(idx->isrequired[idx]==true && !LoweredCodeUtils.iscf(src.code[idx]), r)
         end
     end
 
@@ -454,7 +471,8 @@ module ModSelective end
         end)
         lwr = Meta.lower(Main, ex)
         src = lwr.args[1]
-        LoweredCodeUtils.print_with_code(io, src, trues(length(src.code)))
+        isrq = fill!(LoweredCodeUtils.initialize_isrequired(length(src.code)), true)
+        LoweredCodeUtils.print_with_code(io, src, isrq)
         str = String(take!(io))
         @test count("s = ", str) == 2
         @test count("i = ", str) == 1
@@ -470,7 +488,9 @@ end
         stmts = src.code
         edges = CodeEdges(m, src)
 
-        isrq = lines_required!(istypedef.(stmts), src, edges)
+        isrq = LoweredCodeUtils.initialize_isrequired(length(stmts))
+        copyto!(isrq, istypedef.(stmts))
+        lines_required!(isrq, src, edges)
         frame = Frame(m, src)
         selective_eval_fromstart!(frame, isrq, #=toplevel=#true)
 

test/signatures.jl

@@ -414,9 +414,10 @@ bodymethtest5(x, y=Dict(1=>2)) = 5
     oldenv = Pkg.project().path
     try
         # we test with the old version of CBinding, let's do it in an isolated environment
+        # so we don't cause package conflicts with everything else
         Pkg.activate(; temp=true, io=devnull)
 
-        @info "Adding CBinding to the environment for test purposes"
+        @info "Adding CBinding 0.9.4 to the environment for test purposes"
         Pkg.add(; name="CBinding", version="0.9.4", io=devnull) # `@cstruct` isn't defined for v1.0 and above
 
         m = Module()