Skip to content

Commit

Permalink
Always disallow assignments to constants
Browse files Browse the repository at this point in the history
We have historically allowed the following without warning or error:

```
const x = 1
x = 1
```

As well as
```
const x = 1
x = 2
```
with a UB warning.

In 1.12, we made the second case error, as part of the general binding
partition changes, but did not touch the first case, because it did not
have the warning.

I think this made a reasonable amount of sense, because we essentially
treated constants as special kinds of mutable globals (to which assignment
happened to be UB).

However, in the 1.12+ design, constants and globals are quite sepearate
beasts, and I think it makes sense to extend the error to the egal case
also, even if it is technically more breaking.

In fact, I already thought that's what I did when I implemented the new
effect model for global assignment, causing #57566. I can't think of a
legitimate reason to keep this special case. For those who want to do
binding replacement, the `const` keyword is mandatory, so the assignment
is now literally always a semantic no-op or an error.

Fixes #57566.
  • Loading branch information
Keno committed Feb 28, 2025
1 parent b0323ab commit 89260fa
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 13 deletions.
19 changes: 7 additions & 12 deletions src/module.c
Original file line number Diff line number Diff line change
Expand Up @@ -545,14 +545,19 @@ JL_DLLEXPORT void jl_check_binding_currently_writable(jl_binding_t *b, jl_module
{
jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age);
enum jl_partition_kind kind = jl_binding_kind(bpart);
if (kind != BINDING_KIND_GLOBAL && kind != BINDING_KIND_DECLARED && !jl_bkind_is_some_constant(kind)) {
if (kind != BINDING_KIND_GLOBAL && kind != BINDING_KIND_DECLARED) {
if (jl_bkind_is_some_guard(kind)) {
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 %s` inside `%s` before attempting assignment.",
jl_symbol_name(m->name), jl_symbol_name(s),
jl_symbol_name(s), jl_symbol_name(m->name));
} else {
}
else if (jl_bkind_is_some_constant(kind)) {
jl_errorf("invalid assignment to constant %s.%s. This redefinition may be permitted using the `const` keyword.",
jl_symbol_name(m), jl_symbol_name(s));
}
else {
jl_module_t *from = jl_binding_dbgmodule(b, m, s);
if (from == m)
jl_errorf("cannot assign a value to imported variable %s.%s",
Expand Down Expand Up @@ -1449,16 +1454,6 @@ jl_value_t *jl_check_binding_assign_value(jl_binding_t *b JL_PROPAGATES_ROOT, jl
JL_GC_PUSH1(&rhs); // callee-rooted
jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age);
enum jl_partition_kind kind = jl_binding_kind(bpart);
if (jl_bkind_is_some_constant(kind)) {
jl_value_t *old = bpart->restriction;
JL_GC_PROMISE_ROOTED(old);
if (jl_egal(rhs, old)) {
JL_GC_POP();
return NULL;
}
jl_errorf("invalid assignment to constant %s.%s. This redefinition may be permitted using the `const` keyword.",
jl_symbol_name(mod->name), jl_symbol_name(var));
}
assert(kind == BINDING_KIND_DECLARED || kind == BINDING_KIND_GLOBAL);
jl_value_t *old_ty = kind == BINDING_KIND_DECLARED ? (jl_value_t*)jl_any_type : bpart->restriction;
JL_GC_PROMISE_ROOTED(old_ty);
Expand Down
7 changes: 6 additions & 1 deletion test/syntax.jl
Original file line number Diff line number Diff line change
Expand Up @@ -3969,8 +3969,13 @@ end

# Test trying to define a constant and then trying to assign to the same value
module AssignConstValueTest
using Test
const x = 1
x = 1
@test_throws ErrorException @eval x = 1
@test_throws ErrorException @eval begin
@Base.Experimental.force_compile
global x = 1
end
end
@test isconst(AssignConstValueTest, :x)

Expand Down

0 comments on commit 89260fa

Please sign in to comment.