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

REPL: Expand macros before looking for using statements #53821

Merged
merged 2 commits into from
Mar 23, 2024
Merged

Conversation

Keno
Copy link
Member

@Keno Keno commented Mar 22, 2024

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.

@Keno Keno force-pushed the kf/lateusingprompt branch from 4d28808 to fa764a5 Compare March 22, 2024 23:52
@IanButterworth
Copy link
Member

SGTM. Side note, the using Pkg shouldn't be needed anymore. That was fixed

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.
@Keno Keno force-pushed the kf/lateusingprompt branch from fa764a5 to 82a7f9f Compare March 23, 2024 00:07
@Keno
Copy link
Member Author

Keno commented Mar 23, 2024

Fair point, the version I was developing this on didn't have that fix, and I accidentally broke it here, so fixed that too.

@Keno Keno merged commit 63e365f into master Mar 23, 2024
7 checks passed
@Keno Keno deleted the kf/lateusingprompt branch March 23, 2024 05:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants