Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

checker: fix immutable to mutable reference #22663

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions vlib/term/ui/termios_nix.c.v
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
4 changes: 2 additions & 2 deletions vlib/toml/parser/parser.v
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down Expand Up @@ -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..]
}
Expand Down
32 changes: 5 additions & 27 deletions vlib/v/checker/assign.v
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down
74 changes: 73 additions & 1 deletion vlib/v/checker/checker.v
Original file line number Diff line number Diff line change
Expand Up @@ -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.error('`${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.error('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) {
Expand Down Expand Up @@ -4759,7 +4831,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 {}
Expand Down
4 changes: 2 additions & 2 deletions vlib/v/checker/infix.v
Original file line number Diff line number Diff line change
Expand Up @@ -314,12 +314,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]
Expand Down
14 changes: 14 additions & 0 deletions vlib/v/checker/tests/array_or_map_assign_err.out
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
vlib/v/checker/tests/array_or_map_assign_err.vv:5:7: error: 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:5:5: error: use `array2 = array1.clone()` instead of `array2 = array1` (or use `unsafe`)
3 | a2 := a1
4 | mut a3 := []int{}
Expand All @@ -12,6 +19,13 @@ vlib/v/checker/tests/array_or_map_assign_err.vv:10:8: error: cannot copy map: ca
| ~~
11 | mut m3 := map[string]int{}
12 | m3 = m1
vlib/v/checker/tests/array_or_map_assign_err.vv:12:7: error: 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:12:7: error: cannot copy map: call `move` or `clone` method (or use a reference)
10 | m2 := m1
11 | mut m3 := map[string]int{}
Expand Down
14 changes: 14 additions & 0 deletions vlib/v/checker/tests/fixed_array_conv.out
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@ vlib/v/checker/tests/fixed_array_conv.vv:6:5: warning: cannot cast a fixed array
| ~~~~~~~~~
7 | _ = p
8 | _ = ip
vlib/v/checker/tests/fixed_array_conv.vv:3:5: error: 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:3:3: error: mismatched types `voidptr` and `[2]int`
1 | arr := [2, 3]!
2 | mut p := unsafe { nil }
Expand All @@ -19,6 +26,13 @@ vlib/v/checker/tests/fixed_array_conv.vv:3:5: error: cannot assign to `p`: expec
| ~~~
4 | mut ip := &int(0)
5 | ip = arr
vlib/v/checker/tests/fixed_array_conv.vv:5:6: error: 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:5:4: error: mismatched types `&int` and `[2]int`
3 | p = arr
4 | mut ip := &int(0)
Expand Down
28 changes: 28 additions & 0 deletions vlib/v/checker/tests/immutable_to_mutable_err.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
vlib/v/checker/tests/immutable_to_mutable_err.vv:10:27: error: 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: error: 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]
31 changes: 31 additions & 0 deletions vlib/v/checker/tests/immutable_to_mutable_err.vv
Original file line number Diff line number Diff line change
@@ -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
}
7 changes: 7 additions & 0 deletions vlib/v/checker/tests/unsafe_fixed_array_assign.out
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
vlib/v/checker/tests/unsafe_fixed_array_assign.vv:10:10: error: 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]!
Expand Down
2 changes: 1 addition & 1 deletion vlib/v/gen/c/str.v
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion vlib/v/gen/js/str.v
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion vlib/v/tests/generics/generic_selector_test.v
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ pub fn (mut l List[T]) append(mut node Node[T]) ?int {
for {
if curr_node != none {
if next_node := curr_node?.next {
curr_node = next_node
curr_node = unsafe { next_node }
} else {
curr_node?.next = &node
l.size = l.size + 1
Expand Down
2 changes: 1 addition & 1 deletion vlib/v/token/keywords_matcher_trie.v
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading