Export enable, disable, and remove, and rework some types

This makes a clearer distinction between a "breakpoint reference" and
the actual information needed to test the breakpoint.

Author: timholy

src/JuliaInterpreter.jl

@@ -11,7 +11,8 @@ using UUIDs
 using Random.DSFMT
 using InteractiveUtils
 
-export @enter, @make_stack, @interpret, Compiled, JuliaStackFrame, Breakpoints, breakpoint
+export @enter, @make_stack, @interpret, Compiled, JuliaStackFrame,
+       Breakpoints, breakpoint, @breakpoint, breakpoints, enable, disable, remove
 
 module CompiledCalls
 # This module is for handling intrinsics that must be compiled (llvmcall)
@@ -40,6 +41,29 @@ isless(x::JuliaProgramCounter, y::Integer) = isless(x.next_stmt, y)
 
 Base.show(io::IO, pc::JuliaProgramCounter) = print(io, "JuliaProgramCounter(", pc.next_stmt, ')')
 
+truecondition(frame) = true
+falsecondition(frame) = false
+
+"""
+    BreakpointState(isactive=true, condition=JuliaInterpreter.truecondition)
+
+`BreakpointState` represents a breakpoint at a particular statement in
+a `JuliaFrameCode`. `isactive` indicates whether the breakpoint is currently
+[`enable`](@ref)d or [`disable`](@ref)d. `condition` is a function that accepts
+a single `JuliaStackFrame`, and `condition(frame)` must return either
+`true` or `false`. Execution will stop at a breakpoint only if `isactive`
+and `condition(frame)` both evaluate as `true`. The default `condition` always
+returns `true`.
+
+To create these objects, see [`breakpoint`](@ref).
+"""
+struct BreakpointState
+    isactive::Bool
+    condition::Function
+end
+BreakpointState(isactive::Bool) = BreakpointState(isactive, truecondition)
+BreakpointState() = BreakpointState(true)
+
 # A type used transiently in renumbering CodeInfo SSAValues (to distinguish a new SSAValue from an old one)
 struct NewSSAValue
     id::Int
@@ -61,7 +85,7 @@ struct JuliaFrameCode
     scope::Union{Method,Module}
     code::CodeInfo
     methodtables::Vector{Union{Compiled,TypeMapEntry}} # line-by-line method tables for generic-function :call Exprs
-    breakpoints::Vector{Tuple{Bool,Function}}          # (isactive, condition)
+    breakpoints::Vector{BreakpointState}
     used::BitSet
     wrapper::Bool
     generator::Bool
