Add "mea culpa" to foreign module assignment error. #56956
Conversation
|
Should this be backported to v1.11? |
|
Yes, you beat me to it ;). |
src/module.c
Outdated
| jl_errorf("Global %s.%s does not exist and cannot be assigned.\n" | ||
| "Note: Julia 1.9 and 1.10 inadvertently omitted this error check (#56933).\n" | ||
| "Hint: Declare it using `global` before attempting assignment.", jl_symbol_name(m->name), jl_symbol_name(var)); |
There was a problem hiding this comment.
| jl_errorf("Global %s.%s does not exist and cannot be assigned.\n" | |
| "Note: Julia 1.9 and 1.10 inadvertently omitted this error check (#56933).\n" | |
| "Hint: Declare it using `global` before attempting assignment.", jl_symbol_name(m->name), jl_symbol_name(var)); | |
| jl_errorf("Global %s.%s does not exist and cannot be assigned.\n" | |
| "Note: Julia 1.9 and 1.10 inadvertently omitted this error check, meaning it was possible to write `{module}.{var} = {value}` without a preceding `:global`.\n" | |
| "Hint: Declare it using `global` before attempting assignment:\n" | |
| " ```\n" | |
| " %s.eval(Expr(:global, :%s)\n" | |
| " %s.%s = ...\n" | |
| " ```", | |
| jl_symbol_name(m->name), jl_symbol_name(var), | |
| jl_symbol_name(m->name), jl_symbol_name(var), | |
| jl_symbol_name(m->name), jl_symbol_name(var)); |
There was a problem hiding this comment.
This is just to clarify things, as when I first tried this I thought I could do
global A.xbased on the error message, before realising I needed to evaluate global inside A.
There was a problem hiding this comment.
My primary concern with being too explicit here is that there are other paths that reach this error, not just the frontend syntax, so if you reach it through one of those paths, that error will be confusing.
There was a problem hiding this comment.
I don't think my issue needs to be linked in the error though. Just something to:
- Help people update their code (the "declare using
global" should be more precise), and - Let them know that this syntax used to work, it wasn't their imagination or mistake
There was a problem hiding this comment.
How about Hint: Declare it using `global %s` inside `%s` before attempting assignment.
There was a problem hiding this comment.
In particular f() = (Main.x = 1) and f() = (global x = 1) are not equivalent, because Main could be redefined later.
There was a problem hiding this comment.
Makes sense. I’m just trying to figure out how to use the exact mechanism the REPL uses for this. Because then we can just use that inside PythonCall to communicate new globals.
There was a problem hiding this comment.
When you write x = 1, the REPL does the moral equivalent of Core.eval(Main, :(x = 1)). Since there's no Main., the global gets introduced automatically at definition time.
There was a problem hiding this comment.
Calling Core.eval(::Module, Expr(:global, sym)) should be perfectly fine here though, as there isn't any opportunity to optimize this code anyways (it is python), and is what x=1 expands to normally. As Keno mentioned, we should add a builtin too (similar to setglobal, there should be a declareglobal and declareconst functions), so that you don't need the awkward eval form for them, but that is merely a difference in spelling of the operation
There was a problem hiding this comment.
Got it, thanks for the detailed discussion on this.
As requested in #56933, acknowledge that this error was missing in Julia 1.9 and 1.10 and provide a reference to the issue so that people have an easier starting point.
As requested in #56933, acknowledge that this error was missing in Julia 1.9 and 1.10 and provide a reference to the issue so that people have an easier starting point.