From 73b85cfc04d83cb4b630dbd36ad2c270cf548330 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Tue, 22 Oct 2024 21:12:50 -0400 Subject: [PATCH] Use a curried helper for module-local eval/include (#55949) In https://github.com/JuliaLang/julia/pull/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 https://github.com/JuliaLang/julia/pull/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] https://github.com/JuliaLang/julia/commit/957848b899c7b5389af34cf815aa7bd2b6e2bf82 --------- Co-authored-by: Jameson Nash --- base/Base.jl | 14 ++++++++++++-- base/boot.jl | 8 ++++++-- base/docs/basedocs.jl | 2 +- base/errorshow.jl | 5 ++++- base/loading.jl | 8 ++++---- base/show.jl | 19 +++++++++++++++++++ base/sysimg.jl | 8 ++------ src/jlfrontend.scm | 22 ---------------------- src/toplevel.c | 14 ++++++++++---- test/docs.jl | 2 +- test/reflection.jl | 4 ++-- 11 files changed, 61 insertions(+), 45 deletions(-) diff --git a/base/Base.jl b/base/Base.jl index 5fb764bd4cc01..bfac74e5d7bab 100644 --- a/base/Base.jl +++ b/base/Base.jl @@ -25,6 +25,11 @@ function include(mod::Module, path::String) end include(path::String) = include(Base, path) +struct IncludeInto <: Function + m::Module +end +(this::IncludeInto)(fname::AbstractString) = include(this.m, fname) + # from now on, this is now a top-module for resolving syntax const is_primary_base_module = ccall(:jl_module_parent, Ref{Module}, (Any,), Base) === Core.Main ccall(:jl_set_istopmod, Cvoid, (Any, Bool), Base, is_primary_base_module) @@ -572,6 +577,9 @@ include("precompilation.jl") for m in methods(include) delete_method(m) end +for m in methods(IncludeInto(Base)) + delete_method(m) +end # This method is here only to be overwritten during the test suite to test # various sysimg related invalidation scenarios. @@ -579,8 +587,10 @@ a_method_to_overwrite_in_test() = inferencebarrier(1) # These functions are duplicated in client.jl/include(::String) for # nicer stacktraces. Modifications here have to be backported there -include(mod::Module, _path::AbstractString) = _include(identity, mod, _path) -include(mapexpr::Function, mod::Module, _path::AbstractString) = _include(mapexpr, mod, _path) +@noinline include(mod::Module, _path::AbstractString) = _include(identity, mod, _path) +@noinline include(mapexpr::Function, mod::Module, _path::AbstractString) = _include(mapexpr, mod, _path) +(this::IncludeInto)(fname::AbstractString) = include(identity, this.m, fname) +(this::IncludeInto)(mapexpr::Function, fname::AbstractString) = include(mapexpr, this.m, fname) # External libraries vendored into Base Core.println("JuliaSyntax/src/JuliaSyntax.jl") diff --git a/base/boot.jl b/base/boot.jl index 861c83a2edac5..ed3e22391f215 100644 --- a/base/boot.jl +++ b/base/boot.jl @@ -454,9 +454,13 @@ Nothing() = nothing # This should always be inlined getptls() = ccall(:jl_get_ptls_states, Ptr{Cvoid}, ()) -include(m::Module, fname::String) = ccall(:jl_load_, Any, (Any, Any), m, fname) +include(m::Module, fname::String) = (@noinline; ccall(:jl_load_, Any, (Any, Any), m, fname)) +eval(m::Module, @nospecialize(e)) = (@noinline; ccall(:jl_toplevel_eval_in, Any, (Any, Any), m, e)) -eval(m::Module, @nospecialize(e)) = ccall(:jl_toplevel_eval_in, Any, (Any, Any), m, e) +struct EvalInto <: Function + m::Module +end +(this::EvalInto)(@nospecialize(e)) = eval(this.m, e) mutable struct Box contents::Any diff --git a/base/docs/basedocs.jl b/base/docs/basedocs.jl index 0d5d5ac00e8d0..b080bf51e5e98 100644 --- a/base/docs/basedocs.jl +++ b/base/docs/basedocs.jl @@ -2580,7 +2580,7 @@ cases. See also [`setproperty!`](@ref Base.setproperty!) and [`getglobal`](@ref) # Examples -```jldoctest; filter = r"Stacktrace:(\\n \\[[0-9]+\\].*)*" +```jldoctest; filter = r"Stacktrace:(\\n \\[[0-9]+\\].*\\n.*)*" julia> module M; global a; end; julia> M.a # same as `getglobal(M, :a)` diff --git a/base/errorshow.jl b/base/errorshow.jl index 20bdee1de6ec0..7225a024f529e 100644 --- a/base/errorshow.jl +++ b/base/errorshow.jl @@ -850,7 +850,10 @@ function _simplify_include_frames(trace) for i in length(trace):-1:1 frame::StackFrame, _ = trace[i] mod = parentmodule(frame) - if first_ignored === nothing + if mod === Base && frame.func === :IncludeInto || + mod === Core && frame.func === :EvalInto + kept_frames[i] = false + elseif first_ignored === nothing if mod === Base && frame.func === :_include # Hide include() machinery by default first_ignored = i diff --git a/base/loading.jl b/base/loading.jl index dfcf22b2b0751..69bb332193519 100644 --- a/base/loading.jl +++ b/base/loading.jl @@ -2890,12 +2890,12 @@ julia> rm("testfile.jl") ``` """ function evalfile(path::AbstractString, args::Vector{String}=String[]) - return Core.eval(Module(:__anon__), + m = Module(:__anon__) + return Core.eval(m, Expr(:toplevel, :(const ARGS = $args), - :(eval(x) = $(Expr(:core, :eval))(__anon__, x)), - :(include(x::AbstractString) = $(Expr(:top, :include))(__anon__, x)), - :(include(mapexpr::Function, x::AbstractString) = $(Expr(:top, :include))(mapexpr, __anon__, x)), + :(const include = $(Base.IncludeInto(m))), + :(const eval = $(Core.EvalInto(m))), :(include($path)))) end evalfile(path::AbstractString, args::Vector) = evalfile(path, String[args...]) diff --git a/base/show.jl b/base/show.jl index 93f0a58b7cdd6..ee467ae90ff50 100644 --- a/base/show.jl +++ b/base/show.jl @@ -3418,3 +3418,22 @@ function show(io::IO, ::MIME"text/plain", bnd::Core.Binding) end end end + +# Special pretty printing for EvalInto/IncludeInto +function show(io::IO, ii::IncludeInto) + if getglobal(ii.m, :include) === ii + print(io, ii.m) + print(io, ".include") + else + show_default(io, ii) + end +end + +function show(io::IO, ei::Core.EvalInto) + if getglobal(ei.m, :eval) === ei + print(io, ei.m) + print(io, ".eval") + else + show_default(io, ei) + end +end diff --git a/base/sysimg.jl b/base/sysimg.jl index 966ed76751f28..ccc8ef38e81bc 100644 --- a/base/sysimg.jl +++ b/base/sysimg.jl @@ -32,11 +32,7 @@ Use [`Base.include`](@ref) to evaluate a file into another module. !!! compat "Julia 1.5" Julia 1.5 is required for passing the `mapexpr` argument. """ -include(mapexpr::Function, fname::AbstractString) = Base._include(mapexpr, Main, fname) -function include(fname::AbstractString) - isa(fname, String) || (fname = Base.convert(String, fname)::String) - Base._include(identity, Main, fname) -end +const include = Base.IncludeInto(Main) """ eval(expr) @@ -45,7 +41,7 @@ Evaluate an expression in the global scope of the containing module. Every `Module` (except those defined with `baremodule`) has its own 1-argument definition of `eval`, which evaluates expressions in that module. """ -eval(x) = Core.eval(Main, x) +const eval = Core.EvalInto(Main) # Ensure this file is also tracked pushfirst!(Base._included_files, (@__MODULE__, abspath(@__FILE__))) diff --git a/src/jlfrontend.scm b/src/jlfrontend.scm index 463e39c41d00a..808af18ebfdbd 100644 --- a/src/jlfrontend.scm +++ b/src/jlfrontend.scm @@ -199,28 +199,6 @@ (error-wrap (lambda () (julia-expand-macroscope expr)))) -;; construct default definitions of `eval` for non-bare modules -;; called by jl_eval_module_expr -(define (module-default-defs name file line) - (jl-expand-to-thunk - (let* ((loc (if (and (eq? file 'none) (eq? line 0)) '() `((line ,line ,file)))) - (x (if (eq? name 'x) 'y 'x)) - (mex (if (eq? name 'mapexpr) 'map_expr 'mapexpr))) - `(block - (= (call eval ,x) - (block - ,@loc - (call (core eval) ,name ,x))) - (= (call include (:: ,x (top AbstractString))) - (block - ,@loc - (call (core _call_latest) (top include) ,name ,x))) - (= (call include (:: ,mex (top Function)) (:: ,x (top AbstractString))) - (block - ,@loc - (call (core _call_latest) (top include) ,mex ,name ,x))))) - file line)) - ; run whole frontend on a string. useful for testing. (define (fe str) (expand-toplevel-expr (julia-parse str) 'none 0)) diff --git a/src/toplevel.c b/src/toplevel.c index 6f2e0cf77568a..c2fbc38d067eb 100644 --- a/src/toplevel.c +++ b/src/toplevel.c @@ -206,11 +206,17 @@ static jl_value_t *jl_eval_module_expr(jl_module_t *parent_module, jl_expr_t *ex if (std_imports) { if (jl_base_module != NULL) { jl_add_standard_imports(newm); + jl_datatype_t *include_into = (jl_datatype_t *)jl_get_global(jl_base_module, jl_symbol("IncludeInto")); + if (include_into) { + form = jl_new_struct(include_into, newm); + jl_set_const(newm, jl_symbol("include"), form); + } + } + jl_datatype_t *eval_into = (jl_datatype_t *)jl_get_global(jl_core_module, jl_symbol("EvalInto")); + if (eval_into) { + form = jl_new_struct(eval_into, newm); + jl_set_const(newm, jl_symbol("eval"), form); } - // add `eval` function - form = jl_call_scm_on_ast_and_loc("module-default-defs", (jl_value_t*)name, newm, filename, lineno); - jl_toplevel_eval_flex(newm, form, 0, 1, &filename, &lineno); - form = NULL; } newm->file = jl_symbol(filename); diff --git a/test/docs.jl b/test/docs.jl index 92d45fe05e397..8db9db30b8463 100644 --- a/test/docs.jl +++ b/test/docs.jl @@ -101,7 +101,7 @@ end @test Docs.undocumented_names(_ModuleWithUndocumentedNames) == [Symbol("@foo"), :f, :⨳] @test isempty(Docs.undocumented_names(_ModuleWithSomeDocumentedNames)) -@test Docs.undocumented_names(_ModuleWithSomeDocumentedNames; private=true) == [:eval, :g, :include] +@test Docs.undocumented_names(_ModuleWithSomeDocumentedNames; private=true) == [:g] # issue #11548 diff --git a/test/reflection.jl b/test/reflection.jl index 634390e0680d1..8c701acb9c09d 100644 --- a/test/reflection.jl +++ b/test/reflection.jl @@ -179,7 +179,7 @@ let @test Base.binding_module(TestMod7648.TestModSub9475, :b9475) == TestMod7648.TestModSub9475 defaultset = Set(Symbol[:Foo7648, :TestMod7648, :a9475, :c7648, :f9475, :foo7648, :foo7648_nomethods]) allset = defaultset ∪ Set(Symbol[ - Symbol("#eval"), Symbol("#foo7648"), Symbol("#foo7648_nomethods"), Symbol("#include"), + Symbol("#foo7648"), Symbol("#foo7648_nomethods"), :TestModSub9475, :d7648, :eval, :f7648, :include]) imported = Set(Symbol[:convert, :curmod_name, :curmod]) usings_from_Test = Set(Symbol[ @@ -265,7 +265,7 @@ let defaultset = Set((:A,)) imported = Set((:M2,)) usings_from_Base = delete!(Set(names(Module(); usings=true)), :anonymous) # the name of the anonymous module itself usings = Set((:A, :f, :C, :y, :M1, :m1_x)) ∪ usings_from_Base - allset = Set((:A, :B, :C, :eval, :include, Symbol("#eval"), Symbol("#include"))) + allset = Set((:A, :B, :C, :eval, :include)) @test Set(names(TestMod54609.A)) == defaultset @test Set(names(TestMod54609.A, imported=true)) == defaultset ∪ imported @test Set(names(TestMod54609.A, usings=true)) == defaultset ∪ usings