Skip to content

Commit

Permalink
Turn Method Overwritten Error into a PrecompileError -- turning off c…
Browse files Browse the repository at this point in the history
…aching (#52214)

Fixes #52213

Overwritting methods during cache creation is currently not something
that the system
can support and can lead to surprising, counter-intuitive and fatal
errors.

In 1.10 we turned it from a warning to a strong error, with this PR it
remains
a strong error, but the precompilation system recognizes it and
essentially sets `__precompile__(false)`
for this module and all modules that depend on it.

Before:
```
julia> using OverwriteMethodError
[ Info: Precompiling OverwriteMethodError [top-level]
WARNING: Method definition +(Bool, Bool) in module Base at bool.jl:166 overwritten in module OverwriteMethodError at /home/vchuravy/src/julia2/OverwriteMethodError.jl:2.
ERROR: LoadError: Method overwriting is not permitted during Module precompile.
Stacktrace:
 [1] top-level scope
   @ ~/src/julia2/OverwriteMethodError.jl:2
 [2] include
   @ Base ./Base.jl:489 [inlined]
 [3] include_package_for_output(pkg::Base.PkgId, input::String, depot_path::Vector{String}, dl_load_path::Vector{String}, load_path::Vector{String}, concrete_deps::Vector{Pair{Base.PkgId, UInt128}}, source::Nothing)
   @ Base ./loading.jl:2216
 [4] top-level scope
   @ stdin:3
in expression starting at /home/vchuravy/src/julia2/OverwriteMethodError.jl:1
in expression starting at stdin:3
ERROR: Failed to precompile OverwriteMethodError [top-level] to "/home/vchuravy/.julia/compiled/v1.10/jl_guiuCQ".
Stacktrace:
  [1] error(s::String)
    @ Base ./error.jl:35
  [2] compilecache(pkg::Base.PkgId, path::String, internal_stderr::IO, internal_stdout::IO, keep_loaded_modules::Bool)
    @ Base ./loading.jl:2462
  [3] compilecache
    @ Base ./loading.jl:2334 [inlined]
  [4] (::Base.var"#968#969"{Base.PkgId})()
    @ Base ./loading.jl:1968
  [5] mkpidlock(f::Base.var"#968#969"{Base.PkgId}, at::String, pid::Int32; kwopts::@kwargs{stale_age::Int64, wait::Bool})
    @ FileWatching.Pidfile ~/.julia/juliaup/julia-1.10.0-rc1+0.x64.linux.gnu/share/julia/stdlib/v1.10/FileWatching/src/pidfile.jl:93
  [6] #mkpidlock#6
    @ FileWatching.Pidfile ~/.julia/juliaup/julia-1.10.0-rc1+0.x64.linux.gnu/share/julia/stdlib/v1.10/FileWatching/src/pidfile.jl:88 [inlined]
  [7] trymkpidlock(::Function, ::Vararg{Any}; kwargs::@kwargs{stale_age::Int64})
    @ FileWatching.Pidfile ~/.julia/juliaup/julia-1.10.0-rc1+0.x64.linux.gnu/share/julia/stdlib/v1.10/FileWatching/src/pidfile.jl:111
  [8] #invokelatest#2
    @ Base ./essentials.jl:889 [inlined]
  [9] invokelatest
    @ Base ./essentials.jl:884 [inlined]
 [10] maybe_cachefile_lock(f::Base.var"#968#969"{Base.PkgId}, pkg::Base.PkgId, srcpath::String; stale_age::Int64)
    @ Base ./loading.jl:2977
 [11] maybe_cachefile_lock
    @ Base ./loading.jl:2974 [inlined]
 [12] _require(pkg::Base.PkgId, env::String)
    @ Base ./loading.jl:1964
 [13] __require_prelocked(uuidkey::Base.PkgId, env::String)
    @ Base ./loading.jl:1806
 [14] #invoke_in_world#3
    @ Base ./essentials.jl:921 [inlined]
 [15] invoke_in_world
    @ Base ./essentials.jl:918 [inlined]
 [16] _require_prelocked(uuidkey::Base.PkgId, env::String)
    @ Base ./loading.jl:1797
 [17] macro expansion
    @ Base ./loading.jl:1784 [inlined]
 [18] macro expansion
    @ Base ./lock.jl:267 [inlined]
 [19] __require(into::Module, mod::Symbol)
    @ Base ./loading.jl:1747
 [20] #invoke_in_world#3
    @ Base ./essentials.jl:921 [inlined]
 [21] invoke_in_world
    @ Base ./essentials.jl:918 [inlined]
 [22] require(into::Module, mod::Symbol)
    @ Base ./loading.jl:1740
```

After:
```
julia> using OverwriteMethodError
┌ Info: Precompiling OverwriteMethodError [top-level]
└ @ Base loading.jl:2486
WARNING: Method definition +(Bool, Bool) in module Base at bool.jl:166 overwritten in module OverwriteMethodError at /home/vchuravy/src/julia2/OverwriteMethodError.jl:2.
ERROR: Method overwriting is not permitted during Module precompile.
┌ Info: Skipping precompilation since __precompile__(false). Importing OverwriteMethodError [top-level].
└ @ Base loading.jl:2084
```

---------

Co-authored-by: Kristoffer Carlsson <[email protected]>
(cherry picked from commit 9e8fe68)
  • Loading branch information
vchuravy authored and KristofferC committed Nov 27, 2023
1 parent 9c097b6 commit 2030e7d
Show file tree
Hide file tree
Showing 8 changed files with 23 additions and 4 deletions.
2 changes: 2 additions & 0 deletions base/boot.jl
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,8 @@ struct InitError <: WrappedException
error
end

struct PrecompilableError <: Exception end

String(s::String) = s # no constructor yet

const Cvoid = Nothing
Expand Down
2 changes: 1 addition & 1 deletion base/loading.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1686,7 +1686,7 @@ function include_dependency(path::AbstractString)
end

# we throw PrecompilableError when a module doesn't want to be precompiled
struct PrecompilableError <: Exception end
import Core: PrecompilableError
function show(io::IO, ex::PrecompilableError)
print(io, "Declaring __precompile__(false) is not allowed in files that are being precompiled.")
end
Expand Down
6 changes: 4 additions & 2 deletions src/gf.c
Original file line number Diff line number Diff line change
Expand Up @@ -1568,8 +1568,10 @@ static void method_overwrite(jl_typemap_entry_t *newentry, jl_method_t *oldvalue
jl_printf(s, ".\n");
jl_uv_flush(s);
}
if (jl_generating_output())
jl_error("Method overwriting is not permitted during Module precompile.");
if (jl_generating_output()) {
jl_printf(JL_STDERR, "ERROR: Method overwriting is not permitted during Module precompilation. Use `__precompile__(false)` to opt-out of precompilation.\n");
jl_throw(jl_precompilable_error);
}
}

