Align interpreter and codegen error behavior of setglobal! and friends #59766
Conversation
base/errorshow.jl
Outdated
| elseif isa(ex.context, Core.Binding) | ||
| gr = ex.context.globalref | ||
| ctx = "in $(ex.func) of global binding `$(gr.mod).$(gr.name)`" |
There was a problem hiding this comment.
I do not like assuming that br.globalref is reliable, since it both often has the wrong module (it is often the resolved value instead of the original one) and it assumes that every Binding must have been associated with one GlobalRef, but we might want to have Bindings which are not just equivalent to a GlobalRef.
There was a problem hiding this comment.
Alright, I'll switch it to provide a GlobalRef for the original binding.
Currently this is an ErrorException in the runtime/interpreter, but a TypeError in codegen. This is not permitted - which error is thrown is semantically observable and codegen is not permitted to change it. Worse, inference is also inconsistent about whether this is TypeError or ErrorException, so this could actually lead to type confusion and crashes. Fix all that by having the runtime also emit a TypeError here. However, in order to not lose the binding name in the error message, adjust the TypeError context field to permit a binding.
Keno
force-pushed
the
kf/assigntypeerror
branch
from
d01041a to
3cebd25
Compare
| jl_value_t *gr = jl_module_globalref(mod, sym); | ||
| jl_value_t *ex = jl_new_struct(jl_typeerror_type, jl_symbol(fname), gr, expected, got); |
There was a problem hiding this comment.
Missing GC pushes here (the correctness of your annotations is not checked in this file)
There was a problem hiding this comment.
Both the symbol and the globalref should be globally rooted here?
There was a problem hiding this comment.
both expected and got are marked as unrooted
There was a problem hiding this comment.
Ah, I see what you're saying - will fix.
#59766) Currently this is an ErrorException in the runtime/interpreter, but a TypeError in codegen. This is not permitted - which error is thrown is semantically observable and codegen is not permitted to change it. Worse, inference is also inconsistent about whether this is TypeError or ErrorException, so this could actually lead to type confusion and crashes. Fix all that by having the runtime also emit a TypeError here. However, in order to not lose the binding name in the error message, adjust the TypeError context field to permit a binding. (cherry picked from commit 17e0df5)
2 participants
Currently this is an ErrorException in the runtime/interpreter, but a TypeError in codegen. This is not permitted - which error is thrown is semantically observable and codegen is not permitted to change it. Worse, inference is also inconsistent about whether this is TypeError or ErrorException, so this could actually lead to type confusion and crashes. Fix all that by having the runtime also emit a TypeError here. However, in order to not lose the binding name in the error message, adjust the TypeError context field to permit a binding.