Skip to content

Commit

Permalink
REPL: Expand macros before looking for using statements (#53821)
Browse files Browse the repository at this point in the history
Currently, in order to give the nice prompt for missing packages, we
look for any `using`/`import` statements in the AST before evaluation.
However, this misses any `using` statements introduced by macros:

```
julia> using Pkg

julia> using BenchmarkTools
 │ Package BenchmarkTools not found, but a package named BenchmarkTools is
 │ available from a registry.
 │ Install package?
 │   (@v1.11) pkg> add BenchmarkTools
 └ (y/n/o) [y]: n
ERROR: ArgumentError: Package BenchmarkTools not found in current path.
- Run `import Pkg; Pkg.add("BenchmarkTools")` to install the BenchmarkTools package.
Stacktrace:
 [1] macro expansion
   @ Base ./loading.jl:1781 [inlined]
 [2] macro expansion
   @ Base ./lock.jl:267 [inlined]
 [3] __require(into::Module, mod::Symbol)
   @ Base ./loading.jl:1762
 [4] #invoke_in_world#3
   @ Base ./essentials.jl:963 [inlined]
 [5] invoke_in_world
   @ Base ./essentials.jl:960 [inlined]
 [6] require(into::Module, mod::Symbol)
   @ Base ./loading.jl:1755

julia> macro foo()
       :(using BenchmarkTools)
       end
@foo (macro with 1 method)

julia> @foo
ERROR: ArgumentError: Package BenchmarkTools not found in current path.
- Run `import Pkg; Pkg.add("BenchmarkTools")` to install the BenchmarkTools package.
Stacktrace:
 [1] macro expansion
   @ Base ./loading.jl:1781 [inlined]
 [2] macro expansion
   @ Base ./lock.jl:267 [inlined]
 [3] __require(into::Module, mod::Symbol)
   @ Base ./loading.jl:1762
 [4] #invoke_in_world#3
   @ Base ./essentials.jl:963 [inlined]
 [5] invoke_in_world
   @ Base ./essentials.jl:960 [inlined]
 [6] require(into::Module, mod::Symbol)
   @ Base ./loading.jl:1755
 [7] top-level scope
   @ REPL[4]:1
```

Generally, it doesn't matter, but embedded DSLs may want to do this kind
of thing, so we might as well try to support it.
  • Loading branch information
Keno authored Mar 23, 2024
1 parent 6172020 commit 63e365f
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 9 deletions.
2 changes: 1 addition & 1 deletion base/client.jl
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ function scrub_repl_backtrace(bt)
if bt !== nothing && !(bt isa Vector{Any}) # ignore our sentinel value types
bt = bt isa Vector{StackFrame} ? copy(bt) : stacktrace(bt)
# remove REPL-related frames from interactive printing
eval_ind = findlast(frame -> !frame.from_c && frame.func === :eval, bt)
eval_ind = findlast(frame -> !frame.from_c && startswith(String(frame.func), "__repl_entry"), bt)
eval_ind === nothing || deleteat!(bt, eval_ind:length(bt))
end
return bt
Expand Down
46 changes: 39 additions & 7 deletions stdlib/REPL/src/REPL.jl
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,27 @@ const repl_ast_transforms = Any[softscope] # defaults for new REPL backends
# to e.g. install packages on demand
const install_packages_hooks = Any[]

# N.B.: Any functions starting with __repl_entry cut off backtraces when printing in the REPL.
# We need to do this for both the actual eval and macroexpand, since the latter can cause custom macro
# code to run (and error).
__repl_entry_lower_with_loc(mod::Module, @nospecialize(ast), toplevel_file::Ref{Ptr{UInt8}}, toplevel_line::Ref{Cint}) =
ccall(:jl_expand_with_loc, Any, (Any, Any, Ptr{UInt8}, Cint), ast, mod, toplevel_file[], toplevel_line[])
__repl_entry_eval_expanded_with_loc(mod::Module, @nospecialize(ast), toplevel_file::Ref{Ptr{UInt8}}, toplevel_line::Ref{Cint}) =
ccall(:jl_toplevel_eval_flex, Any, (Any, Any, Cint, Cint, Ptr{Ptr{UInt8}}, Ptr{Cint}), mod, ast, 1, 1, toplevel_file, toplevel_line)

function toplevel_eval_with_hooks(mod::Module, @nospecialize(ast), toplevel_file=Ref{Ptr{UInt8}}(Base.unsafe_convert(Ptr{UInt8}, :REPL)), toplevel_line=Ref{Cint}(1))
if !isexpr(ast, :toplevel)
ast = __repl_entry_lower_with_loc(mod, ast, toplevel_file, toplevel_line)
check_for_missing_packages_and_run_hooks(ast)
return __repl_entry_eval_expanded_with_loc(mod, ast, toplevel_file, toplevel_line)
end
local value=nothing
for i = 1:length(ast.args)
value = toplevel_eval_with_hooks(mod, ast.args[i], toplevel_file, toplevel_line)
end
return value
end

function eval_user_input(@nospecialize(ast), backend::REPLBackend, mod::Module)
lasterr = nothing
Base.sigatomic_begin()
Expand All @@ -222,11 +243,10 @@ function eval_user_input(@nospecialize(ast), backend::REPLBackend, mod::Module)
put!(backend.response_channel, Pair{Any, Bool}(lasterr, true))
else
backend.in_eval = true
check_for_missing_packages_and_run_hooks(ast)
for xf in backend.ast_transforms
ast = Base.invokelatest(xf, ast)
end
value = Core.eval(mod, ast)
value = toplevel_eval_with_hooks(mod, ast)
backend.in_eval = false
setglobal!(Base.MainInclude, :ans, value)
put!(backend.response_channel, Pair{Any, Bool}(value, false))
Expand Down Expand Up @@ -256,7 +276,7 @@ function check_for_missing_packages_and_run_hooks(ast)
end
end

function modules_to_be_loaded(ast::Expr, mods::Vector{Symbol} = Symbol[])
function _modules_to_be_loaded!(ast::Expr, mods::Vector{Symbol})
ast.head === :quote && return mods # don't search if it's not going to be run during this eval
if ast.head === :using || ast.head === :import
for arg in ast.args
Expand All @@ -271,12 +291,24 @@ function modules_to_be_loaded(ast::Expr, mods::Vector{Symbol} = Symbol[])
end
end
end
for arg in ast.args
if isexpr(arg, (:block, :if, :using, :import))
modules_to_be_loaded(arg, mods)
if ast.head !== :thunk
for arg in ast.args
if isexpr(arg, (:block, :if, :using, :import))
_modules_to_be_loaded!(arg, mods)
end
end
else
code = ast.args[1]
for arg in code.code
isa(arg, Expr) || continue
_modules_to_be_loaded!(arg, mods)
end
end
filter!(mod -> !in(String(mod), ["Base", "Main", "Core"]), mods) # Exclude special non-package modules
end

function modules_to_be_loaded(ast::Expr, mods::Vector{Symbol} = Symbol[])
_modules_to_be_loaded!(ast, mods)
filter!(mod::Symbol -> !in(mod, (:Base, :Main, :Core)), mods) # Exclude special non-package modules
return unique(mods)
end

Expand Down
29 changes: 29 additions & 0 deletions stdlib/REPL/test/repl.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1480,6 +1480,35 @@ end
end
end

# Test that the REPL can find `using` statements inside macro expansions
global packages_requested = Any[]
old_hooks = copy(REPL.install_packages_hooks)
empty!(REPL.install_packages_hooks)
push!(REPL.install_packages_hooks, function(pkgs)
append!(packages_requested, pkgs)
end)

fake_repl() do stdin_write, stdout_read, repl
repltask = @async begin
REPL.run_repl(repl)
end

# Just consume all the output - we only test that the callback ran
read_resp_task = @async while !eof(stdout_read)
readavailable(stdout_read)
end

write(stdin_write, "macro usingfoo(); :(using FooNotFound); end\n")
write(stdin_write, "@usingfoo\n")
write(stdin_write, "\x4")
Base.wait(repltask)
close(stdin_write)
close(stdout_read)
Base.wait(read_resp_task)
end
@test packages_requested == Any[:FooNotFound]
empty!(REPL.install_packages_hooks); append!(REPL.install_packages_hooks, old_hooks)

# err should reprint error if deeper than top-level
fake_repl() do stdin_write, stdout_read, repl
repltask = @async begin
Expand Down
2 changes: 1 addition & 1 deletion test/precompile.jl
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,7 @@ precompile_test_harness(false) do dir
error("break me")
end
""")
@test_warn r"LoadError: break me\nStacktrace:\n \[1\] [\e01m\[]*error" try
@test_warn r"LoadError: break me\nStacktrace:\n[ ]*\[1\] [\e01m\[]*error" try
Base.require(Main, :FooBar2)
error("the \"break me\" test failed")
catch exc
Expand Down

0 comments on commit 63e365f

Please sign in to comment.