From 4ccc799512aec0781c26f590722857e6229fff27 Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Wed, 25 Mar 2026 23:09:30 +0300 Subject: [PATCH] checker: fix immutability of variable may be broken (fixes #17135) --- vlib/v/checker/checker.v | 169 +++++++++++++++++- .../tests/immutable_to_mutable_err.out | 79 ++++---- .../checker/tests/immutable_to_mutable_err.vv | 11 ++ 3 files changed, 214 insertions(+), 45 deletions(-) diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index 664c273ab..b1585f62c 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -1026,13 +1026,164 @@ fn (mut c Checker) expand_iface_embeds(idecl &ast.InterfaceDecl, level int, ifac return ares } +fn (mut c Checker) expr_is_immutable_source(expr ast.Expr) bool { + match expr { + ast.Ident { + return match expr.obj { + ast.Var { !expr.obj.is_mut } + ast.ConstField { true } + else { false } + } + } + ast.SelectorExpr { + if expr.expr_type == 0 { + return false + } + typ_sym := c.table.final_sym(c.unwrap_generic(expr.expr_type)) + match typ_sym.kind { + .struct { + if field_info := c.table.find_field_with_embeds(typ_sym, expr.field_name) { + return !field_info.is_mut || c.expr_is_immutable_source(expr.expr) + } + } + .interface { + interface_info := typ_sym.info as ast.Interface + if field_info := interface_info.find_field(expr.field_name) { + return !field_info.is_mut || c.expr_is_immutable_source(expr.expr) + } + } + .sum_type { + sumtype_info := typ_sym.info as ast.SumType + if field_info := sumtype_info.find_sum_type_field(expr.field_name) { + return !field_info.is_mut || c.expr_is_immutable_source(expr.expr) + } + } + else {} + } + return c.expr_is_immutable_source(expr.expr) + } + ast.IndexExpr { + return c.expr_is_immutable_source(expr.left) + } + ast.ParExpr { + return c.expr_is_immutable_source(expr.expr) + } + else { + return false + } + } +} + +fn (mut c Checker) type_contains_mutable_aliasing(typ ast.Type, mut checked_types []ast.Type) bool { + unwrapped_typ := c.unwrap_generic(typ.clear_option_and_result()) + if unwrapped_typ == 0 { + return false + } + if unwrapped_typ.has_flag(.shared_f) || unwrapped_typ.is_any_kind_of_pointer() { + return true + } + if unwrapped_typ in checked_types { + return false + } + checked_types << unwrapped_typ + sym := c.table.sym(unwrapped_typ) + match sym.info { + ast.Alias { + return c.type_contains_mutable_aliasing(sym.info.parent_type, mut checked_types) + } + ast.Array, ast.Map, ast.Interface { + return true + } + ast.ArrayFixed { + return c.type_contains_mutable_aliasing(sym.info.elem_type, mut checked_types) + } + ast.Struct { + if sym.kind == .struct && sym.language == .v { + for field in c.table.struct_fields(sym) { + if !field.is_mut { + continue + } + if c.type_contains_mutable_aliasing(field.typ, mut checked_types) { + return true + } + } + } + } + ast.SumType { + for variant in sym.info.variants { + if c.type_contains_mutable_aliasing(variant, mut checked_types) { + return true + } + } + } + else {} + } + return false +} + +fn (mut c Checker) expr_is_mutable_alias_of_immutable_source(expr ast.Expr) bool { + match expr { + ast.Ident { + if expr.obj is ast.Var && expr.obj.is_mut && expr.obj.expr is ast.Ident { + mut checked_types := []ast.Type{} + if c.type_contains_mutable_aliasing(expr.obj.typ, mut checked_types) { + return c.expr_is_immutable_source(expr.obj.expr) + } + } + return false + } + ast.SelectorExpr { + if expr.expr_type == 0 { + return false + } + typ_sym := c.table.final_sym(c.unwrap_generic(expr.expr_type)) + match typ_sym.kind { + .struct { + if field_info := c.table.find_field_with_embeds(typ_sym, expr.field_name) { + mut checked_types := []ast.Type{} + return field_info.is_mut + && c.type_contains_mutable_aliasing(field_info.typ, mut checked_types) + && c.expr_is_mutable_alias_of_immutable_source(expr.expr) + } + } + .interface { + interface_info := typ_sym.info as ast.Interface + if field_info := interface_info.find_field(expr.field_name) { + mut checked_types := []ast.Type{} + return field_info.is_mut + && c.type_contains_mutable_aliasing(field_info.typ, mut checked_types) + && c.expr_is_mutable_alias_of_immutable_source(expr.expr) + } + } + .sum_type { + sumtype_info := typ_sym.info as ast.SumType + if field_info := sumtype_info.find_sum_type_field(expr.field_name) { + mut checked_types := []ast.Type{} + return field_info.is_mut + && c.type_contains_mutable_aliasing(field_info.typ, mut checked_types) + && c.expr_is_mutable_alias_of_immutable_source(expr.expr) + } + } + else {} + } + return false + } + ast.ParExpr { + return c.expr_is_mutable_alias_of_immutable_source(expr.expr) + } + else { + return false + } + } +} + // fail_if_immutable_to_mutable checks if there is a immutable reference on right-side of assignment for mutable var fn (mut c Checker) fail_if_immutable_to_mutable(left_type ast.Type, right_type ast.Type, right ast.Expr) bool { + if c.inside_unsafe || c.pref.translated || c.file.is_translated { + return true + } match right { ast.Ident { - if c.inside_unsafe || c.pref.translated || c.file.is_translated { - return true - } if right.obj is ast.Var { if left_type.is_ptr() && !right.is_mut() && right_type.is_ptr() { c.note('`${right.name}` is immutable, cannot have a mutable reference to an immutable object', @@ -1048,9 +1199,6 @@ fn (mut c Checker) fail_if_immutable_to_mutable(left_type ast.Type, right_type a } } ast.IfExpr { - if c.inside_unsafe || c.pref.translated || c.file.is_translated { - return true - } for branch in right.branches { stmts := branch.stmts.filter(it is ast.ExprStmt) if stmts.len > 0 { @@ -1060,9 +1208,6 @@ fn (mut c Checker) fail_if_immutable_to_mutable(left_type ast.Type, right_type a } } ast.MatchExpr { - if c.inside_unsafe || c.pref.translated || c.file.is_translated { - return true - } for branch in right.branches { stmts := branch.stmts.filter(it is ast.ExprStmt) if stmts.len > 0 { @@ -1157,6 +1302,12 @@ fn (mut c Checker) fail_if_immutable(mut expr ast.Expr) (string, token.Pos) { return to_lock, pos } left_sym := c.table.sym(expr.left_type) + if !c.inside_unsafe && left_sym.kind == .array + && c.expr_is_mutable_alias_of_immutable_source(expr.left) { + c.error('`${expr.left}` aliases mutable data from an immutable value, clone it first (or use `unsafe`)', + expr.left.pos()) + return '', expr.pos + } mut elem_type := ast.no_type mut kind := '' match left_sym.info { diff --git a/vlib/v/checker/tests/immutable_to_mutable_err.out b/vlib/v/checker/tests/immutable_to_mutable_err.out index b3d4c1edd..88903081e 100644 --- a/vlib/v/checker/tests/immutable_to_mutable_err.out +++ b/vlib/v/checker/tests/immutable_to_mutable_err.out @@ -1,42 +1,49 @@ -vlib/v/checker/tests/immutable_to_mutable_err.vv:10:20: notice: condition is always true - 8 | fn main() { - 9 | arr := [1, 2, 3] // declared as immutable! - 10 | mut arr_mut := if true { arr } else { []int{} } +vlib/v/checker/tests/immutable_to_mutable_err.vv:15:20: notice: condition is always true + 13 | fn main() { + 14 | arr := [1, 2, 3] // declared as immutable! + 15 | mut arr_mut := if true { arr } else { []int{} } | ~~~~ - 11 | mut arr_mut2 := match true { - 12 | true { arr } -vlib/v/checker/tests/immutable_to_mutable_err.vv:10:27: notice: left-side of assignment expects a mutable reference, but variable `arr` is immutable, declare it with `mut` to make it mutable or clone it - 8 | fn main() { - 9 | arr := [1, 2, 3] // declared as immutable! - 10 | mut arr_mut := if true { arr } else { []int{} } + 16 | mut arr_mut2 := match true { + 17 | true { arr } +vlib/v/checker/tests/immutable_to_mutable_err.vv:15:27: notice: left-side of assignment expects a mutable reference, but variable `arr` is immutable, declare it with `mut` to make it mutable or clone it + 13 | fn main() { + 14 | arr := [1, 2, 3] // declared as immutable! + 15 | mut arr_mut := if true { arr } else { []int{} } | ~~~ - 11 | mut arr_mut2 := match true { - 12 | true { arr } -vlib/v/checker/tests/immutable_to_mutable_err.vv:12:3: notice: match is always true - 10 | mut arr_mut := if true { arr } else { []int{} } - 11 | mut arr_mut2 := match true { - 12 | true { arr } + 16 | mut arr_mut2 := match true { + 17 | true { arr } +vlib/v/checker/tests/immutable_to_mutable_err.vv:17:3: notice: match is always true + 15 | mut arr_mut := if true { arr } else { []int{} } + 16 | mut arr_mut2 := match true { + 17 | true { arr } | ~~~~ - 13 | else { [0] } - 14 | } -vlib/v/checker/tests/immutable_to_mutable_err.vv:12:10: notice: left-side of assignment expects a mutable reference, but variable `arr` is immutable, declare it with `mut` to make it mutable or clone it - 10 | mut arr_mut := if true { arr } else { []int{} } - 11 | mut arr_mut2 := match true { - 12 | true { arr } + 18 | else { [0] } + 19 | } +vlib/v/checker/tests/immutable_to_mutable_err.vv:17:10: notice: left-side of assignment expects a mutable reference, but variable `arr` is immutable, declare it with `mut` to make it mutable or clone it + 15 | mut arr_mut := if true { arr } else { []int{} } + 16 | mut arr_mut2 := match true { + 17 | true { arr } | ~~~ - 13 | else { [0] } - 14 | } -vlib/v/checker/tests/immutable_to_mutable_err.vv:22:15: error: use `mut array2 := array1.clone()` instead of `mut array2 := array1` (or use `unsafe`) - 20 | - 21 | a := Test{} - 22 | mut arr_mut3 := a.foo - | ~~ - 23 | arr_mut3[0] = 999 - 24 | assert a.foo == [1, 2, 3] -vlib/v/checker/tests/immutable_to_mutable_err.vv:26:15: error: use `mut array2 := array1.clone()` instead of `mut array2 := array1` (or use `unsafe`) - 24 | assert a.foo == [1, 2, 3] + 18 | else { [0] } + 19 | } +vlib/v/checker/tests/immutable_to_mutable_err.vv:27:15: error: use `mut array2 := array1.clone()` instead of `mut array2 := array1` (or use `unsafe`) 25 | - 26 | mut arr_mut4 := a.bar + 26 | a := Test{} + 27 | mut arr_mut3 := a.foo + | ~~ + 28 | arr_mut3[0] = 999 + 29 | assert a.foo == [1, 2, 3] +vlib/v/checker/tests/immutable_to_mutable_err.vv:31:15: error: use `mut array2 := array1.clone()` instead of `mut array2 := array1` (or use `unsafe`) + 29 | assert a.foo == [1, 2, 3] + 30 | + 31 | mut arr_mut4 := a.bar | ~~ - 27 | arr_mut4[0] = 999 - 28 | assert a.bar == [1, 2, 3] + 32 | arr_mut4[0] = 999 + 33 | assert a.bar == [1, 2, 3] +vlib/v/checker/tests/immutable_to_mutable_err.vv:39:5: error: `s2.list` aliases mutable data from an immutable value, clone it first (or use `unsafe`) + 37 | } + 38 | mut s2 := s + 39 | s2.list[0] = 'bbb' + | ~~~~ + 40 | + 41 | _ := a.bar diff --git a/vlib/v/checker/tests/immutable_to_mutable_err.vv b/vlib/v/checker/tests/immutable_to_mutable_err.vv index b5a86100d..6baa0ffaa 100644 --- a/vlib/v/checker/tests/immutable_to_mutable_err.vv +++ b/vlib/v/checker/tests/immutable_to_mutable_err.vv @@ -5,6 +5,11 @@ pub mut: foo []int = [1, 2, 3] } +struct TestAlias { +mut: + list []string +} + fn main() { arr := [1, 2, 3] // declared as immutable! mut arr_mut := if true { arr } else { []int{} } @@ -27,5 +32,11 @@ fn main() { arr_mut4[0] = 999 assert a.bar == [1, 2, 3] + s := TestAlias{ + list: ['aaa'] + } + mut s2 := s + s2.list[0] = 'bbb' + _ := a.bar } -- 2.39.5