Skip to content

Commit 7c38999

Browse files
committed
Use a curried helper for module-local eval/include
In #55466, the automatically added `include` method for non-bare modules was adjusted to conform the signature to the version of those methods in Main (defined in sysimg.jl, since `Main` is technically a bare module). Unfortunately, this broke some downstream packages which overload Base.include with additional functionality and got broken by the additional type restriction. The motivation in #55466 was to give a slightly nicer MethodError. While I don't think this is per-se a particularly strong justification, I do agree that it's awkward for the `Main` version of these functions to have (marginally) different behavior than the version of these functions that gets introduced automatically in new modules (Which has been the case ever since [1], which added the AbstractString restriction in `Main`, but not in the auto-generated versions). This is particularly true, because we use the `Main` version to document the auto-introduction of these methods, which has regularly been a point of confusion. This PR tries to address this problem once and for all, but just not generating special methods into every new module. Instead, there are curried helpers for eval and include in Core and Base (respectively), which can be added to a module simply by doing `const include = IncludeInto(MyModule)` (and similarly for `eval`). As before, this happens automatically for non-bare modules. It thus conforms the behavior of the `Main` version of these functions and the auto-generated versions by construction. Additionally, it saves us having to generate all the additional code/types/objects, etc associated with having extra generic functions in each new module. The impact of this isn't huge, because there aren't that many modules, but it feels conceptually nicer. There is a little bit of extra work in this PR because we have special snowflake backtrace printing code for the `include` machinery, which needs adjusting, but other than that the change is straightforward. [1] 957848b
1 parent 24e2dba commit 7c38999

File tree

10 files changed

+60
-44
lines changed

10 files changed

+60
-44
lines changed

base/Base.jl

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,11 @@ function include(mod::Module, path::String)
2525
end
2626
include(path::String) = include(Base, path)
2727

28+
struct IncludeInto <: Function
29+
m::Module
30+
end
31+
(this::IncludeInto)(fname::AbstractString) = include(this.m, fname)
32+
2833
# from now on, this is now a top-module for resolving syntax
2934
const is_primary_base_module = ccall(:jl_module_parent, Ref{Module}, (Any,), Base) === Core.Main
3035
ccall(:jl_set_istopmod, Cvoid, (Any, Bool), Base, is_primary_base_module)
@@ -571,15 +576,20 @@ include("precompilation.jl")
571576
for m in methods(include)
572577
delete_method(m)
573578
end
579+
for m in methods(IncludeInto(Base))
580+
delete_method(m)
581+
end
574582

575583
# This method is here only to be overwritten during the test suite to test
576584
# various sysimg related invalidation scenarios.
577585
a_method_to_overwrite_in_test() = inferencebarrier(1)
578586

579587
# These functions are duplicated in client.jl/include(::String) for
580588
# nicer stacktraces. Modifications here have to be backported there
581-
include(mod::Module, _path::AbstractString) = _include(identity, mod, _path)
582-
include(mapexpr::Function, mod::Module, _path::AbstractString) = _include(mapexpr, mod, _path)
589+
@noinline include(mod::Module, _path::AbstractString) = _include(identity, mod, _path)
590+
@noinline include(mapexpr::Function, mod::Module, _path::AbstractString) = _include(mapexpr, mod, _path)
591+
(this::IncludeInto)(fname::AbstractString) = include(identity, this.m, fname)
592+
(this::IncludeInto)(mapexpr::Function, fname::AbstractString) = include(mapexpr, this.m, fname)
583593

584594
# External libraries vendored into Base
585595
Core.println("JuliaSyntax/src/JuliaSyntax.jl")

base/boot.jl

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -434,9 +434,13 @@ Nothing() = nothing
434434
# This should always be inlined
435435
getptls() = ccall(:jl_get_ptls_states, Ptr{Cvoid}, ())
436436

437-
include(m::Module, fname::String) = ccall(:jl_load_, Any, (Any, Any), m, fname)
437+
include(m::Module, fname::String) = (@noinline; ccall(:jl_load_, Any, (Any, Any), m, fname))
438+
eval(m::Module, @nospecialize(e)) = (@noinline; ccall(:jl_toplevel_eval_in, Any, (Any, Any), m, e))
438439

