From a9c939adf63ae9aa0836ff08385603deb4b99aaa Mon Sep 17 00:00:00 2001 From: JMD <56417208+StunxFS@users.noreply.github.com> Date: Mon, 20 Oct 2025 17:15:12 -0400 Subject: [PATCH] checker: fix mutability in `for in` loops with pointer values (fix #25520) (#25536) --- vlib/arrays/reverse_iterator_test.v | 2 +- vlib/v/checker/for.v | 10 ++++--- vlib/v/gen/c/for.v | 27 +++++++++--------- .../v/tests/loops/for_in_mut_with_ptrs_test.v | 28 +++++++++++++++++++ 4 files changed, 48 insertions(+), 19 deletions(-) create mode 100644 vlib/v/tests/loops/for_in_mut_with_ptrs_test.v diff --git a/vlib/arrays/reverse_iterator_test.v b/vlib/arrays/reverse_iterator_test.v index 8c0dec9cd..9cefc89c9 100644 --- a/vlib/arrays/reverse_iterator_test.v +++ b/vlib/arrays/reverse_iterator_test.v @@ -45,7 +45,7 @@ fn test_reverse_iterator_with_mut() { mut after := []int{cap: original.len} for mut x in arrays.reverse_iterator(original) { before << *x - (**x)++ + (*x)++ after << *x } assert before == [20, 10] diff --git a/vlib/v/checker/for.v b/vlib/v/checker/for.v index bb68ef989..48f3b9937 100644 --- a/vlib/v/checker/for.v +++ b/vlib/v/checker/for.v @@ -150,7 +150,7 @@ fn (mut c Checker) for_in_stmt(mut node ast.ForInStmt) { c.error('iterator method `next()` must have 0 parameters', node.cond.pos()) } mut val_type := next_fn.return_type.clear_option_and_result() - if node.val_is_mut { + if node.val_is_mut && !val_type.is_any_kind_of_pointer() { val_type = val_type.ref() } node.cond_type = typ @@ -201,7 +201,7 @@ fn (mut c Checker) for_in_stmt(mut node ast.ForInStmt) { } mut value_type := c.table.value_type(unwrapped_typ) - if node.val_is_mut { + if node.val_is_mut && !value_type.is_any_kind_of_pointer() { value_type = value_type.ref() } node.scope.update_var_type(node.val_var, value_type) @@ -253,7 +253,9 @@ fn (mut c Checker) for_in_stmt(mut node ast.ForInStmt) { } } if node.val_is_mut { - value_type = value_type.ref() + if !value_type.is_any_kind_of_pointer() { + value_type = value_type.ref() + } if value_type.has_flag(.option) { value_type = value_type.set_flag(.option_mut_param_t) } @@ -287,7 +289,7 @@ fn (mut c Checker) for_in_stmt(mut node ast.ForInStmt) { } else {} } - } else if node.val_is_ref { + } else if node.val_is_ref && !value_type.is_any_kind_of_pointer() { value_type = value_type.ref() } node.cond_type = typ diff --git a/vlib/v/gen/c/for.v b/vlib/v/gen/c/for.v index 02d550927..e89b5db46 100644 --- a/vlib/v/gen/c/for.v +++ b/vlib/v/gen/c/for.v @@ -294,15 +294,14 @@ fn (mut g Gen) for_in_stmt(node_ ast.ForInStmt) { } else { needs_memcpy := !node.val_type.is_ptr() && !node.val_type.has_flag(.option) && g.table.final_sym(node.val_type).kind == .array_fixed - // If val is mutable (pointer behind the scenes), we need to generate - // `int* val = ((int*)arr.data) + i;` - // instead of - // `int* val = ((int**)arr.data)[i];` - // right := if node.val_is_mut { styp } else { styp + '*' } right := if cond_is_option { '((${styp}*)${opt_expr}${op_field}data)[${i}]' } else if node.val_is_mut || node.val_is_ref { - '((${styp})${cond_var}${op_field}data) + ${i}' + if g.table.value_type(node.cond_type).is_ptr() { + '((${styp}*)${cond_var}${op_field}data)[${i}]' + } else { + '((${styp})${cond_var}${op_field}data) + ${i}' + } } else if val_sym.kind == .array_fixed { '((${styp}*)${cond_var}${op_field}data)[${i}]' } else { @@ -443,11 +442,11 @@ fn (mut g Gen) for_in_stmt(node_ ast.ForInStmt) { g.writeln('memcpy(*(${val_styp}*)${c_name(node.val_var)}, (byte*)builtin__DenseArray_value(&${cond_var}${dot_or_ptr}key_values, ${idx}), sizeof(${val_styp}));') } else { val_styp := g.styp(node.val_type) - if node.val_type.is_ptr() { - if node.val_is_mut || node.val_is_ref { - g.write('${val_styp} ${c_name(node.val_var)} = &(*(${val_styp})') - } else { + if node.val_is_mut || node.val_is_ref { + if g.table.value_type(node.cond_type).is_ptr() { g.write('${val_styp} ${c_name(node.val_var)} = (*(${val_styp}*)') + } else { + g.write('${val_styp} ${c_name(node.val_var)} = ((${val_styp})') } } else { g.write('${val_styp} ${c_name(node.val_var)} = (*(${val_styp}*)') @@ -547,14 +546,14 @@ fn (mut g Gen) for_in_stmt(node_ ast.ForInStmt) { g.writeln('\tif (${t_var}.state != 0) break;') val := if node.val_var in ['', '_'] { g.new_tmp_var() } else { node.val_var } val_styp := g.styp(ret_typ.clear_option_and_result()) - ret_is_fixed_array := g.table.sym(ret_typ).is_array_fixed() if node.val_is_mut { - if ret_typ.has_flag(.option) { - g.writeln('\t${val_styp}* ${val} = (${val_styp}*)${t_var}.data;') + if ret_typ.is_any_kind_of_pointer() { + g.writeln('\t${val_styp} ${val} = *(${val_styp}*)${t_var}.data;') } else { - g.writeln('\t${val_styp} ${val} = (${val_styp})${t_var}.data;') + g.writeln('\t${val_styp}* ${val} = (${val_styp}*)${t_var}.data;') } } else { + ret_is_fixed_array := g.table.sym(ret_typ).is_array_fixed() if ret_is_fixed_array { g.writeln('\t${val_styp} ${val} = {0};') g.write('\tmemcpy(${val}, ${t_var}.data, sizeof(${val_styp}));') diff --git a/vlib/v/tests/loops/for_in_mut_with_ptrs_test.v b/vlib/v/tests/loops/for_in_mut_with_ptrs_test.v new file mode 100644 index 000000000..97b7eb4f4 --- /dev/null +++ b/vlib/v/tests/loops/for_in_mut_with_ptrs_test.v @@ -0,0 +1,28 @@ +struct MyStruct { +mut: + v int +} + +fn inc(mut s MyStruct) { + s.v++ +} + +fn test_for_in_mut_with_ptrs() { + mut ex1 := [&MyStruct{ + v: 1 + }] + + for mut c in ex1 { // c is **MyStruct whereas I would expect it to be *MyStruct + inc(mut *c) // Need to de-reference c + } + assert ex1[0].v == 2 + + mut ex2 := [&MyStruct{ + v: 1 + }] + + for mut c in ex2 { + inc(mut c) // We can see that we're not incrementing the correct memory location as we're incrementing the reference to c :/ + } + assert ex2[0].v == 2 +} -- 2.39.5