Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code locations of iteration with break are incorrect. #485

Closed
bicycle1885 opened this issue May 29, 2021 · 3 comments · Fixed by JuliaLang/julia#41857
Closed

Code locations of iteration with break are incorrect. #485

bicycle1885 opened this issue May 29, 2021 · 3 comments · Fixed by JuliaLang/julia#41857

Comments

@bicycle1885
Copy link

I found Debugger.jl points to a wrong code location in the following example:

julia> @enter stringsearch("abra", "abra")
In stringsearch(a, b) at /home/kenta/workspace/JuliaInterpreter/bug.jl:1
 1  function stringsearch(a::String, b::String)
 2      found = true
>3      for i in eachindex(a)
 4          if a[i] != b[i]
 5              found = false
 6              break
 7          end

About to run: (eachindex)("abra")
1|debug> n
In stringsearch(a, b) at /home/kenta/workspace/JuliaInterpreter/bug.jl:1
 1  function stringsearch(a::String, b::String)
 2      found = true
 3      for i in eachindex(a)
>4          if a[i] != b[i]
 5              found = false
 6              break
 7          end
 8      end

About to run: (getindex)("abra", 1)
1|debug>
In stringsearch(a, b) at /home/kenta/workspace/JuliaInterpreter/bug.jl:1
  2      found = true
  3      for i in eachindex(a)
  4          if a[i] != b[i]
  5              found = false
> 6              break  <---- WRONG: it won't break since a[i] == b[i]
  7          end
  8      end
  9      return found
 10  end

About to run: (iterate)(Base.EachStringIndex{String}("abra"), 2)
1|debug>

This is because the current call frame misunderstand that the iteration comes from line 6, not line 3.

function stringsearch(a::String, b::String)
    found = true
    for i in eachindex(a)
        if a[i] != b[i]
            found = false
            break
        end
    end
    return found
end

using JuliaInterpreter
frame = JuliaInterpreter.enter_call(stringsearch, "abra", "abra")
show(IOContext(stdout, :limit => false), frame)
Frame for stringsearch(a::String, b::String) in Main at /home/kenta/workspace/JuliaInterpreter/bug.jl:1
   1 2  1 ─       found = true
   2 3  │   %2  = Base.eachindex(a)
   3 3  │         @_4 = Base.iterate(%2)
   4 3  │   %4  = @_4 === nothing
   5 3  │   %5  = ($(QuoteNode(Core.Intrinsics.not_int)))(%4)
   6 3  └──       goto #6 if not %5
   7 3  2 ┄ %7  = @_4
   8 3  │         i = Core.getfield(%7, 1)
   9 3  │   %9  = Core.getfield(%7, 2)
  10 4  │   %10 = Base.getindex(a, i)
  11 4  │   %11 = Base.getindex(b, i)
  12 4  │   %12 = %10 != %11
  13 4  └──       goto #4 if not %12
  14 5  3 ─       found = false
  15 6  └──       goto #6
  16 6  4 ─       @_4 = Base.iterate(%2, %9)  <---- This originates from line 3, not line 6.
  17 6  │   %17 = @_4 === nothing
  18 6  │   %18 = ($(QuoteNode(Core.Intrinsics.not_int)))(%17)
  19 6  └──       goto #6 if not %18
  20 6  5 ─       goto #2
  21 9  6 ┄       return found
a = "abra"
b = "abra"               _
@pfitzseb
Copy link
Member

pfitzseb commented Jun 2, 2021

I'd argue this is an issue with Base inserting the wrong line number node for this function (and probably iteration in general):

julia> Base.uncompressed_ast(first(methods(stringsearch)))
CodeInfo(
    @ REPL[12]:2 within `stringsearch'
1 ─       found = true
│   @ REPL[12]:3 within `stringsearch'
│   %2  = Main.eachindex(a)
│         @_4 = Base.iterate(%2)
│   %4  = @_4 === nothing
│   %5  = Base.not_int(%4)
└──       goto #6 if not %5
2 ┄ %7  = @_4
│         i = Core.getfield(%7, 1)
│   %9  = Core.getfield(%7, 2)
│   @ REPL[12]:4 within `stringsearch'
│   %10 = Base.getindex(a, i)
│   %11 = Base.getindex(b, i)
│   %12 = %10 != %11
└──       goto #4 if not %12
    @ REPL[12]:5 within `stringsearch'
3 ─       found = false
│   @ REPL[12]:6 within `stringsearch'
└──       goto #6
4 ─       @_4 = Base.iterate(%2, %9)
│   %17 = @_4 === nothing
│   %18 = Base.not_int(%17)
└──       goto #6 if not %18
5 ─       goto #2
    @ REPL[12]:9 within `stringsearch'
6 ┄       return found
)

I don't think we can do anything about this here.

@bicycle1885
Copy link
Author

Thank you. I'd keep this open until I open an issue upstream.

simeonschaub added a commit to JuliaLang/julia that referenced this issue Aug 10, 2021
Since lowering always inserts gotos and other instructions after loop
bodies, this allows debuggers to give a more useful location for these
instructions.

fixes JuliaDebug/JuliaInterpreter.jl#485
vtjnash pushed a commit to JuliaLang/julia that referenced this issue Aug 25, 2021
Since lowering always inserts gotos and other instructions after loop
bodies, this allows debuggers to give a more useful location for these
instructions.

fixes JuliaDebug/JuliaInterpreter.jl#485
@simeonschaub
Copy link
Collaborator

Closed by JuliaLang/julia#41857

LilithHafner pushed a commit to LilithHafner/julia that referenced this issue Feb 22, 2022
Since lowering always inserts gotos and other instructions after loop
bodies, this allows debuggers to give a more useful location for these
instructions.

fixes JuliaDebug/JuliaInterpreter.jl#485
LilithHafner pushed a commit to LilithHafner/julia that referenced this issue Mar 8, 2022
Since lowering always inserts gotos and other instructions after loop
bodies, this allows debuggers to give a more useful location for these
instructions.

fixes JuliaDebug/JuliaInterpreter.jl#485
Keno pushed a commit to JuliaLang/julia that referenced this issue Jun 5, 2024
Since lowering always inserts gotos and other instructions after loop
bodies, this allows debuggers to give a more useful location for these
instructions.

fixes JuliaDebug/JuliaInterpreter.jl#485
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants