From f282cc7c7286820a8445533b69ccba54ca7409d8 Mon Sep 17 00:00:00 2001 From: Tim Holy Date: Thu, 28 May 2020 15:55:43 -0500 Subject: [PATCH] Rewrite based on LoweredCodeUtils.CodeEdges This is a major rewrite of the backedge component. Moving it to LoweredCodeUtils and giving it a "real" API led to many improvements in selectivity of code-evaluation. This rewrite aims to make Revise do just one thing, which is to update methods. "Data" is not touched. In this world, #249 is not a bug. The benefit is that `includet` should work much more reliably to distinguish "code" from "method definitions"; by and large, the days of needing to separate code from method definitions in order to avoid confusing Revise should be over. Importantly, this introduces multiple "modes" of operation. `:sigs` is for signature extraction, `:eval` is for full evaluation (as you might do during the initial `includet` on a file), and `:evalmeth` is used during revision. This is essentially a generalization of `define::Bool` to allow more nuanced behavior. Fixes #479 Closes #249 --- Project.toml | 4 +- src/Revise.jl | 53 ++++++--- src/backedges.jl | 268 ---------------------------------------------- src/lowered.jl | 248 +++++++++++++++++++++++++++++------------- src/pkgs.jl | 2 +- src/precompile.jl | 22 ++-- src/utils.jl | 2 + test/backedges.jl | 6 +- test/runtests.jl | 62 +++++------ test/sigtest.jl | 3 +- 10 files changed, 264 insertions(+), 406 deletions(-) delete mode 100644 src/backedges.jl diff --git a/Project.toml b/Project.toml index 6e129af9..4773bf11 100644 --- a/Project.toml +++ b/Project.toml @@ -1,6 +1,6 @@ name = "Revise" uuid = "295af30f-e4ad-537b-8983-00126c2a3abe" -version = "2.7.0" +version = "3.0.0" [deps] CodeTracking = "da1fd8a2-8d9e-5ec2-8556-3022fb5608a2" @@ -18,7 +18,7 @@ Unicode = "4ec0a83e-493e-50e2-b9ac-8f72acf5a8f5" [compat] CodeTracking = "0.5.9" JuliaInterpreter = "0.7.16" -LoweredCodeUtils = "0.4" +LoweredCodeUtils = "1.1.0" OrderedCollections = "1" julia = "1" diff --git a/src/Revise.jl b/src/Revise.jl index 2adee4b3..67f9bc3a 100644 --- a/src/Revise.jl +++ b/src/Revise.jl @@ -14,8 +14,8 @@ using OrderedCollections, CodeTracking, JuliaInterpreter, LoweredCodeUtils using CodeTracking: PkgFiles, basedir, srcfiles using JuliaInterpreter: whichtt, is_doc_expr, step_expr!, finish_and_return!, get_return using JuliaInterpreter: @lookup, moduleof, scopeof, pc_expr, prepare_thunk, split_expressions, - linetable, codelocs, LineTypes -using LoweredCodeUtils: next_or_nothing!, isanonymous_typedef, define_anonymous + linetable, codelocs, LineTypes, is_global_ref, is_quotenode +using LoweredCodeUtils: next_or_nothing!, trackedheads, structheads, callee_matches export revise, includet, entr, MethodSummary @@ -67,11 +67,12 @@ environment variable to customize it. """ const tracking_Main_includes = Ref(false) +const revise_mode = Ref(:evalmeth) + include("relocatable_exprs.jl") include("types.jl") include("utils.jl") include("parsing.jl") -include("backedges.jl") include("lowered.jl") include("pkgs.jl") include("git.jl") @@ -421,7 +422,7 @@ function eval_new!(exs_sigs_new::ExprsSigs, exs_sigs_old, mod::Module) # ex is not present in old @debug "Eval" _group="Action" time=time() deltainfo=(mod, ex) # try - sigs, deps, _includes, thunk = eval_with_signatures(mod, ex) # All signatures defined by `ex` + sigs, deps, _includes, thunk = eval_with_signatures(mod, ex; mode=revise_mode[]) # All signatures defined by `ex` append!(includes, _includes) if VERSION < v"1.3.0" || !isexpr(thunk, :thunk) thunk = ex @@ -475,6 +476,26 @@ function eval_new!(mod_exs_sigs_new::ModuleExprsSigs, mod_exs_sigs_old) return mod_exs_sigs_new, includes end +""" + CodeTrackingMethodInfo(ex::Expr) + +Create a cache for storing information about method definitions. +Adding signatures to such an object inserts them into `CodeTracking.method_info`, +which maps signature Tuple-types to `(lnn::LineNumberNode, ex::Expr)` pairs. +Because method signatures are unique within a module, this is the foundation for +identifying methods in a manner independent of source-code location. + +It also has the following fields: + +- `exprstack`: used when descending into `@eval` statements (via `push_expr` and `pop_expr!`) + `ex` (used in creating the `CodeTrackingMethodInfo` object) is the first entry in the stack. +- `allsigs`: a list of all method signatures defined by a given expression +- `deps`: list of top-level named objects (`Symbol`s and `GlobalRef`s) that method definitions + in this block depend on. For example, `if Sys.iswindows() f() = 1 else f() = 2 end` would + store `Sys.iswindows` here. +- `includes`: a list of `module=>filename` for any `include` statements encountered while the + expression was parsed. +""" struct CodeTrackingMethodInfo exprstack::Vector{Expr} allsigs::Vector{Any} @@ -490,12 +511,18 @@ function add_signature!(methodinfo::CodeTrackingMethodInfo, @nospecialize(sig), end push_expr!(methodinfo::CodeTrackingMethodInfo, mod::Module, ex::Expr) = (push!(methodinfo.exprstack, ex); methodinfo) pop_expr!(methodinfo::CodeTrackingMethodInfo) = (pop!(methodinfo.exprstack); methodinfo) -function add_dependencies!(methodinfo::CodeTrackingMethodInfo, be::BackEdges, src, chunks) +function add_dependencies!(methodinfo::CodeTrackingMethodInfo, edges::CodeEdges, src, musteval) isempty(src.code) && return methodinfo stmt1 = first(src.code) if isexpr(stmt1, :gotoifnot) && isa(stmt1.args[1], Union{GlobalRef,Symbol}) - if any(chunk->hastrackedexpr(src, chunk), chunks) - push!(methodinfo.deps, stmt1.args[1]) + # This is basically a hack to look for symbols that control definition of methods via a conditional. + # It is aimed at solving #249, but this will have to be generalized for anything real. + for (stmt, me) in zip(src.code, musteval) + me || continue + if hastrackedexpr(stmt)[1] + push!(methodinfo.deps, stmt1.args[1]) + break + end end end # for (dep, lines) in be.byname @@ -516,18 +543,18 @@ function add_includes!(methodinfo::CodeTrackingMethodInfo, mod::Module, filename end # Eval and insert into CodeTracking data -function eval_with_signatures(mod, ex::Expr; define=true, kwargs...) +function eval_with_signatures(mod, ex::Expr; mode=:eval, kwargs...) methodinfo = CodeTrackingMethodInfo(ex) docexprs = DocExprs() - _, frame = methods_by_execution!(finish_and_return!, methodinfo, docexprs, mod, ex; define=define, kwargs...) + _, frame = methods_by_execution!(finish_and_return!, methodinfo, docexprs, mod, ex; mode=mode, kwargs...) return methodinfo.allsigs, methodinfo.deps, methodinfo.includes, frame end -function instantiate_sigs!(modexsigs::ModuleExprsSigs; define=false, kwargs...) +function instantiate_sigs!(modexsigs::ModuleExprsSigs; mode=:sigs, kwargs...) for (mod, exsigs) in modexsigs for rex in keys(exsigs) is_doc_expr(rex.ex) && continue - sigs, deps, _ = eval_with_signatures(mod, rex.ex; define=define, kwargs...) + sigs, deps, _ = eval_with_signatures(mod, rex.ex; mode=mode, kwargs...) exsigs[rex.ex] = sigs storedeps(deps, rex, mod) end @@ -901,7 +928,7 @@ end Load `filename` and track any future changes to it. `includet` is essentially shorthand for - Revise.track(Main, filename; define=true, skip_include=false) + Revise.track(Main, filename; mode=:eval, skip_include=false) `includet` is intended for "user scripts," e.g., a file you use locally for a specific purpose such as loading a specific data set or performing a particular analysis. @@ -925,7 +952,7 @@ function includet(mod::Module, file::AbstractString) tls = task_local_storage() tls[:SOURCE_PATH] = file try - track(mod, file; define=true, skip_include=false) + track(mod, file; mode=:eval, skip_include=false) finally if prev === nothing delete!(tls, :SOURCE_PATH) diff --git a/src/backedges.jl b/src/backedges.jl deleted file mode 100644 index 518d45e7..00000000 --- a/src/backedges.jl +++ /dev/null @@ -1,268 +0,0 @@ -using Core.Compiler: CodeInfo, NewvarNode, GotoNode -using Base.Meta: isexpr -using JuliaInterpreter: is_global_ref - -const SSAValues = Union{Core.Compiler.SSAValue, JuliaInterpreter.SSAValue} - -const structheads = VERSION >= v"1.5.0-DEV.702" ? () : (:struct_type, :abstract_type, :primitive_type) -const trackedheads = (:method, structheads...) - -isssa(stmt) = isa(stmt, Core.Compiler.SSAValue) | isa(stmt, JuliaInterpreter.SSAValue) -isslotnum(stmt) = isa(stmt, Core.Compiler.SlotNumber) | isa(stmt, JuliaInterpreter.SlotNumber) - -""" -`SlotDep` is an internal type used for holding information about dependencies of a SlotNumber -variable. If `sd` is a `SlotDep`, then -- `sd.lineassigned` is the statement number on which the SlotNumber was most recently assigned -- `sd.linedeps` is the set of previous SSAValues upon which this assigment depends (encoded as Int) -""" -mutable struct SlotDep - lineassigned::Int - linedeps::Vector{Int} # we use Int to handle Core.Compiler.SSAValue & JuliaInterpreter.SSAValue -end -function SlotDep(i::Int, stmt, slotdeps) - deps = add_deps!(Int[], stmt, slotdeps) - SlotDep(i, deps) -end -function add_deps!(linedeps, stmt, slotdeps) - if isssa(stmt) - push!(linedeps, stmt.id) - elseif isslotnum(stmt) - append!(linedeps, slotdeps[stmt.id].linedeps) - push!(linedeps, slotdeps[stmt.id].lineassigned) - elseif isa(stmt, Expr) - for a in stmt.args - if isssa(a) || isslotnum(a) || isa(a, Expr) - add_deps!(linedeps, a, slotdeps) - end - end - end - return linedeps -end - -# See docs below for `BackEdges(ci::CodeInfo)` -mutable struct Uses - ids::Vector{Int} # lines where it is used - defined::Vector{Int} # lines where it is defined -end -Uses() = Uses(Int[], Int[]) -Base.push!(uses::Uses, loc::Int) = push!(uses.ids, loc) - -struct BackEdges - byid::Vector{Vector{Int}} - byname::Dict{Union{GlobalRef,Symbol},Uses} -end -BackEdges(n::Integer) = BackEdges([Int[] for i = 1:n], Dict{Union{GlobalRef,Symbol},Uses}()) - -id_loc(pr::Pair) = pr.first, pr.second -id_loc(pr::Pair{<:SSAValues,Int}) = pr.first.id, pr.second -id_loc(pr::Pair{Bool,Int}) = pr.second - 1, pr.second # handles statements like `if true f() = 1 else f() = 2 end` -function Base.push!(be::BackEdges, pr::Pair{<:Union{Integer,SSAValues},Int}) - @noinline errorder(y, x) = throw(ArgumentError("SSA form requires that dependencies come after the statement, got $y and $x")) - - id, loc = id_loc(pr) - loc > id || errorder(loc, id) - push!(be.byid[id], loc) - return be -end -function Base.push!(be::BackEdges, pr::Pair{named,Int}) where named <: Union{Symbol,GlobalRef} - id, loc = pr - if isa(id, GlobalRef) - obj = getfield(id.mod, id.name) - isa(obj, Core.Builtin) && return be - end - ref = get(be.byname, id, nothing) - if ref === nothing - be.byname[id] = ref = Uses() - end - push!(ref, loc) - return be -end - -function add_to_backedges!(backedges::BackEdges, slotdeps, loc, stmt) - if isssa(stmt) - push!(backedges, stmt=>loc) - elseif isslotnum(stmt) - isassigned(slotdeps, stmt.id) || return backedges # issue #428 (it's inside an `if false...end`) - sd = slotdeps[stmt.id] - if sd.lineassigned != 0 - push!(backedges, sd.lineassigned=>loc) - end - for id in sd.linedeps - push!(backedges, id=>loc) - end - elseif stmt isa GlobalRef - push!(backedges, stmt=>loc) - elseif stmt isa Symbol - push!(backedges, stmt=>loc) - elseif stmt isa Expr - head = stmt.head - if head === :call && !(isssa(stmt.args[1]) || isslotnum(stmt.args[1])) - for a in Iterators.drop(stmt.args, 1) # don't track the callee - add_to_backedges!(backedges, slotdeps, loc, a) - end - elseif head ∈ trackedheads && (name = stmt.args[1]; isa(name, Symbol) | isa(name, GlobalRef)) - push!(backedges, name=>loc) - push!(backedges.byname[name].defined, loc) - for a in Iterators.drop(stmt.args, 1) - add_to_backedges!(backedges, slotdeps, loc, a) - end - elseif head === :(=) && (lhs = stmt.args[1]; isa(lhs, Symbol) || isa(lhs, GlobalRef)) - add_to_backedges!(backedges, slotdeps, loc, lhs) - push!(backedges.byname[lhs].defined, loc) - add_to_backedges!(backedges, slotdeps, loc, stmt.args[2]) - else - for a in stmt.args - add_to_backedges!(backedges, slotdeps, loc, a) - end - end - elseif stmt isa Pair - add_to_backedges!(backedges, slotdeps, loc, stmt.first) - add_to_backedges!(backedges, slotdeps, loc, stmt.second) - elseif stmt isa Tuple - for item in stmt - add_to_backedges!(backedges, slotdeps, loc, item) - end - # elseif isa(stmt, QuoteNode) || isa(stmt, NewvarNode) || isa(stmt, GotoNode) || - # isa(stmt, Real) || isa(stmt, CodeInfo) || isa(stmt, Nothing) || - # isa(stmt, Module) || isa(stmt, String) || isa(stmt, Char) || isa(stmt, Type) || - # isa(stmt, LineNumberNode) - # else - # error("unhandled stmt ", stmt, " of type ", typeof(stmt), " at ", loc) - end - return backedges -end - -function toplevel_blocks(bbs::Core.Compiler.CFG) - istoplevel = falses(length(bbs.blocks)) - next = 1 - for (i, block) in enumerate(bbs.blocks) - if i == 1 - istoplevel[i] = true - elseif i < next - else - istoplevel[i] = sum(block.preds .< i) == 2 - end - if istoplevel[i] && !isempty(block.succs) - next = maximum(block.succs) - end - end - return istoplevel -end - -function add_block_dependents!(backedges::BackEdges, bbs, istoplevel, i, bbidx) - for s in bbs.blocks[bbidx].succs - s > bbidx || continue # follow only in the forward direction - istoplevel[s] && continue - r = bbs.blocks[s].stmts - for j = Core.Compiler.first(r):Core.Compiler.last(r) - push!(backedges, i=>j) - end - add_block_dependents!(backedges, bbs, istoplevel, i, s) - end - return backedges -end - -""" - backedges = BackEdges(code::CodeInfo) - -Analyze `code` and determine the chain of dependencies. -`backedges.byid[i]` lists the succeeding lines that depend on `i`. -(In addition to SSAValue dependencies, this includes basic-block control flow -dependencies.) -`backedges.byname[sym]` lists the lines that depend on a particular symbol. -""" -function BackEdges(ci::CodeInfo) - ci.inferred && error("supply lowered but not inferred code") - bbs = Core.Compiler.compute_basic_blocks(ci.code) - istoplevel = toplevel_blocks(bbs) - codelocs = ci.codelocs - n = length(ci.code) - backedges = BackEdges(n) - slotdeps = Vector{SlotDep}(undef, length(ci.slotnames)) - slotassign = zeros(Int, length(ci.slotnames)) - i, n = 1, length(ci.code) - while i < n - stmt = ci.code[i] - if isa(stmt, Expr) - head = stmt.head - if head === :(=) && isslotnum(stmt.args[1]) - id = stmt.args[1].id - slotdeps[id] = SlotDep(i, stmt.args[2], slotdeps) - i += 1 - elseif head === :gotoifnot - dep, _ = stmt.args - add_to_backedges!(backedges, slotdeps, i, dep) - # Add the non-toplevel successor basic blocks as dependents of this line - bbidx = searchsortedlast(bbs.index, i) + 1 # bb index of this line - add_block_dependents!(backedges, bbs, istoplevel, i, bbidx) - i += 1 - elseif head === :thunk - ci_thunk = stmt.args[1] - if isa(ci_thunk, CodeInfo) - bethunk = BackEdges(ci_thunk) - for (name, uses) in bethunk.byname - add_to_backedges!(backedges, slotdeps, i, name) - if !isempty(uses.defined) - push!(backedges.byname[name].defined, i) - end - end - end - i += 1 - else - add_to_backedges!(backedges, slotdeps, i, stmt) - i += 1 - end - else - add_to_backedges!(backedges, slotdeps, i, stmt) - i += 1 - end - end - return backedges -end - -## Now that we can construct BackEdges, let's use them for analysis - -function toplevel_chunks(backedges::BackEdges) - be = [copy(usedby) for usedby in backedges.byid] - n = length(be) - for (name, uses) in backedges.byname - isempty(uses.defined) && continue - id = minimum(uses.ids) - append!(be[id], uses.ids) - end - chunkid = collect(1:n) - for i = n:-1:1 - if !isempty(be[i]) - chunkid[i] = maximum(chunkid[be[i]]) - end - end - chunks = UnitRange{Int}[] - i = 1 - while i <= n - push!(chunks, i:chunkid[i]) - i = chunkid[i]+1 - end - return chunks -end - -""" - hastrackedexpr(src, chunk=1:length(src.code)) - -Detect whether the specified `chunk` of `src` contains a definition tracked -by Revise. By default these are methods and type definitions (the latter because -they might contain constructors). -""" -function hastrackedexpr(code::CodeInfo, chunk::AbstractUnitRange=axes(code.code, 1); heads=trackedheads) - for stmtidx in chunk - stmt = code.code[stmtidx] - if isa(stmt, Expr) - stmt.head == :call && is_global_ref(stmt.args[1], Core, :_typebody!) && return true - stmt.head ∈ heads && return true - if stmt.head == :thunk - hastrackedexpr(stmt.args[1]; heads=heads) && return true - end - end - end - return false -end diff --git a/src/lowered.jl b/src/lowered.jl index a49a2156..6efc2fb1 100644 --- a/src/lowered.jl +++ b/src/lowered.jl @@ -19,66 +19,163 @@ function assign_this!(frame, value) end # This defines the API needed to store signatures using methods_by_execution! -# This default version is simple; there's a more involved one in the file Revise.jl -# that interacts with CodeTracking. +# This default version is simple and only used for testing purposes. +# The "real" one is CodeTrackingMethodInfo in Revise.jl. const MethodInfo = IdDict{Type,LineNumberNode} add_signature!(methodinfo::MethodInfo, @nospecialize(sig), ln) = push!(methodinfo, sig=>ln) push_expr!(methodinfo::MethodInfo, mod::Module, ex::Expr) = methodinfo pop_expr!(methodinfo::MethodInfo) = methodinfo -add_dependencies!(methodinfo::MethodInfo, be::BackEdges, src, chunks) = methodinfo +add_dependencies!(methodinfo::MethodInfo, be::CodeEdges, src, isrequired) = methodinfo add_includes!(methodinfo::MethodInfo, mod::Module, filename) = methodinfo -function minimal_evaluation!(methodinfo, frame) - src = frame.framecode.src - be = BackEdges(src) - chunks = toplevel_chunks(be) - musteval = falses(length(src.code)) - for chunk in chunks - if hastrackedexpr(frame.framecode.src, chunk) - musteval[chunk] .= true +# This is not generally used, see `is_method_or_eval` instead +function hastrackedexpr(stmt; heads=LoweredCodeUtils.trackedheads) + haseval = false + if isa(stmt, Expr) + haseval = matches_eval(stmt) + if stmt.head === :call + f = stmt.args[1] + callee_matches(f, Core, :_typebody!) && return true, haseval + callee_matches(f, Core, :_setsuper!) && return true, haseval + f === :include && return true, haseval + elseif stmt.head === :thunk + any(s->any(hastrackedexpr(s; heads=heads)), stmt.args[1].code) && return true, haseval + elseif stmt.head ∈ heads + return true, haseval end end - # Conservatively, we need to step in to each Core.eval in case the expression defines a method. - hadeval = false - for id in eachindex(src.code) - stmt = src.code[id] - me = false - if isa(stmt, Expr) - if stmt.head === :call - f = stmt.args[1] - me |= f === :include - me |= JuliaInterpreter.hasarg(isequal(:eval), stmt.args) + return false, haseval +end + +function matches_eval(stmt::Expr) + stmt.head === :call || return false + f = stmt.args[1] + return f === :eval || (callee_matches(f, Base, :getproperty) && is_quotenode(stmt.args[end], :eval)) +end + +function is_method_or_eval(stmt) + ismeth, haseval, isinclude = false, false, false + if isa(stmt, Expr) + haseval = matches_eval(stmt) + ismeth = stmt.head === :method + isinclude = stmt.head === :call && stmt.args[1] === :include + end + return ismeth | isinclude, haseval +end + +""" + isrequired, evalassign = minimal_evaluation!([predicate,] methodinfo, src::Core.CodeInfo, mode::Symbol) + +Mark required statements in `src`: `isrequired[i]` is `true` if `src.code[i]` should be evaluated. +Statements are analyzed by `isreq, haseval = predicate(stmt)`, and `predicate` defaults +to `Revise.is_method_or_eval`. +`haseval` is true if the statement came from `@eval` or `eval(...)` call. +Since the contents of such expression are difficult to analyze, it is generally +safest to execute all such evals. +""" +function minimal_evaluation!(predicate, methodinfo, src::Core.CodeInfo, mode::Symbol) + edges = CodeEdges(src) + # LoweredCodeUtils.print_with_code(stdout, src, edges) + isrequired = fill(false, length(src.code)) + evalassign = false + for (i, stmt) in enumerate(src.code) + if !isrequired[i] + isrequired[i], haseval = predicate(stmt) + if haseval # line `i` may be the equivalent of `f = Core.eval`, so... + isrequired[edges.succs[i]] .= true # ...require each stmt that calls `eval` via `f(expr)` + isrequired[i] = true end end - if me - chunkid = findfirst(chunk->id∈chunk, chunks) - musteval[chunks[chunkid]] .= true + if mode === :evalassign && isexpr(stmt, :(=)) + evalassign = true end end - add_dependencies!(methodinfo, be, src, chunks) - return musteval + # Check for docstrings + if length(src.code) > 1 + stmt = src.code[end-1] + if isexpr(stmt, :call) && (stmt::Expr).args[1] === Base.Docs.doc! + isrequired[end-1] = true + end + end + # All tracked expressions are marked. Now add their dependencies. + # LoweredCodeUtils.print_with_code(stdout, src, isrequired) + lines_required!(isrequired, src, edges; exclude_named_typedefs=mode===:sigs) + add_dependencies!(methodinfo, edges, src, isrequired) + return isrequired, evalassign end +minimal_evaluation!(predicate, methodinfo, frame::JuliaInterpreter.Frame, mode::Symbol) = + minimal_evaluation!(predicate, methodinfo, frame.framecode.src, mode) + +minimal_evaluation!(methodinfo, frame, mode::Symbol) = minimal_evaluation!(is_method_or_eval, methodinfo, frame, mode) + function methods_by_execution(mod::Module, ex::Expr; kwargs...) methodinfo = MethodInfo() docexprs = DocExprs() - value, frame = methods_by_execution!(finish_and_return!, methodinfo, docexprs, mod, ex; kwargs...) + value, frame = methods_by_execution!(JuliaInterpreter.Compiled(), methodinfo, docexprs, mod, ex; kwargs...) return methodinfo, docexprs, frame end -function methods_by_execution!(@nospecialize(recurse), methodinfo, docexprs, mod::Module, ex::Expr; always_rethrow=false, define=true, kwargs...) +""" + methods_by_execution!(recurse=JuliaInterpreter.Compiled(), methodinfo, docexprs, mod::Module, ex::Expr; + mode=:eval, disablebp=true, skip_include=mode!==:eval, always_rethrow=false) + +Evaluate or analyze `ex` in the context of `mod`. +Depending on the setting of `mode` (see the Extended help), it supports full evaluation or just the minimal +evaluation needed to extract method signatures. +`recurse` controls JuliaInterpreter's evaluation of any non-intercepted statement; +likely choices are `JuliaInterpreter.Compiled()` or `JuliaInterpreter.finish_and_return!`. +`methodinfo` is a cache for storing information about any method definitions (see [`CodeTrackingMethodInfo`](@ref)). +`docexprs` is a cache for storing documentation expressions; obtain an empty one with `Revise.DocExprs()`. + +# Extended help + +The action depends on `mode`: + +- `:eval` evaluates the expression in `mod`, similar to `Core.eval(mod, ex)` except that `methodinfo` and `docexprs` + will be populated with information about any signatures or docstrings. This mode is used to implement `includet`. +- `:sigs` analyzes `ex` and extracts signatures of methods and docstrings (specifically, statements flagged by + [`Revise.hastrackedexpr`](@ref)), but does not evaluate `ex` in the traditional sense. + It will selectively execute statements needed to form the signatures of defined methods. + It will also expand any `@eval`ed expressions, since these might contain method definitions. +- `:evalmeth` analyzes `ex` and extracts signatures and docstrings like `:sigs`, but takes the additional step of + evaluating any `:method` statements. +- `:evalassign` acts similarly to `:evalmeth`, and also evaluates assignment statements. + +When selectively evaluating an expression, Revise will incorporate required dependencies, even for +minimal-evaluation modes like `:sigs`. For example, the method definition + + max_values(T::Union{map(X -> Type{X}, Base.BitIntegerSmall_types)...}) = 1 << (8*sizeof(T)) + +found in `base/abstractset.jl` requires that it create the anonymous function in order to compute the +signature. + +The other keyword arguments are more straightforward: + +- `disablebp` controls whether JuliaInterpreter's breakpoints are disabled before stepping through the code. + They are restored on exit. +- `skip_include` prevents execution of `include` statements, instead inserting them into `methodinfo`'s + cache. This defaults to `true` unless `mode` is `:eval`. +- `always_rethrow`, if true, causes an error to be thrown if evaluating `ex` triggered an error. + If false, the error is logged with `@error`. `InterruptException`s are always rethrown. + This is primarily useful for debugging. +""" +function methods_by_execution!(@nospecialize(recurse), methodinfo, docexprs, mod::Module, ex::Expr; + mode::Symbol=:eval, disablebp::Bool=true, always_rethrow::Bool=false, kwargs...) lwr = Meta.lower(mod, ex) + # @show ex isa(lwr, Expr) || return nothing, nothing frame = prepare_thunk(mod, copy(lwr), true) frame === nothing && return nothing, nothing - define || LoweredCodeUtils.rename_framemethods!(recurse, frame) + mode===:eval || LoweredCodeUtils.rename_framemethods!(recurse, frame) # Determine whether we need interpreted mode - musteval = minimal_evaluation!(methodinfo, frame) - if !any(musteval) + isrequired, evalassign = minimal_evaluation!(methodinfo, frame, mode) + # LoweredCodeUtils.print_with_code(stdout, frame.framecode.src, isrequired) + if !any(isrequired) && (mode===:eval || !evalassign) # We can evaluate the entire expression in compiled mode - if define + if mode===:eval ret = try - Core.eval(mod, ex) # evaluate in compiled mode if we don't need to interpret + Core.eval(mod, ex) catch err (always_rethrow || isa(err, InterruptException)) && rethrow(err) loc = location_string(whereis(frame)...) @@ -90,28 +187,35 @@ function methods_by_execution!(@nospecialize(recurse), methodinfo, docexprs, mod end else # Use the interpreter - # We have to turn off all active breakpoints, https://github.com/timholy/CodeTracking.jl/issues/27 - bp_refs = JuliaInterpreter.breakpoints() - if eltype(bp_refs) !== JuliaInterpreter.BreakpointRef - bp_refs = JuliaInterpreter.BreakpointRef[] - foreach(bp -> append!(bp_refs, bp.instances), bp_refs) + local active_bp_refs + if disablebp + # We have to turn off all active breakpoints, https://github.com/timholy/CodeTracking.jl/issues/27 + bp_refs = JuliaInterpreter.breakpoints() + if eltype(bp_refs) !== JuliaInterpreter.BreakpointRef + bp_refs = JuliaInterpreter.BreakpointRef[] + foreach(bp -> append!(bp_refs, bp.instances), bp_refs) + end + active_bp_refs = filter(bp->bp[].isactive, bp_refs) + foreach(disable, active_bp_refs) end - active_bp_refs = filter(bp->bp[].isactive, bp_refs) - foreach(disable, active_bp_refs) ret = try - methods_by_execution!(recurse, methodinfo, docexprs, frame, musteval; define=define, kwargs...) + methods_by_execution!(recurse, methodinfo, docexprs, frame, isrequired; mode=mode, kwargs...) catch err - (always_rethrow || isa(err, InterruptException)) && rethrow(err) + (always_rethrow || isa(err, InterruptException)) && (disablebp && foreach(enable, active_bp_refs); rethrow(err)) loc = location_string(whereis(frame)...) @error "evaluation error starting at $loc" mod ex exception=(err, trim_toplevel!(catch_backtrace())) nothing end - foreach(enable, active_bp_refs) + if disablebp + foreach(enable, active_bp_refs) + end end return ret, lwr end +methods_by_execution!(methodinfo, docexprs, mod::Module, ex::Expr; kwargs...) = + methods_by_execution!(JuliaInterpreter.Compiled(), methodinfo, docexprs, mod, ex; kwargs...) -function methods_by_execution!(@nospecialize(recurse), methodinfo, docexprs, frame, musteval; define::Bool=true, skip_include::Bool=true) +function methods_by_execution!(@nospecialize(recurse), methodinfo, docexprs, frame, isrequired; mode::Symbol=:eval, skip_include::Bool=mode!==:eval) isok(lnn::LineTypes) = !iszero(lnn.line) || lnn.file !== :none # might fail either one, but accept anything mod = moduleof(frame) @@ -121,28 +225,29 @@ function methods_by_execution!(@nospecialize(recurse), methodinfo, docexprs, fra pc = frame.pc while true JuliaInterpreter.is_leaf(frame) || (@warn("not a leaf"); break) - if !musteval[pc] && !define + stmt = pc_expr(frame, pc) + if !isrequired[pc] && mode !== :eval && !(mode === :evalassign && isexpr(stmt, :(=))) pc = next_or_nothing!(frame) pc === nothing && break continue end - stmt = pc_expr(frame, pc) if isa(stmt, Expr) head = stmt.head if head ∈ structheads - if define - pc = step_expr!(recurse, frame, stmt, true) # This should check that they are unchanged + if mode !== :sigs + pc = step_expr!(recurse, frame, stmt, true) # This checks that they are unchanged else pc = next_or_nothing!(frame) end - elseif head === :thunk && isanonymous_typedef(stmt.args[1]) - # Anonymous functions should just be defined anew, since there does not seem to be a practical - # way to "find" them. They may be needed to define later signatures. - # Note that named inner methods don't require special treatment - pc = step_expr!(recurse, frame, stmt, true) + # elseif head === :thunk && isanonymous_typedef(stmt.args[1]) + # # Anonymous functions should just be defined anew, since there does not seem to be a practical + # # way to find them within the already-defined module. + # # They may be needed to define later signatures. + # # Note that named inner methods don't require special treatment. + # pc = step_expr!(recurse, frame, stmt, true) elseif head === :method empty!(signatures) - ret = methoddef!(recurse, signatures, frame, stmt, pc; define=define) + ret = methoddef!(recurse, signatures, frame, stmt, pc; define=mode!==:sigs) if ret === nothing # This was just `function foo end` or similar. # However, it might have been followed by a thunk that defined a @@ -239,22 +344,9 @@ function methods_by_execution!(@nospecialize(recurse), methodinfo, docexprs, fra end end end - elseif head === :(=) && isa(stmt.args[1], Symbol) - if define - pc = step_expr!(recurse, frame, stmt, true) - else - # FIXME: Code that initializes a global, performs some operations that - # depend on the value, and then mutates it will run into serious trouble here. - # sym = stmt.args[1] - # if isconst(mod, sym) - rhs = stmt.args[2] - val = isa(rhs, Expr) ? JuliaInterpreter.eval_rhs(recurse, frame, rhs) : @lookup(frame, rhs) - assign_this!(frame, val) - pc = next_or_nothing!(frame) - # else - # pc = step_expr!(recurse, frame, stmt, true) - # end - end + elseif head === :(=) + # If we're here, either isrequired[pc] is true, or the mode forces us to eval assignments + pc = step_expr!(recurse, frame, stmt, true) elseif head === :call f = @lookup(frame, stmt.args[1]) if f === Core.eval @@ -270,12 +362,8 @@ function methods_by_execution!(@nospecialize(recurse), methodinfo, docexprs, fra value = nothing for (newmod, newex) in thismodexs newex = unwrap(newex) - newframe = prepare_thunk(newmod, newex) - newframe === nothing && continue - define || LoweredCodeUtils.rename_framemethods!(recurse, newframe) - newmusteval = minimal_evaluation!(methodinfo, newframe) push_expr!(methodinfo, newmod, newex) - value = methods_by_execution!(recurse, methodinfo, docexprs, newframe, newmusteval; define=define) + value = methods_by_execution!(recurse, methodinfo, docexprs, newmod, newex; mode=mode, skip_include=skip_include, disablebp=false) pop_expr!(methodinfo) end assign_this!(frame, value) @@ -285,11 +373,17 @@ function methods_by_execution!(@nospecialize(recurse), methodinfo, docexprs, fra add_includes!(methodinfo, mod, @lookup(frame, stmt.args[2])) assign_this!(frame, nothing) # FIXME: the file might return something different from `nothing` pc = next_or_nothing!(frame) - elseif !define && f === Base.Docs.doc! + elseif mode !== :eval && f === Base.Docs.doc! fargs = JuliaInterpreter.collect_args(frame, stmt) popfirst!(fargs) length(fargs) == 3 && push!(fargs, Union{}) # add the default sig dmod::Module, b::Base.Docs.Binding, str::Base.Docs.DocStr, sig = fargs + if isdefined(b.mod, b.var) + tmpvar = getfield(b.mod, b.var) + if isa(tmpvar, Module) + dmod = tmpvar + end + end m = get!(Base.Docs.meta(dmod), b, Base.Docs.MultiDoc())::Base.Docs.MultiDoc if haskey(m.docs, sig) currentstr = m.docs[sig]::Base.Docs.DocStr @@ -320,5 +414,5 @@ function methods_by_execution!(@nospecialize(recurse), methodinfo, docexprs, fra end pc === nothing && break end - return musteval[frame.pc] ? get_return(frame) : nothing + return isrequired[frame.pc] ? get_return(frame) : nothing end diff --git a/src/pkgs.jl b/src/pkgs.jl index c4986e4d..46b7dd8b 100644 --- a/src/pkgs.jl +++ b/src/pkgs.jl @@ -299,7 +299,7 @@ function maybe_add_includes_to_pkgdata!(pkgdata::PkgData, file, includes) fullfile = joinpath(basedir(pkgdata), incrp) if isfile(fullfile) parse_source!(fi.modexsigs, fullfile, mod) - instantiate_sigs!(fi.modexsigs; define=true) + instantiate_sigs!(fi.modexsigs; mode=:eval) end # Add to watchlist init_watching(pkgdata, (incrp,)) diff --git a/src/precompile.jl b/src/precompile.jl index 2557c919..1f37ab31 100644 --- a/src/precompile.jl +++ b/src/precompile.jl @@ -16,19 +16,27 @@ function _precompile_() @assert precompile(Tuple{typeof(setindex!), Dict{String,WatchList}, WatchList, String}) MI = CodeTrackingMethodInfo - @assert precompile(Tuple{typeof(minimal_evaluation!), MI, Core.CodeInfo}) + @assert precompile(Tuple{typeof(minimal_evaluation!), MI, Core.CodeInfo, Symbol}) @assert precompile(Tuple{typeof(methods_by_execution!), Any, MI, DocExprs, Module, Expr}) @assert precompile(Tuple{typeof(methods_by_execution!), Any, MI, DocExprs, JuliaInterpreter.Frame, Vector{Bool}}) @assert precompile(Tuple{typeof(Core.kwfunc(methods_by_execution!)), - NamedTuple{(:skip_include,),Tuple{Bool}}, - typeof(methods_by_execution!), Function, MI, DocExprs, Module, Expr}) + NamedTuple{(:mode,),Tuple{Symbol}}, + typeof(methods_by_execution!), Function, MI, DocExprs, Module, Expr}) @assert precompile(Tuple{typeof(Core.kwfunc(methods_by_execution!)), - NamedTuple{(:define, :skip_include),Tuple{Bool,Bool}}, - typeof(methods_by_execution!), Function, MI, DocExprs, Module, Expr}) + NamedTuple{(:skip_include,),Tuple{Bool}}, + typeof(methods_by_execution!), Function, MI, DocExprs, Module, Expr}) @assert precompile(Tuple{typeof(Core.kwfunc(methods_by_execution!)), - NamedTuple{(:define, :skip_include),Tuple{Bool,Bool}}, - typeof(methods_by_execution!), Function, MI, DocExprs, JuliaInterpreter.Frame, Vector{Bool}}) + NamedTuple{(:mode, :skip_include),Tuple{Symbol,Bool}}, + typeof(methods_by_execution!), Function, MI, DocExprs, Module, Expr}) + @assert precompile(Tuple{typeof(Core.kwfunc(methods_by_execution!)), + NamedTuple{(:mode, :skip_include),Tuple{Symbol,Bool}}, + typeof(methods_by_execution!), Function, MI, DocExprs, JuliaInterpreter.Frame, Vector{Bool}}) + + m = which(methods_by_execution!, (Function, MI, DocExprs, Module, Expr)) + mbody = bodymethod(m) + @assert precompile(Tuple{mbody.sig.parameters[1], Symbol, Bool, Bool, Iterators.Pairs{Symbol,Bool,Tuple{Symbol},NamedTuple{(:skip_include,),Tuple{Bool}}}, typeof(methods_by_execution!), Any, MI, DocExprs, Module, Expr}) + @assert precompile(Tuple{typeof(hastrackedexpr), Expr}) @assert precompile(Tuple{typeof(get_def), Method}) @assert precompile(Tuple{typeof(parse_pkg_files), PkgId}) @assert precompile(Tuple{typeof(Base.stale_cachefile), String, String}) diff --git a/src/utils.jl b/src/utils.jl index b6d92e4b..fcb44c22 100644 --- a/src/utils.jl +++ b/src/utils.jl @@ -83,6 +83,8 @@ unwrap(rex::RelocatableExpr) = unwrap(rex.ex) istrivial(a) = a === nothing || isa(a, LineNumberNode) +isgoto(stmt) = isa(stmt, Core.GotoNode) | isexpr(stmt, :gotoifnot) + ## WatchList utilities function systime() # It's important to use the same clock used by the filesystem diff --git a/test/backedges.jl b/test/backedges.jl index 2682c9cf..a0934067 100644 --- a/test/backedges.jl +++ b/test/backedges.jl @@ -13,9 +13,9 @@ do_test("Backedges") && @testset "Backedges" begin # Find the inner struct def for the anonymous function idtype = findall(stmt->isexpr(stmt, :thunk) && isa(stmt.args[1], Core.CodeInfo), src.code)[end] src2 = src.code[idtype].args[1] - be = Revise.BackEdges(src2) - chunks = Revise.toplevel_chunks(be) - @test chunks[1] == 1:length(src2.code)-1 # skips the `return` at the end + methodinfo = Revise.MethodInfo() + isrequired = Revise.minimal_evaluation!(methodinfo, src, :sigs)[1] + @test sum(isrequired) == length(src.code)-(1 + (VERSION>=v"1.2")) # skips the `return` at the end src = """ # issue #249 diff --git a/test/runtests.jl b/test/runtests.jl index 18587867..5019d260 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -201,7 +201,7 @@ k(x) = 4 mexs = Revise.ModuleExprsSigs(ReviseTestPrivate) mexs[ReviseTestPrivate][ex] = nothing logs, _ = Test.collect_test_logs() do - Revise.instantiate_sigs!(mexs; define=true) + Revise.instantiate_sigs!(mexs; mode=:eval) end @test isempty(logs) @test isdefined(ReviseTestPrivate, :nolineinfo) @@ -936,7 +936,19 @@ end sleep(mtimedelay) @test ChangeDocstring.f() == 1 ds = @doc(ChangeDocstring.f) - @test ds.content[1].content[1].content[1].content[1] == "f" + @test get_docstring(ds) == "f" + # Ordinary route + open(joinpath(dn, "ChangeDocstring.jl"), "w") do io + println(io, """ + module ChangeDocstring + "h" f() = 1 + end + """) + end + yry() + ds = @doc(ChangeDocstring.f) + @test get_docstring(ds) == "h" + # Now manually change the docstring ex = quote "g" f() = 1 end lwr = Meta.lower(ChangeDocstring, ex) @@ -944,9 +956,9 @@ end methodinfo = Revise.MethodInfo() docexprs = Revise.DocExprs() ret = Revise.methods_by_execution!(JuliaInterpreter.finish_and_return!, methodinfo, - docexprs, frame, trues(length(frame.framecode.src.code)); define=false) + docexprs, frame, trues(length(frame.framecode.src.code)); mode=:sigs) ds = @doc(ChangeDocstring.f) - @test ds.content[1].content[1].content[1].content[1] == "g" + @test get_docstring(ds) == "g" rm_precompile("ChangeDocstring") pop!(LOAD_PATH) @@ -1856,7 +1868,7 @@ end @test occursin("Test301.jl:10", logs[1].message) logs, _ = Test.collect_test_logs() do - Revise.track("callee_error.jl"; define=true) + Revise.track("callee_error.jl"; mode=:eval) end @test length(logs) == 2 @test occursin("(compiled mode) evaluation error", logs[1].message) @@ -1937,36 +1949,15 @@ end @test RevisionInterrupt.f(0) == 2 # Compiled mode - open(fn, "w") do io - println(io, """ - module RevisionInterrupt - throw(InterruptException()) - f(x) = 3 - end - """) - end - logs, _ = Test.collect_test_logs() do - yry() - end - check_revision_interrupt(logs) - # @test RevisionInterrupt.f(0) == 2 - logs, _ = Test.collect_test_logs() do - yry() - end - check_revision_interrupt(logs) - # @test RevisionInterrupt.f(0) == 2 - open(fn, "w") do io - println(io, """ - module RevisionInterrupt - f(x) = 3 - end - """) - end + # Due to selective evaluation we can't check compiled mode via high-level code: it will discover + # the method and run this expression in the interpreter. + # This requires low-level intervention + @test_throws InterruptException Revise.methods_by_execution!(Revise.MethodInfo(), Revise.DocExprs(), Main, :(throw(InterruptException())); mode=:eval) logs, _ = Test.collect_test_logs() do - yry() + Revise.methods_by_execution!(Revise.MethodInfo(), Revise.DocExprs(), Main, :(throw(ArgumentError("AA"))); mode=:eval) end - @test isempty(logs) - @test RevisionInterrupt.f(0) == 3 + @test length(logs) == 1 + @test startswith(logs[1].message, "(compiled mode) evaluation error starting at") end do_test("get_def") && @testset "get_def" begin @@ -2111,6 +2102,9 @@ end sleep(mtimedelay) lines = readlines(logfile) @test length(lines) == 1 && chomp(lines[1]) == "executed" + # In older versions of Revise, it would do the work again when the file + # changed. Starting with 3.0, Revise modifies methods and docstrings but + # does not "do work." open(srcfile, "w") do io print(io, """ println("executed again") @@ -2122,7 +2116,7 @@ end end end lines = readlines(logfile) - @test length(lines) == 1 && chomp(lines[1]) == "executed again" + @test isempty(lines) # tls path (issue #264) srcdir = joinpath(tempdir(), randtmp()) diff --git a/test/sigtest.jl b/test/sigtest.jl index c17c9141..5653212c 100644 --- a/test/sigtest.jl +++ b/test/sigtest.jl @@ -91,7 +91,8 @@ end basefiles = Set{String}() @time for (i, (mod, file)) in enumerate(Base._included_files) - (isdefinedmod(mod) && mod != Base.__toplevel__) || continue + # (isdefinedmod(mod) && mod != Base.__toplevel__) || continue + endswith(file, "FileWatching.jl") && continue endswith(file, "sysimg.jl") && continue file = Revise.fixpath(file) push!(basefiles, reljpath(file))