fix some cases of setting breakpoints in functions that contain macro…

… expansions (#527)

Project.toml

@@ -19,6 +19,7 @@ Distributed = "8ba89e20-285c-5b6f-9357-94700520ee1b"
 FunctionWrappers = "069b7b12-0de2-55c6-9aab-29f3d0a68a2e"
 HTTP = "cd3eb016-35fb-5094-929b-558a96fad6f3"
 LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e"
+Logging = "56ddb016-857b-54e1-b83d-db4d58db5568"
 Mmap = "a63ad114-7e13-5084-954f-fe012c677804"
 PyCall = "438e738f-606a-5dbb-bf0a-cddfbfd45ab0"
 SHA = "ea8e919c-243c-51af-8825-aaa63cd721ce"
@@ -27,4 +28,4 @@ Tensors = "48a634ad-e948-5137-8d70-aa71f2a747f4"
 Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
 
 [targets]
-test = ["DataFrames", "Dates", "Distributed", "FunctionWrappers", "HTTP", "LinearAlgebra", "Mmap", "PyCall", "SHA", "SparseArrays", "Tensors", "Test"]
+test = ["DataFrames", "Dates", "Distributed", "FunctionWrappers", "HTTP", "LinearAlgebra", "Logging", "Mmap", "PyCall", "SHA", "SparseArrays", "Tensors", "Test"]

docs/src/dev_reference.md

@@ -105,7 +105,6 @@ JuliaInterpreter.is_doc_expr
 JuliaInterpreter.is_global_ref
 CodeTracking.whereis
 JuliaInterpreter.linenumber
-JuliaInterpreter.statementnumber
 JuliaInterpreter.Variable
 JuliaInterpreter.locals
 JuliaInterpreter.whichtt

src/breakpoints.jl

@@ -62,11 +62,20 @@ function add_to_existing_framecodes(bp::AbstractBreakpoint)
     end
 end
 
-function add_breakpoint_if_match!(framecode::FrameCode, bp::AbstractBreakpoint)
+function add_breakpoint_if_match!(framecode::FrameCode, bp::BreakpointSignature)
     if framecode_matches_breakpoint(framecode, bp)
-        stmtidx = bp.line === 0 ? 1 : statementnumber(framecode, bp.line)
-        breakpoint!(framecode, stmtidx, bp.condition, bp.enabled[])
-        push!(bp.instances, BreakpointRef(framecode, stmtidx))
+        scope = framecode.scope
+        matching_file = if scope isa Method
+            scope.file
+        else
+            # TODO: make more precise?
+            first(framecode.src.linetable).file
+        end
+        stmtidxs = bp.line === 0 ? [1] : statementnumbers(framecode, bp.line, matching_file::Symbol)
+        stmtidxs === nothing && return
+        breakpoint!(framecode, stmtidxs, bp.condition, bp.enabled[])
+        foreach(stmtidx -> push!(bp.instances, BreakpointRef(framecode, stmtidx)), stmtidxs)
+        return
     end
 end
 
@@ -156,29 +165,24 @@ function breakpoint(file::AbstractString, line::Integer, condition::Condition=no
     return bp
 end
 
-function framecode_matches_breakpoint(framecode::FrameCode, bp::BreakpointFileLocation)
-    if framecode.scope isa Method
-        meth = framecode.scope::Method
-        methpath = CodeTracking.maybe_fix_path(String(meth.file))
-        ispath(methpath) && (methpath = realpath(methpath))
-        if bp.abspath == methpath || endswith(methpath, bp.path)
-            return method_contains_line(meth, bp.line)
-        else
-            return false
-        end
-    else
-        w = whereis(framecode, 1)
-        w === nothing && return false
-        path, _ = w
-        path = CodeTracking.maybe_fix_path(path)
-        ispath(path) && (path = realpath(path))
-
-        if bp.abspath == path || endswith(path, bp.path)
-            return toplevel_code_contains_line(framecode, bp.line)
-        else
-            return false
+function add_breakpoint_if_match!(framecode::FrameCode, bp::BreakpointFileLocation)
+    framecode_contains_file = false
+    matching_file = nothing
+    for file in framecode.unique_files
+        filepath = CodeTracking.maybe_fix_path(String(file))
+        if Base.samefile(bp.abspath, filepath) || endswith(filepath, bp.path)
+            framecode_contains_file = true
+            matching_file = file
+            break
         end
     end
+    framecode_contains_file || return nothing
+
+    stmtidxs = bp.line === 0 ? [1] : statementnumbers(framecode, bp.line, matching_file::Symbol)
+    stmtidxs === nothing && return
+    breakpoint!(framecode, stmtidxs, bp.condition, bp.enabled[])
+    foreach(stmtidx -> push!(bp.instances, BreakpointRef(framecode, stmtidx)), stmtidxs)
+    return
 end
 
 function shouldbreak(frame::Frame, pc::Int)
@@ -240,6 +244,8 @@ function breakpoint!(framecode::FrameCode, pc, condition::Condition=nothing, ena
         framecode.breakpoints[stmtidx] = BreakpointState(enabled, Core.eval(mod, fex))
     end
 end
+breakpoint!(framecode::FrameCode, pcs::AbstractArray, condition::Condition=nothing, enabled=true) = 
+    foreach(pc -> breakpoint!(framecode, pc, condition, enabled), pcs)
 breakpoint!(frame::Frame, pc=frame.pc, condition::Condition=nothing) =
     breakpoint!(frame.framecode, pc, condition)
 

src/construct.jl

@@ -35,14 +35,14 @@ const compiled_modules = Set{Module}()
 
 const junk_framedata = FrameData[] # to allow re-use of allocated memory (this is otherwise a bottleneck)
 const junk_frames = Frame[]
-debug_recycle() = false
+debug_mode() = false
 @noinline function _check_frame_not_in_junk(frame)
     @assert frame.framedata ∉ junk_framedata
     @assert frame ∉ junk_frames
 end
 
 @inline function recycle(frame)
-    debug_recycle() && _check_frame_not_in_junk(frame)
+    debug_mode() && _check_frame_not_in_junk(frame)
     push!(junk_framedata, frame.framedata)
     push!(junk_frames, frame)
 end

src/types.jl

@@ -97,6 +97,7 @@ struct FrameCode
     used::BitSet
     generator::Bool   # true if this is for the expression-generator of a @generated function
     report_coverage::Bool
+    unique_files::Set{Symbol}
 end
 
 const BREAKPOINT_EXPR = :($(QuoteNode(getproperty))($JuliaInterpreter, :__BREAKPOINT_MARKER__))
@@ -131,7 +132,14 @@ function FrameCode(scope, src::CodeInfo; generator=false, optimize=true)
     end
     used = find_used(src)
     report_coverage = do_coverage(moduleof(scope))
-    framecode = FrameCode(scope, src, methodtables, breakpoints, slotnamelists, used, generator, report_coverage)
+
+    lt = linetable(src)
+    unique_files = Set{Symbol}()
+    for entry in lt
+        push!(unique_files, entry.file)
+    end
+
+    framecode = FrameCode(scope, src, methodtables, breakpoints, slotnamelists, used, generator, report_coverage, unique_files)
     if scope isa Method
         for bp in _breakpoints
             # Manual union splitting

src/utils.jl

@@ -38,7 +38,9 @@ function whichtt(@nospecialize(tt))
         # for now, actual code execution doesn't ever need to consider overlayed method table
         result = Core.Compiler._findsup(tt, nothing, get_world_counter())
         result === nothing && return nothing
-        return first(result).method
+        fresult = first(result)
+        fresult === nothing && return nothing
+        return fresult.method
     else
         m = ccall(:jl_gf_invoke_lookup, Any, (Any, UInt), tt, get_world_counter())
         m === nothing && return nothing
@@ -378,42 +380,74 @@ function compute_corrected_linerange(method::Method)
     return line1:getline(lastline) + offset
 end
 
-function method_contains_line(method::Method, line::Integer)
-    # Check to see if this method really contains that line. Methods that fill in a default positional argument,
-    # keyword arguments, and @generated sections may not contain the line.
-    return line in compute_corrected_linerange(method)
-end
-
-function toplevel_code_contains_line(framecode::FrameCode, line::Integer)
-    return getline(linetable(framecode, 1)) <= line <= getline(last(linetable(framecode)))
+function compute_linerange(framecode)
+    getline(linetable(framecode, 1)):getline(last(linetable(framecode)))
 end
 
-"""
-    stmtidx = statementnumber(frame, line::Integer)
-
-Return the index of the first statement in `frame`'s `CodeInfo` that corresponds to
-static line number `line`.
-"""
-function statementnumber(framecode::FrameCode, line::Integer)
-    sortby(lin) = isa(lin, Int) ? lin : getline(lin)  # for comparison to x=Int(line)
+function statementnumbers(framecode::FrameCode, line::Integer, file::Symbol)
+    # Check to see if this framecode really contains that line. Methods that fill in a default positional argument,
+    # keyword arguments, and @generated sections may not contain the line.
+    scope = framecode.scope
+    offset = if scope isa Method
+        method = scope
+        _, line1 = whereis(method)
+        line1 - method.line
+    else
+        0
+    end
 
     lt = linetable(framecode)
-    lineidx = searchsortedfirst(lt, Int(line); by=sortby)::Int
-    1 <= lineidx <= length(lt) || throw(ArgumentError("line $line not found in $(framecode.scope)"))
-    return searchsortedfirst(codelocs(framecode), lineidx)
-end
-statementnumber(frame::Frame, line) = statementnumber(frame.framecode, line)
 
-"""
-    framecode, stmtidx = statementnumber(method, line)
+    # Check if the exact line number exist
+    idxs = findall(entry -> entry.line + offset == line && entry.file == file, lt)
+    locs = codelocs(framecode)
+    if !isempty(idxs)
+        stmtidxs = Int[]
+        stmtidx = 1
+        while stmtidx <= length(locs)
+            loc = locs[stmtidx] 
+            if loc in idxs
+                push!(stmtidxs, stmtidx)
+                stmtidx += 1
+                # Skip continous statements that are on the same line 
+                while stmtidx <= length(locs) && loc == locs[stmtidx]
+                    stmtidx += 1
+                end
+            else
+                stmtidx += 1
+            end
+        end
+        return stmtidxs
+    end
+
+
+    # If the exact line number does not exist in the line table, take the one that is closest after that line
+    # restricted to the line range of the current scope.
+    scope = framecode.scope 
+    range = scope isa Method ? compute_corrected_linerange(scope) : compute_linerange(framecode)
+    if line in range
+        closest = nothing
+        closest_idx = nothing
+        for (i, entry) in enumerate(lt)
+            if entry.file == file && entry.line in range && entry.line >= line
+                if closest === nothing
+                    closest = entry
+                    closest_idx = i
+                else
+                    if entry.line < closest.line
+                        closest = entry
+                        closest_idx = i
+                    end
+                end
+            end
+        end
+        if closest_idx !== nothing
+            idx = findfirst(i-> i==closest_idx, locs)
+            return idx === nothing ? nothing : Int[idx]
+        end
+    end
 
-Return the index of the first statement in `framecode` that corresponds to the given
-static line number `line` in `method`.
-"""
-function statementnumber(method::Method, line; line1=whereis(method)[2])
-    linec = line - line1 + method.line  # line number at time of compilation
-    framecode = get_framecode(method)
-    return framecode, statementnumber(framecode, linec)
+    return nothing
 end
 
 ## Printing

test/breakpoints.jl

@@ -484,3 +484,96 @@ Constructor(x::AbstractString, y::Int) = Constructor(x)
     frame, bp = @interpret Constructor(2)
     @test bp isa BreakpointRef
 end
+
+@testset "test breaking on a line with no statement" begin
+    ln = @__LINE__
+    function f_emptylines()
+        sin(2.0)
+
+
+
+        return sin(2.0)
+    end
+
+    bp = breakpoint(@__FILE__, ln + 4)
+    frame, bpref = @interpret f_emptylines()
+    @test bpref isa BreakpointRef
+    @test JuliaInterpreter.whereis(frame) == (@__FILE__, ln + 6)
+    remove(bp)
+
+    # Don't break if the line is outside the function
+    breakpoint(@__FILE__, ln)
+    @test (@interpret f_emptylines()) == sin(2.0)
+end
+
+@testset "macro expansion breakpoint tests" begin
+    function f_macro()
+        sin(2.0)
+        @info "foo"
+        sin(2.0)
+        @info "foo"
+        return 2
+    end
+    frame = JuliaInterpreter.enter_call(f_macro)
+    file_logging = "logging.jl"
+    line_logging = 0
+    for entry in frame.framecode.src.linetable
+        if entry.file == Symbol(file_logging)
+            line_logging = entry.line
+            break
+        end
+    end
+    bp_log = breakpoint(file_logging, line_logging)
+    with_logger(NullLogger()) do
+        frame, bp = @interpret f_macro()
+        @test bp isa BreakpointRef
+        file, ln = JuliaInterpreter.whereis(frame)
+        @test ln == line_logging
+        @test basename(file) == file_logging
+        bp = JuliaInterpreter.finish_stack!(frame)
+        @test bp isa BreakpointRef
+        frame = leaf(frame)
+        ret = JuliaInterpreter.finish_stack!(frame)
+        @test ret == 2
+    end
+
+    JuliaInterpreter.remove(bp_log)
+
+    # Check that stopping on a line only stops in the correct file
+    mktemp() do path, io
+        for _ in 1:line_logging-5
+            print(io, "\n")
+        end
+        print(io, 
+        """
+        function f_check(x)
+            sin(x)
+            @info "foo"
+            sin(x)
+            sin(x)
+            sin(x)
+            sin(x)
+            sin(x)
+
+            return x
+        end
+        """)
+        bp_f = breakpoint(path, line_logging)
+        flush(io)
+        include(path)
+
+        with_logger(NullLogger()) do
+            frame, bp = @interpret f_check(1)
+            file, ln = JuliaInterpreter.whereis(frame)
+            @test file == path # Should not have stopped in logging.jl at line `line_logging`
+            @test ln == line_logging
+            remove(bp_f)
+            @test (@interpret f_check(1)) == 1
+            breakpoint(f_check, line_logging)
+            frame, bp = @interpret f_check(1)
+            file, ln = JuliaInterpreter.whereis(frame)
+            @test file == path # Should not have stopped in logging.jl at line `line_logging`
+            @test ln == line_logging
+        end
+    end
+end

test/runtests.jl

@@ -1,13 +1,14 @@
 using JuliaInterpreter
 using Test
+using Logging
 
 @test isempty(detect_ambiguities(JuliaInterpreter, Base, Core))
 
 if !isdefined(@__MODULE__, :read_and_parse)
     include("utils.jl")
 end
 
-Core.eval(JuliaInterpreter, :(debug_recycle() = true))
+Core.eval(JuliaInterpreter, :(debug_mode() = true))
 
 @testset "Main tests" begin
     include("core.jl")