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

Track GlobalRef consistently #822

Merged
merged 7 commits into from
Jul 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ jobs:
- '1.6' # LTS
- '1.9' # parsing errors branch on 1.10, so test the last pre-1.10 version
- '1' # current stable
- 'pre' # next release, if available
os:
- ubuntu-latest
- macOS-latest
Expand Down
4 changes: 2 additions & 2 deletions Project.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name = "Revise"
uuid = "295af30f-e4ad-537b-8983-00126c2a3abe"
version = "3.5.15"
version = "3.5.16"

[deps]
CodeTracking = "da1fd8a2-8d9e-5ec2-8556-3022fb5608a2"
Expand All @@ -19,7 +19,7 @@ Unicode = "4ec0a83e-493e-50e2-b9ac-8f72acf5a8f5"
[compat]
CodeTracking = "1.2"
JuliaInterpreter = "0.9"
LoweredCodeUtils = "2.3"
LoweredCodeUtils = "3"
OrderedCollections = "1"
# Exclude Requires-1.1.0 - see https://github.com/JuliaPackaging/Requires.jl/issues/94
Requires = "~1.0, ^1.1.1"
Expand Down
39 changes: 24 additions & 15 deletions src/lowered.jl
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,10 @@
safest to execute all such evals.
"""
function minimal_evaluation!(@nospecialize(predicate), methodinfo, mod::Module, src::Core.CodeInfo, mode::Symbol)
edges = CodeEdges(src)
edges = CodeEdges(mod, src)
# LoweredCodeUtils.print_with_code(stdout, src, edges)
isrequired = fill(false, length(src.code))
namedconstassigned = Dict{Symbol,Bool}()
namedconstassigned = Dict{GlobalRef,Bool}()
evalassign = false
for (i, stmt) in enumerate(src.code)
if !isrequired[i]
Expand All @@ -99,18 +99,24 @@
end
end
if isexpr(stmt, :const)
name = stmt.args[1]::Symbol
namedconstassigned[name] = false
elseif isexpr(stmt, :(=))
name = stmt.args[1]
if isa(name, Symbol)
name = GlobalRef(mod, name)
end
namedconstassigned[name::GlobalRef] = false
elseif LoweredCodeUtils.is_assignment_like(stmt)
lhs = (stmt::Expr).args[1]
if isa(lhs, Symbol)
lhs = GlobalRef(mod, lhs)
end
if isa(lhs, GlobalRef)
if haskey(namedconstassigned, lhs)
namedconstassigned[lhs] = true
end
end
if mode === :evalassign
evalassign = isrequired[i] = true
if isa(lhs, Symbol)
if isa(lhs, GlobalRef)

Check warning on line 119 in src/lowered.jl

View check run for this annotation

Codecov / codecov/patch

src/lowered.jl#L119

Added line #L119 was not covered by tests
isrequired[edges.byname[lhs].succs] .= true # mark any `const` statements or other "uses" in this block
end
end
Expand All @@ -119,7 +125,7 @@
if mode === :sigs
for (name, isassigned) in namedconstassigned
isassigned || continue
if isdefined(mod, name)
if isdefined(name.mod, name.name)
empty!(edges.byname[name].succs) # avoid redefining `consts` in `:sigs` mode (fixes #789)
end
end
Expand Down Expand Up @@ -225,7 +231,7 @@
Core.eval(mod, ex)
catch err
(always_rethrow || isa(err, InterruptException)) && rethrow(err)
loc = location_string(whereis(frame)...)
loc = location_string(whereis(frame))

Check warning on line 234 in src/lowered.jl

View check run for this annotation

Codecov / codecov/patch

src/lowered.jl#L234

Added line #L234 was not covered by tests
bt = trim_toplevel!(catch_backtrace())
throw(ReviseEvalException(loc, err, Any[(sf, 1) for sf in stacktrace(bt)]))
end
Expand All @@ -248,7 +254,7 @@
methods_by_execution!(recurse, methodinfo, docexprs, frame, isrequired; mode=mode, kwargs...)
catch err
(always_rethrow || isa(err, InterruptException)) && (disablebp && foreach(enable, active_bp_refs); rethrow(err))
loc = location_string(whereis(frame)...)
loc = location_string(whereis(frame))
sfs = [] # crafted for interaction with Base.show_backtrace
frame = JuliaInterpreter.leaf(frame)
while frame !== nothing
Expand Down Expand Up @@ -277,7 +283,7 @@
while true
JuliaInterpreter.is_leaf(frame) || (@warn("not a leaf"); break)
stmt = pc_expr(frame, pc)
if !isrequired[pc] && mode !== :eval && !(mode === :evalassign && isexpr(stmt, :(=)))
if !isrequired[pc] && mode !== :eval && !(mode === :evalassign && LoweredCodeUtils.is_assignment_like(stmt))
pc = next_or_nothing!(frame)
pc === nothing && break
continue
Expand Down Expand Up @@ -309,10 +315,13 @@
# However, it might have been followed by a thunk that defined a
# method (issue #435), so we still need to check for additions.
if !isempty(signatures)
file, line = whereis(frame.framecode, pc)
lnn = LineNumberNode(Int(line), Symbol(file))
for sig in signatures
add_signature!(methodinfo, sig, lnn)
loc = whereis(frame.framecode, pc)
if loc !== nothing
file, line = loc
lnn = LineNumberNode(Int(line), Symbol(file))
for sig in signatures
add_signature!(methodinfo, sig, lnn)
end

Check warning on line 324 in src/lowered.jl

View check run for this annotation

Codecov / codecov/patch

src/lowered.jl#L318-L324

Added lines #L318 - L324 were not covered by tests
end
end
pc = next_or_nothing!(frame)
Expand Down Expand Up @@ -400,7 +409,7 @@
end
end
end
elseif head === :(=)
elseif LoweredCodeUtils.is_assignment_like(stmt)
# 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
Expand Down
2 changes: 1 addition & 1 deletion src/parsing.jl
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ function process_source!(mod_exprs_sigs::ModuleExprsSigs, ex, filename, mod::Mod
catch err
bt = trim_toplevel!(catch_backtrace())
lnn = firstline(ex)
loc = location_string(lnn.file, lnn.line)
loc = location_string((lnn.file, lnn.line))
throw(ReviseEvalException(loc, err, Any[(sf, 1) for sf in stacktrace(bt)]))
end
end
Expand Down
5 changes: 3 additions & 2 deletions src/utils.jl
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,9 @@

newloc(methloc::LineNumberNode, ln, lno) = fixpath(ln)

location_string(file::AbstractString, line) = abspath(file)*':'*string(line)
location_string(file::Symbol, line) = location_string(string(file), line)
location_string((file, line)::Tuple{AbstractString, Any},) = abspath(file)*':'*string(line)
location_string((file, line)::Tuple{Symbol, Any},) = location_string((string(file), line))
location_string(::Nothing) = "unknown location"

Check warning on line 64 in src/utils.jl

View check run for this annotation

Codecov / codecov/patch

src/utils.jl#L64

Added line #L64 was not covered by tests

function linediff(la::LineNumberNode, lb::LineNumberNode)
(isa(la.file, Symbol) && isa(lb.file, Symbol) && (la.file::Symbol === lb.file::Symbol)) || return typemax(Int)
Expand Down
Loading