From 7d47219808f5517417290f4bbae4bc1fda35e5ba Mon Sep 17 00:00:00 2001 From: Felipe Pena Date: Tue, 10 Dec 2024 10:15:26 -0300 Subject: [PATCH] checker: fix immutable to mutable reference (fix #22653) (#22663) --- vlib/term/ui/termios_nix.c.v | 4 +- vlib/toml/parser/parser.v | 4 +- vlib/v/checker/assign.v | 32 ++------ vlib/v/checker/checker.v | 74 ++++++++++++++++++- vlib/v/checker/infix.v | 4 +- .../checker/tests/array_or_map_assign_err.out | 14 ++++ .../assign_immutable_reference_var_err.out | 4 +- vlib/v/checker/tests/deprecations_consts.out | 10 +-- vlib/v/checker/tests/deprecations_consts.vv | 1 + vlib/v/checker/tests/fixed_array_conv.out | 14 ++++ .../tests/immutable_to_mutable_err.out | 28 +++++++ .../checker/tests/immutable_to_mutable_err.vv | 31 ++++++++ .../tests/unsafe_fixed_array_assign.out | 7 ++ vlib/v/gen/c/str.v | 2 +- .../c/testdata/platform_wrapper_emscripten.vv | 1 - vlib/v/gen/js/str.v | 2 +- vlib/v/tests/skip_unused/for_c_stmt.vv | 3 +- vlib/v/token/keywords_matcher_trie.v | 2 +- 18 files changed, 190 insertions(+), 47 deletions(-) create mode 100644 vlib/v/checker/tests/immutable_to_mutable_err.out create mode 100644 vlib/v/checker/tests/immutable_to_mutable_err.vv diff --git a/vlib/term/ui/termios_nix.c.v b/vlib/term/ui/termios_nix.c.v index 8356b19b2..a4b30b6bd 100644 --- a/vlib/term/ui/termios_nix.c.v +++ b/vlib/term/ui/termios_nix.c.v @@ -258,12 +258,12 @@ fn (mut ctx Context) parse_events() { mut event := &Event(unsafe { nil }) if ctx.read_buf[0] == 0x1b { e, len := escape_sequence(ctx.read_buf.bytestr()) - event = e + event = unsafe { e } ctx.shift(len) } else { if ctx.read_all_bytes { e, len := multi_char(ctx.read_buf.bytestr()) - event = e + event = unsafe { e } ctx.shift(len) } else { event = single_char(ctx.read_buf.bytestr()) diff --git a/vlib/toml/parser/parser.v b/vlib/toml/parser/parser.v index 5446afd5b..88b69e94b 100644 --- a/vlib/toml/parser/parser.v +++ b/vlib/toml/parser/parser.v @@ -344,7 +344,7 @@ pub fn (mut p Parser) find_sub_table(key DottedKey) !&map[string]ast.Value { ky << p.root_map_key ky << key if p.root_map_key.len == 0 { - ky = key + ky = unsafe { key } } util.printdbg(@MOD + '.' + @STRUCT + '.' + @FN, 'locating "${ky}" in map ${ptr_str(p.root_map)}') mut t := unsafe { &p.root_map } @@ -1047,7 +1047,7 @@ pub fn (mut p Parser) double_array_of_tables_contents(target_key DottedKey) ![]a // Parse `[d.e.f]` p.ignore_while(space_formatting) dotted_key := p.dotted_key()! - implicit_allocation_key = dotted_key + implicit_allocation_key = unsafe { dotted_key } if dotted_key.len > 2 { implicit_allocation_key = dotted_key[2..] } diff --git a/vlib/v/checker/assign.v b/vlib/v/checker/assign.v index e264f7790..f40c4151a 100644 --- a/vlib/v/checker/assign.v +++ b/vlib/v/checker/assign.v @@ -235,33 +235,6 @@ fn (mut c Checker) assign_stmt(mut node ast.AssignStmt) { } } } - if left is ast.Ident && left.is_mut() && !c.inside_unsafe { - if left_type.is_ptr() && mut right is ast.Ident && !right.is_mut() - && right_type.is_ptr() { - c.error('`${right.name}` is immutable, cannot have a mutable reference to an immutable object', - right.pos) - } else if mut right is ast.StructInit { - typ_sym := c.table.sym(right.typ) - for init_field in right.init_fields { - if field_info := c.table.find_field_with_embeds(typ_sym, init_field.name) { - if field_info.is_mut { - if init_field.expr is ast.Ident && !init_field.expr.is_mut() - && init_field.typ.is_ptr() { - c.note('`${init_field.expr.name}` is immutable, cannot have a mutable reference to an immutable object', - init_field.pos) - } else if init_field.expr is ast.PrefixExpr { - if init_field.expr.op == .amp - && init_field.expr.right is ast.Ident - && !init_field.expr.right.is_mut() { - c.note('`${init_field.expr.right.name}` is immutable, cannot have a mutable reference to an immutable object', - init_field.expr.right.pos) - } - } - } - } - } - } - } } else { // Make sure the variable is mutable c.fail_if_immutable(mut left) @@ -275,6 +248,11 @@ fn (mut c Checker) assign_stmt(mut node ast.AssignStmt) { // c.error('cannot assign a `none` value to a non-option variable', right.pos()) // } } + if !c.inside_unsafe && !is_blank_ident && node.op in [.decl_assign, .assign] + && left is ast.Ident && left.is_mut() { + // check if right-side is a immutable reference + c.fail_if_immutable_to_mutable(left_type, right_type, right) + } if mut left is ast.Ident && left.info is ast.IdentVar && right is ast.Ident && right.name in c.global_names { ident_var_info := left.info as ast.IdentVar diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index 0b48afca9..6d03a745e 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -803,6 +803,78 @@ fn (mut c Checker) expand_iface_embeds(idecl &ast.InterfaceDecl, level int, ifac return ares } +// 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 { + 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', + right.pos) + return false + } + if !right.obj.is_mut + && c.table.final_sym(right_type).kind in [.array, .array_fixed, .map] { + c.note('left-side of assignment expects a mutable reference, but variable `${right.name}` is immutable, declare it with `mut` to make it mutable or clone it', + right.pos) + return false + } + } + } + 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 { + last_expr := stmts.last() as ast.ExprStmt + c.fail_if_immutable_to_mutable(left_type, right_type, last_expr.expr) + } + } + } + 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 { + last_expr := stmts.last() as ast.ExprStmt + c.fail_if_immutable_to_mutable(left_type, right_type, last_expr.expr) + } + } + } + ast.StructInit { + typ_sym := c.table.sym(right.typ) + for init_field in right.init_fields { + if field_info := c.table.find_field_with_embeds(typ_sym, init_field.name) { + if field_info.is_mut { + if init_field.expr is ast.Ident && !init_field.expr.is_mut() + && init_field.typ.is_ptr() { + c.note('`${init_field.expr.name}` is immutable, cannot have a mutable reference to an immutable object', + init_field.pos) + } else if init_field.expr is ast.PrefixExpr { + if init_field.expr.op == .amp && init_field.expr.right is ast.Ident + && !init_field.expr.right.is_mut() { + c.note('`${init_field.expr.right.name}` is immutable, cannot have a mutable reference to an immutable object', + init_field.expr.right.pos) + } + } + } + } + } + } + else { + return true + } + } + return true +} + // returns name and position of variable that needs write lock // also sets `is_changed` to true (TODO update the name to reflect this?) fn (mut c Checker) fail_if_immutable(mut expr ast.Expr) (string, token.Pos) { @@ -4800,7 +4872,7 @@ fn (mut c Checker) index_expr(mut node ast.IndexExpr) ast.Type { unwrapped_sym := c.table.final_sym(unwrapped_typ) if unwrapped_sym.kind in [.map, .array, .array_fixed] { typ = unwrapped_typ - typ_sym = unwrapped_sym + typ_sym = unsafe { unwrapped_sym } } } else {} diff --git a/vlib/v/checker/infix.v b/vlib/v/checker/infix.v index fbbde3a3e..7ae7477cd 100644 --- a/vlib/v/checker/infix.v +++ b/vlib/v/checker/infix.v @@ -321,12 +321,12 @@ fn (mut c Checker) infix_expr(mut node ast.InfixExpr) ast.Type { if mut right_sym.info is ast.Alias && (right_sym.info.language != .c && c.mod == c.table.type_to_str(unwrapped_right_type).split('.')[0] && (right_final_sym.is_primitive() || right_final_sym.kind == .enum)) { - right_sym = right_final_sym + right_sym = unsafe { right_final_sym } } if mut left_sym.info is ast.Alias && (left_sym.info.language != .c && c.mod == c.table.type_to_str(unwrapped_left_type).split('.')[0] && (left_final_sym.is_primitive() || left_final_sym.kind == .enum)) { - left_sym = left_final_sym + left_sym = unsafe { left_final_sym } } if c.pref.translated && node.op in [.plus, .minus, .mul] diff --git a/vlib/v/checker/tests/array_or_map_assign_err.out b/vlib/v/checker/tests/array_or_map_assign_err.out index 88edda1e5..0b0d5bc89 100644 --- a/vlib/v/checker/tests/array_or_map_assign_err.out +++ b/vlib/v/checker/tests/array_or_map_assign_err.out @@ -1,3 +1,17 @@ +vlib/v/checker/tests/array_or_map_assign_err.vv:5:7: notice: left-side of assignment expects a mutable reference, but variable `a1` is immutable, declare it with `mut` to make it mutable or clone it + 3 | a2 := a1 + 4 | mut a3 := []int{} + 5 | a3 = a1 + | ~~ + 6 | + 7 | m1 := { +vlib/v/checker/tests/array_or_map_assign_err.vv:12:7: notice: left-side of assignment expects a mutable reference, but variable `m1` is immutable, declare it with `mut` to make it mutable or clone it + 10 | m2 := m1 + 11 | mut m3 := map[string]int{} + 12 | m3 = m1 + | ~~ + 13 | + 14 | _ = a2 vlib/v/checker/tests/array_or_map_assign_err.vv:5:5: error: use `array2 = array1.clone()` instead of `array2 = array1` (or use `unsafe`) 3 | a2 := a1 4 | mut a3 := []int{} diff --git a/vlib/v/checker/tests/assign_immutable_reference_var_err.out b/vlib/v/checker/tests/assign_immutable_reference_var_err.out index fa690a6c8..1e309ea8d 100644 --- a/vlib/v/checker/tests/assign_immutable_reference_var_err.out +++ b/vlib/v/checker/tests/assign_immutable_reference_var_err.out @@ -1,5 +1,5 @@ -vlib/v/checker/tests/assign_immutable_reference_var_err.vv:8:11: error: `x` is immutable, cannot have a mutable reference to an immutable object - 6 | +vlib/v/checker/tests/assign_immutable_reference_var_err.vv:8:11: notice: `x` is immutable, cannot have a mutable reference to an immutable object + 6 | 7 | fn y(x &Foo) { 8 | mut m := x | ^ diff --git a/vlib/v/checker/tests/deprecations_consts.out b/vlib/v/checker/tests/deprecations_consts.out index 214a0c5a6..43160205f 100644 --- a/vlib/v/checker/tests/deprecations_consts.out +++ b/vlib/v/checker/tests/deprecations_consts.out @@ -1,6 +1,6 @@ -vlib/v/checker/tests/deprecations_consts.vv:3:25: error: const `deprecated_consts.a_deprecated_const` has been deprecated since 2023-12-31; use built-in constant min_i8 instead - 1 | import deprecated_consts - 2 | fn main() { - 3 | dump(deprecated_consts.a_deprecated_const) +vlib/v/checker/tests/deprecations_consts.vv:4:25: error: const `deprecated_consts.a_deprecated_const` has been deprecated since 2023-12-31; use built-in constant min_i8 instead + 2 | + 3 | fn main() { + 4 | dump(deprecated_consts.a_deprecated_const) | ~~~~~~~~~~~~~~~~~~ - 4 | } + 5 | } diff --git a/vlib/v/checker/tests/deprecations_consts.vv b/vlib/v/checker/tests/deprecations_consts.vv index e4222275e..d6c2295e2 100644 --- a/vlib/v/checker/tests/deprecations_consts.vv +++ b/vlib/v/checker/tests/deprecations_consts.vv @@ -1,4 +1,5 @@ import deprecated_consts + fn main() { dump(deprecated_consts.a_deprecated_const) } diff --git a/vlib/v/checker/tests/fixed_array_conv.out b/vlib/v/checker/tests/fixed_array_conv.out index 4f54b0c8a..d4e71f39d 100644 --- a/vlib/v/checker/tests/fixed_array_conv.out +++ b/vlib/v/checker/tests/fixed_array_conv.out @@ -1,3 +1,17 @@ +vlib/v/checker/tests/fixed_array_conv.vv:3:5: 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 + 1 | arr := [2, 3]! + 2 | mut p := unsafe { nil } + 3 | p = arr + | ~~~ + 4 | mut ip := &int(0) + 5 | ip = arr +vlib/v/checker/tests/fixed_array_conv.vv:5:6: 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 + 3 | p = arr + 4 | mut ip := &int(0) + 5 | ip = arr + | ~~~ + 6 | _ = &int(arr) + 7 | _ = p vlib/v/checker/tests/fixed_array_conv.vv:6:5: warning: cannot cast a fixed array (use e.g. `&arr[0]` instead) 4 | mut ip := &int(0) 5 | ip = arr diff --git a/vlib/v/checker/tests/immutable_to_mutable_err.out b/vlib/v/checker/tests/immutable_to_mutable_err.out new file mode 100644 index 000000000..c8f49dbab --- /dev/null +++ b/vlib/v/checker/tests/immutable_to_mutable_err.out @@ -0,0 +1,28 @@ +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{} } + | ~~~ + 11 | mut arr_mut2 := match true { + 12 | true { arr } +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 } + | ~~~ + 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] + 25 | + 26 | mut arr_mut4 := a.bar + | ~~ + 27 | arr_mut4[0] = 999 + 28 | assert a.bar == [1, 2, 3] diff --git a/vlib/v/checker/tests/immutable_to_mutable_err.vv b/vlib/v/checker/tests/immutable_to_mutable_err.vv new file mode 100644 index 000000000..b5a86100d --- /dev/null +++ b/vlib/v/checker/tests/immutable_to_mutable_err.vv @@ -0,0 +1,31 @@ +struct Test { +pub: + bar []int = [1, 2, 3] +pub mut: + foo []int = [1, 2, 3] +} + +fn main() { + arr := [1, 2, 3] // declared as immutable! + mut arr_mut := if true { arr } else { []int{} } + mut arr_mut2 := match true { + true { arr } + else { [0] } + } + arr_mut[0] = 999 + arr_mut2[1] = 999 + println(arr) + println(arr) + assert arr == [1, 2, 3] + + a := Test{} + mut arr_mut3 := a.foo + arr_mut3[0] = 999 + assert a.foo == [1, 2, 3] + + mut arr_mut4 := a.bar + arr_mut4[0] = 999 + assert a.bar == [1, 2, 3] + + _ := a.bar +} diff --git a/vlib/v/checker/tests/unsafe_fixed_array_assign.out b/vlib/v/checker/tests/unsafe_fixed_array_assign.out index 248da7231..616355c55 100644 --- a/vlib/v/checker/tests/unsafe_fixed_array_assign.out +++ b/vlib/v/checker/tests/unsafe_fixed_array_assign.out @@ -1,3 +1,10 @@ +vlib/v/checker/tests/unsafe_fixed_array_assign.vv:10:10: notice: left-side of assignment expects a mutable reference, but variable `a` is immutable, declare it with `mut` to make it mutable or clone it + 8 | } + 9 | a := [&box]! + 10 | mut b := a + | ^ + 11 | b[0].num = 0 + 12 | println(a) vlib/v/checker/tests/unsafe_fixed_array_assign.vv:10:7: error: assignment from one fixed array to another with a pointer element type is prohibited outside of `unsafe` 8 | } 9 | a := [&box]! diff --git a/vlib/v/gen/c/str.v b/vlib/v/gen/c/str.v index cc0272847..b3b818fc9 100644 --- a/vlib/v/gen/c/str.v +++ b/vlib/v/gen/c/str.v @@ -72,7 +72,7 @@ fn (mut g Gen) gen_expr_to_string(expr ast.Expr, etype ast.Type) { parent_sym := g.table.sym(sym.info.parent_type) if parent_sym.has_method('str') { typ = sym.info.parent_type - sym = parent_sym + sym = unsafe { parent_sym } } } sym_has_str_method, str_method_expects_ptr, _ := sym.str_method_info() diff --git a/vlib/v/gen/c/testdata/platform_wrapper_emscripten.vv b/vlib/v/gen/c/testdata/platform_wrapper_emscripten.vv index d20696c8e..eb43c17e6 100644 --- a/vlib/v/gen/c/testdata/platform_wrapper_emscripten.vv +++ b/vlib/v/gen/c/testdata/platform_wrapper_emscripten.vv @@ -1,7 +1,6 @@ module main // vtest vflags: -os wasm32_emscripten - import platform_wrapper fn main() { diff --git a/vlib/v/gen/js/str.v b/vlib/v/gen/js/str.v index e685afd16..2421f3120 100644 --- a/vlib/v/gen/js/str.v +++ b/vlib/v/gen/js/str.v @@ -87,7 +87,7 @@ fn (mut g JsGen) gen_expr_to_string(expr ast.Expr, etype ast.Type) { parent_sym := g.table.sym(sym.info.parent_type) if parent_sym.has_method('str') { typ = sym.info.parent_type - sym = parent_sym + sym = unsafe { parent_sym } } } sym_has_str_method, str_method_expects_ptr, _ := sym.str_method_info() diff --git a/vlib/v/tests/skip_unused/for_c_stmt.vv b/vlib/v/tests/skip_unused/for_c_stmt.vv index c45363f52..a6dfbc201 100644 --- a/vlib/v/tests/skip_unused/for_c_stmt.vv +++ b/vlib/v/tests/skip_unused/for_c_stmt.vv @@ -2,9 +2,8 @@ const abc = 12 fn main() { println('ok') - for x := abc; x<15;x++ { + for x := abc; x < 15; x++ { println(x) } println('ok') } - diff --git a/vlib/v/token/keywords_matcher_trie.v b/vlib/v/token/keywords_matcher_trie.v index 46dd9f6e5..2f0ed5953 100644 --- a/vlib/v/token/keywords_matcher_trie.v +++ b/vlib/v/token/keywords_matcher_trie.v @@ -160,7 +160,7 @@ pub fn (root &TrieNode) find(word string) int { if child == unsafe { nil } { return -1 } - node = child + node = unsafe { child } idx++ } return -1 -- 2.39.5