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

fix printing when source code unavailable #22

Merged
merged 5 commits into from
Feb 25, 2019

Conversation

pfitzseb
Copy link
Member

This makes the REPL mode usable when we can't retrieve the actual source code by falling back to printing the lowered code:

julia> @eval function f(x)
           y = sin(x*x)
           y += x
           return y*y
         end
f (generic function with 1 method)

julia> Debugger.@enter f(3)
In f(x) at none:2
1   1 ─ %1 = ($(QuoteNode(*)))(x, x)
2   │        y = ($(QuoteNode(sin)))(%1)
3   │        y = ($(QuoteNode(+)))(y, x)

About to run: (*)(3, 3)
1|debug > n
In f(x) at none:2
1   1 ─ %1 = ($(QuoteNode(*)))(x, x)
2   │        y = ($(QuoteNode(sin)))(%1)
3   │        y = ($(QuoteNode(+)))(y, x)
4   │   %4 = ($(QuoteNode(*)))(y, y)
5   └──      return %4

About to run: (+)(0.4121184852417566, 3)
1|debug > 

This kinda breaks the connection between what is printed as line numbers and what n steps to, but I don't think we can realistically do anything about that. I'd also love to get rid of all the QuoteNode clutter and get something more similar to

julia> @code_lowered f(3)
CodeInfo(
1 ─ %1 = x * x
│        y = (Main.sin)(%1)
│        y = y + x
│   %4 = y * y
└──      return %4
)

but I don't know where to get that from.

Probably should be tested, but I'm not quite sure how the current tests work...

Fixes #21.

@pfitzseb
Copy link
Member Author

Ok, tests finally pass. Not sure I like special-casing tests for certain Julia versions, but eh...

@KristofferC
Copy link
Member

🎉

@KristofferC
Copy link
Member

Good to go?

@pfitzseb
Copy link
Member Author

Yup, unless you know how to make the printing prettier? :)

@pfitzseb
Copy link
Member Author

pfitzseb commented Feb 25, 2019

Actually, do you know if the $(QuoteNode(sin)) -> sin transform would ever be invalid? If it isn't I could just add another string processing step there.
Ideally all of this would happen with the actual CodeInfo object, but I don't have a clue on how to handle those properly.

@KristofferC
Copy link
Member

KristofferC commented Feb 25, 2019

The code for showing QuoteNodes is:

function show_unquoted(io::IO, ex::QuoteNode, indent::Int, prec::Int)
    if isa(ex.value, Symbol)
        show_unquoted_quote_expr(io, ex.value, indent, prec)
    else
        print(io, "\$(QuoteNode(")
        show(io, ex.value)
        print(io, "))")
    end
end

so perhaps a simple string replacement should work in most case (since we just want to do show(io, ex.value) anyway?

Another potential way would be to store the original non optimized CodeInfo and use that one for printing. Edit: Not sure this is feasible since the location of things in the original code info doesnt seem to be the same as the optimized one.

@KristofferC
Copy link
Member

KristofferC commented Feb 25, 2019

I guess the only problem is if have something like

a = QuoteNode(1)
function f()
    b = Main.a
    return b
end

String replacing QuoteNode in the show output for the optimized CodeInfo would be wrong here I think since it would show as $(QuoteNode(:($(QuoteNode(1))))))

However, if we make sure it is not prefixed by :( perhaps we are good?

@pfitzseb
Copy link
Member Author

Hm, if we only ever strip the outermost $(QuoteNode(...)) we should be good, no?

julia> function f()
           b = Main.a
           return b
       end
f (generic function with 1 method)

julia> Debugger.@enter f()
In f() at none:2
1   1 ─     b = :($(QuoteNode(1)))
2   └──     return b

About to run: return $(QuoteNode(1))
1|debug> n
:($(QuoteNode(1)))

@KristofferC
Copy link
Member

Hm, if we only ever strip the outermost $(QuoteNode(...)) we should be good, no?

Yes, I was just thinking about a naive string replacement function. Do you want to make the string parsing a bit more clever or modify the CodeInfo?

@pfitzseb
Copy link
Member Author

A naive regex based string replacement will only strip the outermost $(QuoteNode(...)) :)

I think it's probably fine to merge this as-is -- if the naive string replacement is in fact wrong in certain edge cases someone (e.g. @Keno) will complain.

@pfitzseb
Copy link
Member Author

I was also playing around with iterating over the expressions in frame.code.code.code (heh) directly and showing those, but we'd lose a lot of somewhat interesting info by doing that (or need to recreate the printing logic in Base, which looks pretty complicated afaict).

@KristofferC KristofferC merged commit b6e39f6 into JuliaDebug:master Feb 25, 2019
@pfitzseb pfitzseb deleted the sp/codeinfoprint branch February 25, 2019 15:17
@timholy
Copy link
Member

timholy commented Feb 25, 2019

Yes, the QuoteNodes do make it look uglier. This seems like a nice solution, thanks!

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 this pull request may close these issues.

3 participants