From d21c32f397d115dd037b2d02f7fb7e24283d9286 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Thu, 5 Dec 2024 14:14:00 -0700 Subject: [PATCH] Look through SSAValue to find `include` and similar (#869) There's no guarantee that all include calls will be literal GlobalRefs and in fact https://github.com/JuliaLang/julia/pull/56746 will likely make them never GlobalRef. --- src/lowered.jl | 26 +++++++++++++++++++------- src/packagedef.jl | 26 +++++++++++++++++--------- test/backedges.jl | 5 ++++- 3 files changed, 40 insertions(+), 17 deletions(-) diff --git a/src/lowered.jl b/src/lowered.jl index 3e5ccc88..6939d8bc 100644 --- a/src/lowered.jl +++ b/src/lowered.jl @@ -24,6 +24,7 @@ add_dependencies!(methodinfo::MethodInfo, be::CodeEdges, src, isrequired) = meth add_includes!(methodinfo::MethodInfo, mod::Module, filename) = methodinfo function is_some_include(@nospecialize(f)) + @assert !isa(f, Core.SSAValue) && !isa(f, JuliaInterpreter.SSAValue) if isa(f, GlobalRef) return f.name === :include elseif isa(f, Symbol) @@ -44,17 +45,21 @@ function is_some_include(@nospecialize(f)) end # This is not generally used, see `is_method_or_eval` instead -function hastrackedexpr(stmt; heads=LoweredCodeUtils.trackedheads) +function hastrackedexpr(stmt, code; heads=LoweredCodeUtils.trackedheads) haseval = false if isa(stmt, Expr) haseval = matches_eval(stmt) if stmt.head === :call f = stmt.args[1] + while isa(f, Core.SSAValue) || isa(f, JuliaInterpreter.SSAValue) + f = code[f.id] + end callee_matches(f, Core, :_typebody!) && return true, haseval callee_matches(f, Core, :_setsuper!) && return true, haseval is_some_include(f) && return true, haseval elseif stmt.head === :thunk - any(s->any(hastrackedexpr(s; heads=heads)), (stmt.args[1]::Core.CodeInfo).code) && return true, haseval + newcode = (stmt.args[1]::Core.CodeInfo).code + any(s->any(hastrackedexpr(s, newcode; heads=heads)), newcode) && return true, haseval elseif stmt.head ∈ heads return true, haseval end @@ -70,14 +75,21 @@ function matches_eval(stmt::Expr) (isa(f, GlobalRef) && f.name === :eval) || is_quotenode_egal(f, Core.eval) end -function categorize_stmt(@nospecialize(stmt)) +function categorize_stmt(@nospecialize(stmt), code::Vector{Any}) ismeth, haseval, isinclude, isnamespace, istoplevel = false, false, false, false, false if isa(stmt, Expr) haseval = matches_eval(stmt) ismeth = stmt.head === :method || (stmt.head === :thunk && defines_function(only(stmt.args))) istoplevel = stmt.head === :toplevel isnamespace = stmt.head === :export || stmt.head === :import || stmt.head === :using - isinclude = stmt.head === :call && is_some_include(stmt.args[1]) + isinclude = false + if stmt.head === :call && length(stmt.args) >= 1 + callee = stmt.args[1] + while isa(callee, Core.SSAValue) || isa(callee, JuliaInterpreter.SSAValue) + callee = code[callee.id] + end + isinclude = is_some_include(callee) + end end return ismeth, haseval, isinclude, isnamespace, istoplevel end @@ -112,7 +124,7 @@ function minimal_evaluation!(@nospecialize(predicate), methodinfo, mod::Module, evalassign = false for (i, stmt) in enumerate(src.code) if !isrequired[i] - isrequired[i], haseval = predicate(stmt)::Tuple{Bool,Bool} + isrequired[i], haseval = predicate(stmt, src.code)::Tuple{Bool,Bool} 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 @@ -169,8 +181,8 @@ end minimal_evaluation!(predicate, methodinfo, moduleof(frame), frame.framecode.src, mode) function minimal_evaluation!(methodinfo, frame::JuliaInterpreter.Frame, mode::Symbol) - minimal_evaluation!(methodinfo, frame, mode) do @nospecialize(stmt) - ismeth, haseval, isinclude, isnamespace, istoplevel = categorize_stmt(stmt) + minimal_evaluation!(methodinfo, frame, mode) do @nospecialize(stmt), code + ismeth, haseval, isinclude, isnamespace, istoplevel = categorize_stmt(stmt, code) isreq = ismeth | isinclude | istoplevel return mode === :sigs ? (isreq, haseval) : (isreq | isnamespace, haseval) end diff --git a/src/packagedef.jl b/src/packagedef.jl index 41403425..ca8cf7a7 100644 --- a/src/packagedef.jl +++ b/src/packagedef.jl @@ -447,15 +447,23 @@ push_expr!(methodinfo::CodeTrackingMethodInfo, mod::Module, ex::Expr) = (push!(m pop_expr!(methodinfo::CodeTrackingMethodInfo) = (pop!(methodinfo.exprstack); methodinfo) function add_dependencies!(methodinfo::CodeTrackingMethodInfo, edges::CodeEdges, src, musteval) isempty(src.code) && return methodinfo - stmt1 = first(src.code) - if isa(stmt1, Core.GotoIfNot) && (dep = stmt1.cond; isa(dep, Union{GlobalRef,Symbol})) - # 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, dep) - break + for i = 1:length(src.code) + stmt = src.code[i] + if isa(stmt, Core.GotoIfNot) + dep = stmt.cond + while (isa(dep, Core.SSAValue) || isa(dep, JuliaInterpreter.SSAValue)) + dep = src.code[dep.id] + end + if isa(dep, Union{GlobalRef,Symbol}) + # 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, src.code)[1] + push!(methodinfo.deps, dep) + break + end + end end end end diff --git a/test/backedges.jl b/test/backedges.jl index 8cd82b9e..14022932 100644 --- a/test/backedges.jl +++ b/test/backedges.jl @@ -17,7 +17,10 @@ do_test("Backedges") && @testset "Backedges" begin src2 = src.code[idtype].args[1] methodinfo = Revise.MethodInfo() isrequired = Revise.minimal_evaluation!(methodinfo, frame, :sigs)[1] - @test sum(isrequired) == length(src.code)-count(e->isexpr(e, :latestworld), src.code)-1 # skips the `return` at the end + laststmt = src.code[end] + @assert isa(laststmt, Core.ReturnNode) + to_skip = isa(laststmt.val, Revise.JuliaInterpreter.SSAValue) ? 2 : 1 + @test sum(isrequired) == length(src.code)-count(e->isexpr(e, :latestworld), src.code)-to_skip # skips the `return` at the end (and its argument) src = """ # issue #249