@@ -81,7 +105,7 @@ function JuliaFrameCode(scope, code::CodeInfo; wrapper=false, generator=false, f
         code = copy_codeinfo(code)
         methodtables = Vector{Union{Compiled,TypeMapEntry}}(undef, length(code.code))
     end
-    breakpoints = Vector{Tuple{Bool,Function}}(undef, length(code.code))
+    breakpoints = Vector{BreakpointState}(undef, length(code.code))
     used = find_used(code)
     return JuliaFrameCode(scope, code, methodtables, breakpoints, used, wrapper, generator, fullpath)
 end
@@ -323,6 +347,16 @@ function prepare_framecode(method::Method, argtypes; enter_generated=false)
     return framecode, lenv
 end
 
+function get_framecode(method)
+    framecode = get(framedict, method, nothing)
+    if framecode === nothing
+        code = get_source(method)
+        framecode = JuliaFrameCode(method, code; generator=false)
+        framedict[method] = framecode
+    end
+    return framecode
+end
+
 function whichtt(tt)
     m = ccall(:jl_gf_invoke_lookup, Any, (Any, UInt), tt, typemax(UInt))
     m === nothing && return nothing
@@ -1110,7 +1144,7 @@ end
 
 include("breakpoints.jl")
 using .Breakpoints
-using .Breakpoints: shouldbreak, Breakpoint
+using .Breakpoints: shouldbreak, BreakpointRef
 
 include("precompile.jl")
 _precompile_()

src/breakpoints.jl

@@ -1,39 +1,61 @@
 module Breakpoints
 
 using ..JuliaInterpreter
-using JuliaInterpreter: JuliaFrameCode, JuliaStackFrame,
-                        prepare_framecode, framedict, get_source, sparam_syms,
-                        linenumber
+using JuliaInterpreter: JuliaFrameCode, JuliaStackFrame, BreakpointState,
+                        truecondition, falsecondition, prepare_framecode, get_framecode,
+                        sparam_syms, linenumber
+using Base.Meta: isexpr
+using InteractiveUtils
 
-export breakpoint
+export @breakpoint, breakpoint, enable, disable, remove
 
 # A type that is unique to this package for which there are no valid operations
 struct Unassigned end
 
 """
-    Breakpoint(framecode, stmtidx)
+    BreakpointRef(framecode, stmtidx)
 
 A reference to a breakpoint at a particular statement index `stmtidx` in `framecode`.
 """
-struct Breakpoint
+struct BreakpointRef
     framecode::JuliaFrameCode
     stmtidx::Int
 end
 
-function Base.show(io::IO, bp::Breakpoint)
+function Base.show(io::IO, bp::BreakpointRef)
     lineno = linenumber(bp.framecode, bp.stmtidx)
     print(io, "breakpoint(", bp.framecode.scope, ", ", lineno, ')')
 end
 
-const _breakpoints = Set{Breakpoint}()
+const _breakpoints = BreakpointRef[]
+breakpoints() = copy(_breakpoints)
+
+Base.getindex(bp::BreakpointRef) = bp.framecode.breakpoints[bp.stmtidx]
+function Base.setindex!(bp::BreakpointRef, isactive::Bool)
+    bp.framecode.breakpoints[bp.stmtidx] = BreakpointState(isactive, bp[].condition)
+end
+function toggle!(bp::BreakpointRef)
+    state = bp[]
+    bp.framecode.breakpoints[bp.stmtidx] = BreakpointState(!state.isactive, state.condition)
+end
+
+function add_breakpoint(framecode, stmtidx)
+    bp = BreakpointRef(framecode, stmtidx)
+    # Since there can be only one BreakpointState for a given framecode/stmtidx,
+    # check whether _breakpoints is already storing a reference to that location
+    idx = findfirst(isequal(bp), _breakpoints)
+    if idx === nothing
+        push!(_breakpoints, bp)
+    end
+    return bp
+end
 
 function shouldbreak(frame, pc=frame.pc[])
     idx = convert(Int, pc)
     isassigned(frame.code.breakpoints, idx) || return false
     bp = frame.code.breakpoints[idx]
-    bp[1] || return false
-    slotfunction = bp[2]
-    return slotfunction(frame)::Bool
+    bp.isactive || return false
+    return bp.condition(frame)::Bool
 end
 
 function prepare_slotfunction(framecode::JuliaFrameCode, body::Union{Symbol,Expr})
@@ -65,63 +87,35 @@ function prepare_slotfunction(framecode::JuliaFrameCode, body::Union{Symbol,Expr
     return Expr(:function, Expr(:call, funcname, framename), Expr(:block, assignments..., body))
 end
 
-truecondition(frame) = true
-falsecondition(frame) = false
 
 ## The fundamental implementations of breakpoint-setting
 function breakpoint!(framecode::JuliaFrameCode, pc, condition::Union{Bool,Expr}=true)
     stmtidx = convert(Int, pc)
     if isa(condition, Bool)
-        framecode.breakpoints[stmtidx] = condition ? (true, truecondition) : (false, falsecondition)
+        framecode.breakpoints[stmtidx] = BreakpointState(condition)
     else
         fex = prepare_slotfunction(framecode, condition)
-        framecode.breakpoints[stmtidx] = (true, eval(fex))
+        framecode.breakpoints[stmtidx] = BreakpointState(true, eval(fex))
     end
-    bp = Breakpoint(framecode, stmtidx)
-    push!(_breakpoints, bp)
-    return bp
+    return add_breakpoint(framecode, stmtidx)
 end
 breakpoint!(frame::JuliaStackFrame, pc=frame.pc[], condition::Union{Bool,Expr}=true) =
     breakpoint!(frame.code, pc, condition)
 
-function set_breakpoint_active!(framecode::JuliaFrameCode, active, pc)
-    stmtidx = convert(Int, pc)
-    _, condition = framecode.breakpoints[stmtidx]
-    framecode.breakpoints[stmtidx] = (active, condition)
-    active
-end
-set_breakpoint_active!(frame::JuliaStackFrame, active, pc) =
-    set_breakpoint_active!(frame.code, active, pc)
-
-function get_breakpoint_active(framecode::JuliaFrameCode, pc)
-    stmtidx = convert(Int, pc)
-    return framecode.breakpoints[stmtidx][1]
-end
-get_breakpoint_active(frame::JuliaStackFrame, pc) =
-    get_breakpoint_active(frame.code, pc)
-
-function toggle_breakpoint_active!(framecode::JuliaFrameCode, pc)
-    stmtidx = convert(Int, pc)
-    active, condition = framecode.breakpoints[stmtidx]
-    framecode.breakpoints[stmtidx] = (!active, condition)
-    !active
-end
-toggle_breakpoint_active!(frame::JuliaStackFrame, active, pc) =
-    toggle_breakpoint_active!(frame.code, active, pc)
-
-enable(bp::Breakpoint)  = set_breakpoint_active!(bp.framecode, true, bp.stmtidx)
-disable(bp::Breakpoint) = set_breakpoint_active!(bp.framecode, false, bp.stmtidx)
-function remove(bp::Breakpoint)
-    bp.framecode.breakpoints[bp.stmtidx] = (false, falsecondition)
-    delete!(_breakpoints, bp)
+enable(bp::BreakpointRef)  = bp[] = true
+disable(bp::BreakpointRef) = bp[] = false
+function remove(bp::BreakpointRef)
+    idx = findfirst(isequal(bp), _breakpoints)
+    deleteat!(_breakpoints, idx)
+    bp.framecode.breakpoints[bp.stmtidx] = BreakpointState(false, falsecondition)
     return nothing
 end
 
 enable() = for bp in _breakpoints enable(bp) end
 disable() = for bp in _breakpoints disable(bp) end
 function remove()
     for bp in _breakpoints
-        bp.framecode.breakpoints[bp.stmtidx] = (false, falsecondition)
+        bp.framecode.breakpoints[bp.stmtidx] = BreakpointState(false, falsecondition)
     end
     empty!(_breakpoints)
     return nothing
@@ -155,18 +149,10 @@ end
     breakpoint(method::Method)
     breakpoint(method::Method, condition::Expr)
 
-Add a breakpoint upon entry to `method`. The first will break unconditionally, the second
-only if `condition` evaluates to `true`. `condition` should be written in terms of the
-arguments and local variables of `method`.
+Add a breakpoint upon entry to `method`.
 """
 function breakpoint(method::Method, condition::Union{Bool,Expr}=true)
-    # FIXME: this duplicates stuff in prepare_call
-    framecode = get(framedict, method, nothing)
-    if framecode === nothing
-        code = get_source(method)
-        framecode = JuliaFrameCode(method, code; generator=false)
-        framedict[method] = framecode
-    end
+    framecode = get_framecode(method)
     breakpoint!(framecode, 1, condition)
 end
 
@@ -177,10 +163,11 @@ end
 Break-on-entry to all methods of `f`.
 """
 function breakpoint(f, condition::Union{Bool,Expr}=true)
+    bps = BreakpointRef[]
     for method in methods(f)
-        breakpoint(method, condition)
+        push!(bps, breakpoint(method, condition))
     end
-    nothing
+    return bps
 end
 
 end

src/interpret.jl

@@ -208,7 +208,7 @@ function evaluate_call!(stack, frame::JuliaStackFrame, call_expr::Expr, pc; exec
     newframe = build_frame(framecode, fargs, lenv)
     if shouldbreak(newframe)
         push!(stack, newframe)
-        return Breakpoint(newframe.code, newframe.pc[])
+        return BreakpointRef(newframe.code, newframe.pc[])
     end
     ret = exec!(stack, newframe)
     pop!(stack)
@@ -362,7 +362,7 @@ function _step_expr!(stack, frame, @nospecialize(node), pc::JuliaProgramCounter,
                 else
                     rhs = istoplevel ? @lookup(moduleof(frame), frame, rhs) : @lookup(frame, rhs)
                 end
-                isa(rhs, Breakpoint) && return rhs
+                isa(rhs, BreakpointRef) && return rhs
                 do_assignment!(frame, lhs, rhs)
             elseif node.head == :gotoifnot
                 arg = @lookup(frame, node.args[1])
@@ -456,7 +456,7 @@ function _step_expr!(stack, frame, @nospecialize(node), pc::JuliaProgramCounter,
     catch err
         return handle_err(frame, err)
     end
-    @isdefined(rhs) && isa(rhs, Breakpoint) && return rhs
+    @isdefined(rhs) && isa(rhs, BreakpointRef) && return rhs
     if isassign(frame, pc)
         if !@isdefined(rhs)
             @show frame node pc
@@ -480,7 +480,7 @@ by normal dispatch, whereas a vector of `JuliaStackFrame`s will use recursive in
 """
 function step_expr!(stack, frame, istoplevel::Bool=false)
     pc = _step_expr!(stack, frame, frame.pc[], istoplevel)
-    (pc === nothing || isa(pc, Breakpoint)) && return pc
+    (pc === nothing || isa(pc, BreakpointRef)) && return pc
     frame.pc[] = pc
 end
 
@@ -515,16 +515,16 @@ function finish!(stack, frame, pc::JuliaProgramCounter=frame.pc[], istoplevel::B
     local new_pc
     while true
         new_pc = _step_expr!(stack, frame, pc, istoplevel)
-        (new_pc == nothing || isa(new_pc, Breakpoint)) && break
+        (new_pc == nothing || isa(new_pc, BreakpointRef)) && break
         pc = new_pc
         if shouldbreak(frame, pc)
             push!(stack, frame)
-            new_pc = Breakpoint(frame.code, pc)
+            new_pc = BreakpointRef(frame.code, pc)
             break
         end
     end
     frame.pc[] = pc
-    return isa(new_pc, Breakpoint) ? new_pc : pc
+    return isa(new_pc, BreakpointRef) ? new_pc : pc
 end
 finish!(stack, frame, istoplevel::Bool) = finish!(stack, frame, frame.pc[], istoplevel)
 
@@ -543,7 +543,7 @@ Optionally supply the starting `pc`, if you don't want to start at the current l
 """
 function finish_and_return!(stack, frame, pc::JuliaProgramCounter=frame.pc[], istoplevel::Bool=false)
     pc = finish!(stack, frame, pc, istoplevel)
-    isa(pc, Breakpoint) && return pc
+    isa(pc, BreakpointRef) && return pc
     return get_return(frame, pc)
 end
 finish_and_return!(stack, frame, istoplevel::Bool) = finish_and_return!(stack, frame, frame.pc[], istoplevel)
@@ -572,7 +572,7 @@ function finish_stack!(stack)
     while !isempty(stack)
         frame = pop!(stack)
         ret = finish_and_return!(stack, frame)
-        isa(ret, Breakpoint) && return ret
+        isa(ret, BreakpointRef) && return ret
         isempty(stack) && return ret
         parentframe = stack[end]
         pc = parentframe.pc[]
@@ -582,7 +582,7 @@ function finish_stack!(stack)
         end
         pc += 1
         parentframe.pc[] = pc
-        shouldbreak(parentframe) && return Breakpoint(parentframe.code, pc)
+        shouldbreak(parentframe) && return BreakpointRef(parentframe.code, pc)
     end
     error("stack is empty")
 end
@@ -710,7 +710,7 @@ function next_line!(stack, frame, dbstack = nothing)
         # If this is a goto node, step it and reevaluate
         if isgotonode(expr)
             pc = _step_expr!(stack, frame, pc)
-            (pc == nothing || isa(pc, Breakpoint)) && return pc
+            (pc == nothing || isa(pc, BreakpointRef)) && return pc
         elseif dbstack !== nothing && iswrappercall(expr)
             # With splatting it can happen that we do something like ssa = tuple(#self#), _apply(ssa), which
             # confuses the logic here, just step into the first call that's not a builtin
@@ -727,15 +727,15 @@ function next_line!(stack, frame, dbstack = nothing)
                     break
                 else
                     pc = _step_expr!(stack, frame, pc)
-                    (pc == nothing || isa(pc, Breakpoint)) && return pc
+                    (pc == nothing || isa(pc, BreakpointRef)) && return pc
                 end
             end
         else
             pc = _step_expr!(stack, frame, pc)
-            (pc == nothing || isa(pc, Breakpoint)) && return pc
+            (pc == nothing || isa(pc, BreakpointRef)) && return pc
         end
         frame.pc[] = pc
-        shouldbreak(frame, pc) && return Breakpoint(frame.code, pc)
+        shouldbreak(frame, pc) && return BreakpointRef(frame.code, pc)
     end
     maybe_next_call!(stack, frame, pc)
 end