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.

Sign up for GitHub

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

builtins: add Core.throw_methoderror #55705

Merged
merged 2 commits into from
Sep 9, 2024
Merged

builtins: add Core.throw_methoderror #55705

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3a7a52b
builtins: add `Core.throw_methoderror`
topolarity Sep 6, 2024
ee2dbec
Use `jl_value_t` in place of `jl_function_t`
topolarity Sep 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions base/compiler/abstractinterpretation.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2226,6 +2226,17 @@ function abstract_throw(interp::AbstractInterpreter, argtypes::Vector{Any}, ::Ab
return CallMeta(Union{}, exct, EFFECTS_THROWS, NoCallInfo())
end

function abstract_throw_methoderror(interp::AbstractInterpreter, argtypes::Vector{Any}, ::AbsIntState)
exct = if length(argtypes) == 1
ArgumentError
elseif !isvarargtype(argtypes[2])
MethodError
else
tmerge(𝕃ᵢ, MethodError, ArgumentError)
end
return CallMeta(Union{}, exct, EFFECTS_THROWS, NoCallInfo())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess its not consistent because it captures the world?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great point - I got that right in the tfuncs but not here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I think this is consistent after all.

Consistency doesn't place requirements on exceptions:

   If :consistent functions terminate by throwing an exception, that exception
    itself is not required to meet the egality requirement specified above.

And even if it did, it is per-world-age:

 The :consistent-cy assertion is made world-age wise. More formally, write
    fᵢ for the evaluation of f in world-age i, then this setting requires:
    ∀ i, x, y: x ≡ y → fᵢ(x) ≡ fᵢ(y)
    However, for two world ages i, j s.t. i ≠ j, we may have fᵢ(x) ≢ fⱼ(y).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, :consistent doesn't require the consistency on thrown object. If a method throws consistently, then it is :consistent no matter what it throws.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait that is a kind of odd spec for it to have. Does that mean we can't inline the throw of an exception if we think a function throws?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, even if concrete evaluation reveals that a method call will definitely throw, we can't propagate that exception as a Const.

end

# call where the function is known exactly
function abstract_call_known(interp::AbstractInterpreter, @nospecialize(f),
arginfo::ArgInfo, si::StmtInfo, sv::AbsIntState,
Expand All @@ -2246,6 +2257,8 @@ function abstract_call_known(interp::AbstractInterpreter, @nospecialize(f),
return abstract_applicable(interp, argtypes, sv, max_methods)
elseif f === throw
return abstract_throw(interp, argtypes, sv)
elseif f === Core.throw_methoderror
return abstract_throw_methoderror(interp, argtypes, sv)
end
rt = abstract_call_builtin(interp, f, arginfo, sv)
ft = popfirst!(argtypes)
Expand Down
1 change: 1 addition & 0 deletions base/compiler/ssair/EscapeAnalysis/EscapeAnalysis.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1213,6 +1213,7 @@ escape_builtin!(::typeof(Core.donotdelete), _...) = false
# not really safe, but `ThrownEscape` will be imposed later
escape_builtin!(::typeof(isdefined), _...) = false
escape_builtin!(::typeof(throw), _...) = false
escape_builtin!(::typeof(Core.throw_methoderror), _...) = false

function escape_builtin!(::typeof(ifelse), astate::AnalysisState, pc::Int, args::Vector{Any})
length(args) == 4 || return false
Expand Down
4 changes: 4 additions & 0 deletions base/compiler/tfuncs.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ function add_tfunc(@nospecialize(f::Builtin), minarg::Int, maxarg::Int, @nospeci
end

add_tfunc(throw, 1, 1, @nospecs((𝕃::AbstractLattice, x)->Bottom), 0)
add_tfunc(Core.throw_methoderror, 1, INT_INF, @nospecs((𝕃::AbstractLattice, x)->Bottom), 0)

# the inverse of typeof_tfunc
# returns (type, isexact, isconcrete, istype)
Expand Down Expand Up @@ -2313,6 +2314,7 @@ const _CONSISTENT_BUILTINS = Any[
(<:),
typeassert,
throw,
Core.throw_methoderror,
setfield!,
donotdelete
]
Expand All @@ -2335,6 +2337,7 @@ const _EFFECT_FREE_BUILTINS = [
(<:),
typeassert,
throw,
Core.throw_methoderror,
getglobal,
compilerbarrier,
]
Expand All @@ -2350,6 +2353,7 @@ const _INACCESSIBLEMEM_BUILTINS = Any[
isa,
nfields,
throw,
Core.throw_methoderror,
tuple,
typeassert,
typeof,
Expand Down
1 change: 1 addition & 0 deletions src/builtin_proto.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ DECLARE_BUILTIN(svec);
DECLARE_BUILTIN(swapfield);
DECLARE_BUILTIN(swapglobal);
DECLARE_BUILTIN(throw);
DECLARE_BUILTIN(throw_methoderror);
DECLARE_BUILTIN(tuple);
DECLARE_BUILTIN(typeassert);
DECLARE_BUILTIN(typeof);
Expand Down
9 changes: 9 additions & 0 deletions src/builtins.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,14 @@ JL_CALLABLE(jl_f_throw)
return jl_nothing;
}

JL_CALLABLE(jl_f_throw_methoderror)
{
JL_NARGSV(throw_methoderror, 1);
size_t world = jl_get_tls_world_age();
jl_method_error((jl_function_t *)args[0], &args[1], nargs, world);
topolarity marked this conversation as resolved.
Show resolved Hide resolved
return jl_nothing;
}

JL_CALLABLE(jl_f_ifelse)
{
JL_NARGS(ifelse, 3, 3);
Expand Down Expand Up @@ -2437,6 +2445,7 @@ void jl_init_primitives(void) JL_GC_DISABLED
add_builtin_func("_compute_sparams", jl_f__compute_sparams);
add_builtin_func("_svec_ref", jl_f__svec_ref);
add_builtin_func("current_scope", jl_f_current_scope);
add_builtin_func("throw_methoderror", jl_f_throw_methoderror);

// builtin types
add_builtin("Any", (jl_value_t*)jl_any_type);
Expand Down
2 changes: 1 addition & 1 deletion src/staticdata.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@ static htable_t relocatable_ext_cis;
// (reverse of fptr_to_id)
// This is a manually constructed dual of the fvars array, which would be produced by codegen for Julia code, for C.
static const jl_fptr_args_t id_to_fptrs[] = {
&jl_f_throw, &jl_f_is, &jl_f_typeof, &jl_f_issubtype, &jl_f_isa,
&jl_f_throw, &jl_f_throw_methoderror, &jl_f_is, &jl_f_typeof, &jl_f_issubtype, &jl_f_isa,
&jl_f_typeassert, &jl_f__apply_iterate, &jl_f__apply_pure,
&jl_f__call_latest, &jl_f__call_in_world, &jl_f__call_in_world_total, &jl_f_isdefined,
&jl_f_tuple, &jl_f_svec, &jl_f_intrinsic_call,
Expand Down