439-
eval(m::Module, @nospecialize(e)) = ccall(:jl_toplevel_eval_in, Any, (Any, Any), m, e)
440+
struct EvalInto <: Function
441+
m::Module
442+
end
443+
(this::EvalInto)(@nospecialize(e)) = eval(this.m, e)
440444

441445
mutable struct Box
442446
contents::Any

base/docs/basedocs.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2578,7 +2578,7 @@ cases.
25782578
See also [`setproperty!`](@ref Base.setproperty!) and [`getglobal`](@ref)
25792579
25802580
# Examples
2581-
```jldoctest; filter = r"Stacktrace:(\\n \\[[0-9]+\\].*)*"
2581+
```jldoctest; filter = r"Stacktrace:(\\n \\[[0-9]+\\].*\\n.*)*"
25822582
julia> module M; global a; end;
25832583
25842584
julia> M.a # same as `getglobal(M, :a)`

base/errorshow.jl

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -850,7 +850,10 @@ function _simplify_include_frames(trace)
850850
for i in length(trace):-1:1
851851
frame::StackFrame, _ = trace[i]
852852
mod = parentmodule(frame)
853-
if first_ignored === nothing
853+
if mod === Base && frame.func === :IncludeInto ||
854+
mod === Core && frame.func === :EvalInto
855+
kept_frames[i] = false
856+
elseif first_ignored === nothing
854857
if mod === Base && frame.func === :_include
855858
# Hide include() machinery by default
856859
first_ignored = i

