Skip to content

Commit

Permalink
REPL: Expand macros before looking for using statements
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 committed Mar 22, 2024
1 parent d68a04e commit 4d28808
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 8 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
48 changes: 41 additions & 7 deletions stdlib/REPL/src/REPL.jl
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,29 @@ 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)
if !isempty(install_packages_hooks)
check_for_missing_packages_and_run_hooks(ast)
end
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 +245,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 +278,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 +293,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

0 comments on commit 4d28808

Please sign in to comment.