Skip to content

Commit

Permalink
Re-impose "no double lookup" for invoke (#540)
Browse files Browse the repository at this point in the history
* Re-impose "no double lookup" for invoke

This was added in #442 and inadvertently removed in #532.
However, the test in #442 was not stringent enough; the test added here
comes from #535.

Fixes #535
Fixes timholy/Revise.jl#695
  • Loading branch information
timholy authored Jul 25, 2022
1 parent 75bc4fb commit 53ca034
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 7 deletions.
8 changes: 5 additions & 3 deletions bin/generate_builtins.jl
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const RECENTLY_ADDED = Core.Builtin[
Core.get_binding_type, Core.set_binding_type!,
Core.getglobal, Core.setglobal!,
Core.modifyfield!, Core.replacefield!, Core.swapfield!,
]
]
const kwinvoke = Core.kwfunc(Core.invoke)

function scopedname(f)
Expand Down Expand Up @@ -175,11 +175,13 @@ function maybe_evaluate_builtin(frame, call_expr, expand::Bool)
print(io,
"""
$head f === $fstr
argswrapped = getargs(args, frame)
if !expand
argswrapped = getargs(args, frame)
return Some{Any}($fstr(argswrapped...))
end
return Expr(:call, $fstr, argswrapped...)
# This uses the original arguments to avoid looking them up twice
# See #442
return Expr(:call, invoke, args[2:end]...)
""")
continue
elseif f === Core._call_latest
Expand Down
6 changes: 4 additions & 2 deletions src/builtins.jl
Original file line number Diff line number Diff line change
Expand Up @@ -191,11 +191,13 @@ function maybe_evaluate_builtin(frame, call_expr, expand::Bool)
return Some{Any}(getglobal(getargs(args, frame)...))
end
elseif f === invoke
argswrapped = getargs(args, frame)
if !expand
argswrapped = getargs(args, frame)
return Some{Any}(invoke(argswrapped...))
end
return Expr(:call, invoke, argswrapped...)
# This uses the original arguments to avoid looking them up twice
# See #442
return Expr(:call, invoke, args[2:end]...)
elseif f === isa
if nargs == 2
return Some{Any}(isa(@lookup(frame, args[2]), @lookup(frame, args[3])))
Expand Down
7 changes: 5 additions & 2 deletions test/interpret.jl
Original file line number Diff line number Diff line change
Expand Up @@ -727,9 +727,12 @@ end
D = Diagonal([1.0, 2.0])
@test @interpret(f(D)) === f(D)

# issue #441
# issue #441 & #535
flog() = @info "logging macros"
@test @interpret flog() === nothing
@test_logs (:info, "logging macros") @test @interpret flog() === nothing
flog2() = @error "this error is ok"
frame = JuliaInterpreter.enter_call(flog2)
@test_logs (:error, "this error is ok") @test debug_command(frame, :c) === nothing
end

struct A396
Expand Down

0 comments on commit 53ca034

Please sign in to comment.