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

Rewrite based on lowered code #243

Merged
merged 43 commits into from
Mar 20, 2019
Merged

Rewrite based on lowered code #243

merged 43 commits into from
Mar 20, 2019

Conversation

timholy
Copy link
Owner

@timholy timholy commented Feb 25, 2019

This PR rewrites the package around JuliaInterpreter, which it uses to do much deeper analysis of the methods that get defined by code. A particularly noteworthy change is that it can now dive into methods defined via @eval, and it is finally able to handle kwarg functions robustly. The goal is to make Revise better at understanding the origin of methods and therefore better at deleting expunged ones and managing CodeTracking.jl's state for consumption by others.

This is a "developer preview" for upcoming changes. You need the following packages (probably in dev mode, and updated frequently):

Automatic revision is currently disabled; just manually execute revise() whenever you want to update. (It's much easier to debug failures if they're not running in a separate task.)

Still to do:

  • add tests that check the fraction of extracted signatures from Base
  • restore and/or improve the catching of exceptions generated by Revise
  • restore automatic revision
  • update the documentation
  • insert bounds in the registered version of Rebugger (this will break current versions)
  • work on startup and other latencies

@timholy
Copy link
Owner Author

timholy commented Feb 25, 2019

CC @KristofferC, @pfitzseb

timholy added 11 commits March 3, 2019 12:32
This massive change is intended to make Revise far more robust at identifying
method-signatures. The lowered code makes it easy to identify constructs
that create new methods.
Plots has changed its internals. This seems inevitable, so eventually
we'll have to replace these "use another package" tests with
a crafted example here.
timholy added 4 commits March 6, 2019 15:30
In the rewrite I've seen some cases of inappropriate deletion.
It's not certain this will prevent them, but it seems likely.
@timholy
Copy link
Owner Author

timholy commented Mar 18, 2019

Do you get the warnings with Revise.track(Base)? You shouldn't track individual files in Base, I am not sure what would happen (probably, lots of bad stuff).

@pfitzseb
Copy link
Collaborator

I don't.

@KristofferC
Copy link
Collaborator

I get the following warnings:

julia> using Revise; using Debugger

julia> let
           f_inv(x::Real) = x^2;
           f_inv(x::Integer) = 1 + invoke(f_inv, Tuple{Real}, x)
           @enter f_inv(2)
       end
┌ Warning: error evaluating in module Main
│   exception =
│    :(let
│          #= REPL[6]:2 =#
│          f_inv(x::Real) = begin
│                  #= REPL[6]:2 =#
│                  x ^ 2
│              end
│          #= REPL[6]:3 =#
│          f_inv(x::Integer) = begin
│                  #= REPL[6]:3 =#
│                  1 + invoke(f_inv, Tuple{Real}, x)
│              end
│          #= REPL[6]:4 =#
│          #= REPL[6]:4 =# @enter f_inv(2)
│      end)

@timholy
Copy link
Owner Author

timholy commented Mar 19, 2019

It's probably a world-age issue in that you're defining methods and then doing stuff with them. But commenting out the warning seems to let things run properly anyway...

Not quite sure what to do here.

@timholy
Copy link
Owner Author

timholy commented Mar 19, 2019

Oh wait, that warning had a mistake (from #243 (comment)), it's probably leading us to a real bug...

@cstjean
Copy link
Collaborator

cstjean commented Sep 20, 2019

Am I correct that this means that for a __precompile__(false) module, all macros are expanded by the interpreter? I've been on Revise 0.7.14 for a long time, and Julia 1.2 forced me to upgrade, but now my package just won't load (or rather, __init__ is called and finishes, but then it hangs for a very long time, seemingly proportional to the size of my macroexpansions).

@timholy
Copy link
Owner Author

timholy commented Sep 20, 2019

Sorry about the troubles. First, there is the Revise 1 branch, currently at 1.1.1. That does not use JuliaInterpreter.

Second, if building your package involves a lot of work that's done outside of an __init__, it might be slow. See #300, JuliaAstro/AstroBase.jl#31. If you let me know which package you're working on I might be able to provide more insight.

@cstjean
Copy link
Collaborator

cstjean commented Sep 20, 2019

It's a private package. I just filed an issue without seeing this comment, I'll try 1.1.1. Thank you for the help offer, and for all the work on Revise!

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.

5 participants