base/loading.jl

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2771,12 +2771,12 @@ julia> rm("testfile.jl")
27712771
```
27722772
"""
27732773
function evalfile(path::AbstractString, args::Vector{String}=String[])
2774-
return Core.eval(Module(:__anon__),
2774+
m = Module(:__anon__)
2775+
return Core.eval(m,
27752776
Expr(:toplevel,
27762777
:(const ARGS = $args),
2777-
:(eval(x) = $(Expr(:core, :eval))(__anon__, x)),
2778-
:(include(x) = $(Expr(:top, :include))(__anon__, x)),
2779-
:(include(mapexpr::Function, x) = $(Expr(:top, :include))(mapexpr, __anon__, x)),
2778+
:(const include = $(Base.IncludeInto(m))),
2779+
:(const eval = $(Core.EvalInto(m))),
27802780
:(include($path))))
27812781
end
27822782
evalfile(path::AbstractString, args::Vector) = evalfile(path, String[args...])

base/show.jl

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3338,3 +3338,22 @@ end
33383338
function show(io::IO, ::MIME"text/plain", oc::Core.OpaqueClosure{A, R}) where {A, R}
33393339
show(io, oc)
33403340
end
3341+
3342+
# Special pretty printing for EvalInto/IncludeInto
3343+
function show(io::IO, ii::IncludeInto)
3344+
if getglobal(ii.m, :include) === ii
3345+
print(io, ii.m)
3346+
print(io, ".include")
3347+
else
3348+
show_default(io, ii)
3349+
end
3350+
end
3351+
3352+
function show(io::IO, ei::Core.EvalInto)
3353+
if getglobal(ei.m, :eval) === ei
3354+
print(io, ei.m)
3355+
print(io, ".eval")
3356+
else
3357+
show_default(io, ei)
3358+
end
3359+
end

base/sysimg.jl

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,7 @@ Use [`Base.include`](@ref) to evaluate a file into another module.
3232
!!! compat "Julia 1.5"
3333
Julia 1.5 is required for passing the `mapexpr` argument.
3434
"""
35-
include(mapexpr::Function, fname::AbstractString) = Base._include(mapexpr, Main, fname)
36-
function include(fname::AbstractString)
37-
isa(fname, String) || (fname = Base.convert(String, fname)::String)
38-
Base._include(identity, Main, fname)
39-
end
35+
const include = Base.IncludeInto(Main)
4036

4137
"""
4238
eval(expr)
@@ -45,7 +41,7 @@ Evaluate an expression in the global scope of the containing module.
4541
Every `Module` (except those defined with `baremodule`) has its own 1-argument
4642
definition of `eval`, which evaluates expressions in that module.
4743
"""
48-
eval(x) = Core.eval(Main, x)
44+
const eval = Core.EvalInto(Main)
4945

5046
# Ensure this file is also tracked
5147
pushfirst!(Base._included_files, (@__MODULE__, abspath(@__FILE__)))

src/jlfrontend.scm

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -199,28 +199,6 @@
199199
(error-wrap (lambda ()
200200
(julia-expand-macroscope expr))))
201201

202-
;; construct default definitions of `eval` for non-bare modules
203-
;; called by jl_eval_module_expr
204-
(define (module-default-defs name file line)
205-
(jl-expand-to-thunk
206-
(let* ((loc (if (and (eq? file 'none) (eq? line 0)) '() `((line ,line ,file))))
207-
(x (if (eq? name 'x) 'y 'x))
208-
(mex (if (eq? name 'mapexpr) 'map_expr 'mapexpr)))
209-
`(block
210-
(= (call eval ,x)
211-
(block
212-
,@loc
213-
(call (core eval) ,name ,x)))
214-
(= (call include ,x)
215-
(block
216-
,@loc
217-
(call (core _call_latest) (top include) ,name ,x)))
218-
(= (call include (:: ,mex (top Function)) ,x)
219-
(block
220-
,@loc
221-
(call (core _call_latest) (top include) ,mex ,name ,x)))))
222-
file line))
223-
224202
; run whole frontend on a string. useful for testing.
225203
(define (fe str)
226204
(expand-toplevel-expr (julia-parse str) 'none 0))

src/toplevel.c

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -206,11 +206,17 @@ static jl_value_t *jl_eval_module_expr(jl_module_t *parent_module, jl_expr_t *ex
206206
if (std_imports) {
207207
if (jl_base_module != NULL) {
208208
jl_add_standard_imports(newm);
209+
jl_datatype_t *include_into = (jl_datatype_t *)jl_get_global(jl_base_module, jl_symbol("IncludeInto"));
210+
if (include_into) {
211+
form = jl_new_struct(include_into, newm);
212+
jl_set_const(newm, jl_symbol("include"), form);
213+
}
214+
}
215+
jl_datatype_t *eval_into = (jl_datatype_t *)jl_get_global(jl_core_module, jl_symbol("EvalInto"));
216+
if (eval_into) {
217+
form = jl_new_struct(eval_into, newm);
218+
jl_set_const(newm, jl_symbol("eval"), form);
209219
}
210-
// add `eval` function
211-
form = jl_call_scm_on_ast_and_loc("module-default-defs", (jl_value_t*)name, newm, filename, lineno);
212-
jl_toplevel_eval_flex(newm, form, 0, 1, &filename, &lineno);
213-
form = NULL;
214220
}
215221

216222
for (int i = 0; i < jl_array_nrows(exprs); i++) {

test/reflection.jl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ let
179179
@test Base.binding_module(TestMod7648.TestModSub9475, :b9475) == TestMod7648.TestModSub9475
180180
defaultset = Set(Symbol[:Foo7648, :TestMod7648, :a9475, :c7648, :f9475, :foo7648, :foo7648_nomethods])
181181
allset = defaultset Set(Symbol[
182-
Symbol("#eval"), Symbol("#foo7648"), Symbol("#foo7648_nomethods"), Symbol("#include"),
182+
Symbol("#foo7648"), Symbol("#foo7648_nomethods"),
183183
:TestModSub9475, :d7648, :eval, :f7648, :include])
184184
imported = Set(Symbol[:convert, :curmod_name, :curmod])
185185
usings_from_Test = Set(Symbol[
@@ -265,7 +265,7 @@ let defaultset = Set((:A,))
265265
imported = Set((:M2,))
266266
usings_from_Base = delete!(Set(names(Module(); usings=true)), :anonymous) # the name of the anonymous module itself
267267
usings = Set((:A, :f, :C, :y, :M1, :m1_x)) usings_from_Base
268-
allset = Set((:A, :B, :C, :eval, :include, Symbol("#eval"), Symbol("#include")))
268+
allset = Set((:A, :B, :C, :eval, :include))
269269
@test Set(names(TestMod54609.A)) == defaultset
270270
@test Set(names(TestMod54609.A, imported=true)) == defaultset imported
271271
@test Set(names(TestMod54609.A, usings=true)) == defaultset usings

0 commit comments

Comments
 (0)