Skip to content

Commit 6384f0b

Browse files
committed
Always disallow assignments to constants
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.
1 parent b0323ab commit 6384f0b

File tree

2 files changed

+13
-13
lines changed

2 files changed

+13
-13
lines changed

src/module.c

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -545,14 +545,19 @@ JL_DLLEXPORT void jl_check_binding_currently_writable(jl_binding_t *b, jl_module
545545
{
546546
jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age);
547547
enum jl_partition_kind kind = jl_binding_kind(bpart);
548-
if (kind != BINDING_KIND_GLOBAL && kind != BINDING_KIND_DECLARED && !jl_bkind_is_some_constant(kind)) {
548+
if (kind != BINDING_KIND_GLOBAL && kind != BINDING_KIND_DECLARED) {
549549
if (jl_bkind_is_some_guard(kind)) {
550550
jl_errorf("Global %s.%s does not exist and cannot be assigned.\n"
551551
"Note: Julia 1.9 and 1.10 inadvertently omitted this error check (#56933).\n"
552552
"Hint: Declare it using `global %s` inside `%s` before attempting assignment.",
553553
jl_symbol_name(m->name), jl_symbol_name(s),
554554
jl_symbol_name(s), jl_symbol_name(m->name));
555-
} else {
555+
}
556+
else if (jl_bkind_is_some_constant(kind)) {
557+
jl_errorf("invalid assignment to constant %s.%s. This redefinition may be permitted using the `const` keyword.",
558+
jl_symbol_name(m->name), jl_symbol_name(s));
559+
}
560+
else {
556561
jl_module_t *from = jl_binding_dbgmodule(b, m, s);
557562
if (from == m)
558563
jl_errorf("cannot assign a value to imported variable %s.%s",
@@ -1449,16 +1454,6 @@ jl_value_t *jl_check_binding_assign_value(jl_binding_t *b JL_PROPAGATES_ROOT, jl
14491454
JL_GC_PUSH1(&rhs); // callee-rooted
14501455
jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age);
14511456
enum jl_partition_kind kind = jl_binding_kind(bpart);
1452-
if (jl_bkind_is_some_constant(kind)) {
1453-
jl_value_t *old = bpart->restriction;
1454-
JL_GC_PROMISE_ROOTED(old);
1455-
if (jl_egal(rhs, old)) {
1456-
JL_GC_POP();
1457-
return NULL;
1458-
}
1459-
jl_errorf("invalid assignment to constant %s.%s. This redefinition may be permitted using the `const` keyword.",
1460-
jl_symbol_name(mod->name), jl_symbol_name(var));
1461-
}
14621457
assert(kind == BINDING_KIND_DECLARED || kind == BINDING_KIND_GLOBAL);
14631458
jl_value_t *old_ty = kind == BINDING_KIND_DECLARED ? (jl_value_t*)jl_any_type : bpart->restriction;
14641459
JL_GC_PROMISE_ROOTED(old_ty);

test/syntax.jl

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3969,8 +3969,13 @@ end
39693969

39703970
# Test trying to define a constant and then trying to assign to the same value
39713971
module AssignConstValueTest
3972+
using Test
39723973
const x = 1
3973-
x = 1
3974+
@test_throws ErrorException @eval x = 1
3975+
@test_throws ErrorException @eval begin
3976+
@Base.Experimental.force_compile
3977+
global x = 1
3978+
end
39743979
end
39753980
@test isconst(AssignConstValueTest, :x)
39763981

0 commit comments

Comments
 (0)