static void update_max_args(jl_methtable_t *mt, jl_value_t *type)
Expand Down
1 change: 1 addition & 0 deletions src/jl_exported_data.inc
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@
XX(jl_voidpointer_type) \
XX(jl_void_type) \
XX(jl_weakref_type) \
XX(jl_precompilable_error) \

// Data symbols that are defined inside the public libjulia
#define JL_EXPORTED_DATA_SYMBOLS(XX) \
Expand Down
1 change: 1 addition & 0 deletions src/jltypes.c
Original file line number Diff line number Diff line change
Expand Up @@ -3410,6 +3410,7 @@ void post_boot_hooks(void)
jl_methoderror_type = (jl_datatype_t*)core("MethodError");
jl_loaderror_type = (jl_datatype_t*)core("LoadError");
jl_initerror_type = (jl_datatype_t*)core("InitError");
jl_precompilable_error = jl_new_struct_uninit((jl_datatype_t*)core("PrecompilableError"));
jl_pair_type = core("Pair");
jl_kwcall_func = core("kwcall");
jl_kwcall_mt = ((jl_datatype_t*)jl_typeof(jl_kwcall_func))->name->mt;
Expand Down
1 change: 1 addition & 0 deletions src/julia.h
Original file line number Diff line number Diff line change
Expand Up @@ -829,6 +829,7 @@ extern JL_DLLIMPORT jl_value_t *jl_readonlymemory_exception JL_GLOBALLY_ROOTED;
extern JL_DLLIMPORT jl_value_t *jl_diverror_exception JL_GLOBALLY_ROOTED;
extern JL_DLLIMPORT jl_value_t *jl_undefref_exception JL_GLOBALLY_ROOTED;
extern JL_DLLIMPORT jl_value_t *jl_interrupt_exception JL_GLOBALLY_ROOTED;
extern JL_DLLIMPORT jl_value_t *jl_precompilable_error JL_GLOBALLY_ROOTED;
extern JL_DLLIMPORT jl_datatype_t *jl_boundserror_type JL_GLOBALLY_ROOTED;
extern JL_DLLIMPORT jl_value_t *jl_an_empty_vec_any JL_GLOBALLY_ROOTED;
extern JL_DLLIMPORT jl_value_t *jl_an_empty_string JL_GLOBALLY_ROOTED;
Expand Down
3 changes: 2 additions & 1 deletion src/staticdata.c
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ extern "C" {
// TODO: put WeakRefs on the weak_refs list during deserialization
// TODO: handle finalizers

#define NUM_TAGS 158
#define NUM_TAGS 159

// An array of references that need to be restored from the sysimg
// This is a manually constructed dual of the gvars array, which would be produced by codegen for Julia code, for C.
Expand Down Expand Up @@ -224,6 +224,7 @@ jl_value_t **const*const get_tags(void) {
INSERT_TAG(jl_undefref_exception);
INSERT_TAG(jl_readonlymemory_exception);
INSERT_TAG(jl_atomicerror_type);
INSERT_TAG(jl_precompilable_error);

// other special values
INSERT_TAG(jl_emptysvec);
Expand Down
11 changes: 11 additions & 0 deletions test/precompile.jl
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,17 @@ precompile_test_harness(false) do dir
""")

@test Base.compilecache(Base.PkgId("Baz")) == Base.PrecompilableError() # due to __precompile__(false)

OverwriteMethodError_file = joinpath(dir, "OverwriteMethodError.jl")
write(OverwriteMethodError_file,
"""
module OverwriteMethodError
Base.:(+)(x::Bool, y::Bool) = false
end
""")

@test Base.compilecache(Base.PkgId("OverwriteMethodError")) == Base.PrecompilableError() # due to piracy

@eval using Baz
@test Base.invokelatest(Baz.baz) == 1

Expand Down

0 comments on commit 2030e7d

Please sign in to comment.