From e4e568945e44f2ce1a440712407aeb7dd9a7274e Mon Sep 17 00:00:00 2001 From: Felipe Pena Date: Thu, 5 Jun 2025 07:39:38 -0300 Subject: [PATCH] v: fix mutable option (fix #18818) (fix #24622) (fix #24101) (#19100) --- vlib/v/ast/types.v | 13 ++--- vlib/v/gen/c/assign.v | 23 ++++++-- vlib/v/gen/c/auto_str_methods.v | 13 +++-- vlib/v/gen/c/cgen.v | 53 +++++++++++++++---- vlib/v/gen/c/dumpexpr.v | 3 ++ vlib/v/gen/c/fn.v | 14 ++++- vlib/v/gen/c/if.v | 14 ++++- vlib/v/gen/c/infix.v | 6 ++- vlib/v/gen/c/str.v | 6 ++- vlib/v/gen/c/utils.v | 2 + vlib/v/parser/fn.v | 6 +++ .../options/option_mut_generic_array_test.v | 12 +++++ vlib/v/tests/options/option_mut_param_test.v | 23 ++++++++ .../options/option_mut_struct_init_test.v | 39 ++++++++++++++ 14 files changed, 199 insertions(+), 28 deletions(-) create mode 100644 vlib/v/tests/options/option_mut_generic_array_test.v create mode 100644 vlib/v/tests/options/option_mut_param_test.v create mode 100644 vlib/v/tests/options/option_mut_struct_init_test.v diff --git a/vlib/v/ast/types.v b/vlib/v/ast/types.v index 1ba6ae6bd..e117ee386 100644 --- a/vlib/v/ast/types.v +++ b/vlib/v/ast/types.v @@ -128,12 +128,13 @@ pub mut: // max of 8 pub enum TypeFlag as u32 { - option = 1 << 24 - result = 1 << 25 - variadic = 1 << 26 - generic = 1 << 27 - shared_f = 1 << 28 - atomic_f = 1 << 29 + option = 1 << 24 + result = 1 << 25 + variadic = 1 << 26 + generic = 1 << 27 + shared_f = 1 << 28 + atomic_f = 1 << 29 + option_mut_param_t = 1 << 30 } /* diff --git a/vlib/v/gen/c/assign.v b/vlib/v/gen/c/assign.v index 938855700..e29e2df67 100644 --- a/vlib/v/gen/c/assign.v +++ b/vlib/v/gen/c/assign.v @@ -25,7 +25,8 @@ fn (mut g Gen) expr_with_opt_or_block(expr ast.Expr, expr_typ ast.Type, var_expr } else { '${expr}' } - g.writeln('if (${c_name(expr_var)}.state != 0) { // assign') + dot_or_ptr := if !expr_typ.has_flag(.option_mut_param_t) { '.' } else { '-> ' } + g.writeln('if (${c_name(expr_var)}${dot_or_ptr}state != 0) { // assign') if expr is ast.Ident && expr.or_expr.kind == .propagate_option { g.writeln('\tpanic_option_not_set(_S("none"));') } else { @@ -832,10 +833,20 @@ fn (mut g Gen) assign_stmt(node_ ast.AssignStmt) { if op_overloaded { g.op_arg(left, op_expected_left, var_type) } else { - if !is_decl && !is_shared_re_assign && left.is_auto_deref_var() { + if !is_decl && !is_shared_re_assign && left.is_auto_deref_var() + && !var_type.has_flag(.option) { g.write('*') } - g.expr(left) + if node_.op == .assign && var_type.has_flag(.option_mut_param_t) { + g.write('memcpy(&') + g.expr(left) + g.write('->data, *(${g.styp(val_type)}**)&') + } else if var_type.has_flag(.option_mut_param_t) { + g.expr(left) + g.write(' = ') + } else { + g.expr(left) + } if !is_decl && var_type.has_flag(.shared_f) { g.write('->val') // don't reset the mutex, just change the value } @@ -858,7 +869,8 @@ fn (mut g Gen) assign_stmt(node_ ast.AssignStmt) { continue } } - } else if cur_indexexpr == -1 && !str_add && !op_overloaded { + } else if !var_type.has_flag(.option_mut_param_t) && cur_indexexpr == -1 && !str_add + && !op_overloaded { g.write(' ${op} ') } else if str_add || op_overloaded { g.write(', ') @@ -998,6 +1010,9 @@ fn (mut g Gen) assign_stmt(node_ ast.AssignStmt) { if str_add || op_overloaded { g.write(')') } + if node_.op == .assign && var_type.has_flag(.option_mut_param_t) { + g.write('.data, sizeof(${g.base_type(val_type)}))') + } if cur_indexexpr != -1 { g.cur_indexexpr.delete(cur_indexexpr) g.write(' })') diff --git a/vlib/v/gen/c/auto_str_methods.v b/vlib/v/gen/c/auto_str_methods.v index 966a0afc6..080796f67 100644 --- a/vlib/v/gen/c/auto_str_methods.v +++ b/vlib/v/gen/c/auto_str_methods.v @@ -129,7 +129,8 @@ fn (mut g Gen) final_gen_str(typ StrType) { g.str_fn_names << str_fn_name } if typ.typ.has_flag(.option) { - g.gen_str_for_option(typ.typ, styp, str_fn_name) + opt_typ := if typ.typ.has_flag(.option_mut_param_t) { styp.replace('*', '') } else { styp } + g.gen_str_for_option(typ.typ, opt_typ, str_fn_name) return } if typ.typ.has_flag(.result) { @@ -202,9 +203,15 @@ fn (mut g Gen) gen_str_for_option(typ ast.Type, styp string, str_fn_name string) g.auto_str_funcs.writeln('string indent_${str_fn_name}(${styp} it, int indent_count) {') g.auto_str_funcs.writeln('\tstring res;') g.auto_str_funcs.writeln('\tif (it.state == 0) {') - deref := if typ.is_ptr() { - dot := if expects_ptr { '*'.repeat(typ.nr_muls()) } else { '*'.repeat(typ.nr_muls() + 1) } + deref := if typ.is_ptr() && !typ.has_flag(.option_mut_param_t) { + dot := if expects_ptr { + '*'.repeat(typ.nr_muls()) + } else { + '*'.repeat(typ.nr_muls() + 1) + } '${dot}(${sym.cname}**)&' + } else if typ.has_flag(.option_mut_param_t) { + '*(${sym.cname}*)' } else if expects_ptr { '(${sym.cname}*)' } else { diff --git a/vlib/v/gen/c/cgen.v b/vlib/v/gen/c/cgen.v index 982172191..dc9ba569d 100644 --- a/vlib/v/gen/c/cgen.v +++ b/vlib/v/gen/c/cgen.v @@ -1396,7 +1396,7 @@ fn (g &Gen) result_type_text(styp string, base string) string { fn (mut g Gen) register_option(t ast.Type) string { styp, base := g.option_type_name(t) g.options[base] = styp - return styp + return if !t.has_flag(.option_mut_param_t) { styp } else { '${styp}*' } } fn (mut g Gen) register_result(t ast.Type) string { @@ -2294,7 +2294,12 @@ fn (mut g Gen) expr_with_tmp_var(expr ast.Expr, expr_typ ast.Type, ret_typ ast.T ret_styp := g.styp(unwrapped_ret_typ).replace('*', '_ptr') g.writeln('${ret_styp} ${tmp_var};') } else { - g.writeln('${g.styp(ret_typ)} ${tmp_var};') + if ret_typ.has_flag(.option_mut_param_t) { + ret_styp := g.styp(ret_typ).replace('*', '') + g.writeln('${ret_styp} ${tmp_var};') + } else { + g.writeln('${g.styp(ret_typ)} ${tmp_var};') + } } mut expr_is_fixed_array_var := false mut fn_option_clone := false @@ -2336,8 +2341,21 @@ fn (mut g Gen) expr_with_tmp_var(expr ast.Expr, expr_typ ast.Type, ret_typ ast.T } } if !expr.is_literal() && expr_typ != ast.nil_type - && ret_typ.nr_muls() > expr_typ.nr_muls() { + && ret_typ.nr_muls() > expr_typ.nr_muls() + && !ret_typ.has_flag(.option_mut_param_t) { g.write('&'.repeat(ret_typ.nr_muls() - expr_typ.nr_muls())) + } else if ret_typ.has_flag(.option_mut_param_t) { + if expr_typ.is_ptr() { + if ret_typ.nr_muls() < expr_typ.nr_muls() { + g.write('*') + } + } else { + if expr_typ.has_flag(.option) { + fn_option_clone = true + g.write('(${styp})') + } + g.write('&') + } } } } else { @@ -3789,6 +3807,7 @@ fn (mut g Gen) expr(node_ ast.Expr) { cur_line := g.go_before_last_stmt().trim_space() mut expr_str := '' mut is_unwrapped := true + mut dot_or_ptr := '.' if mut node.expr is ast.ComptimeSelector && node.expr.left is ast.Ident { // val.$(field.name)? expr_str = g.gen_comptime_selector(node.expr) @@ -3796,8 +3815,13 @@ fn (mut g Gen) expr(node_ ast.Expr) { // val? expr_str = node.expr.name is_unwrapped = !g.inside_assign + dot_or_ptr = if !(node.expr.obj is ast.Var && node.expr.obj.is_auto_deref) { + '.' + } else { + '->' + } } - g.writeln('if (${expr_str}.state != 0) {') + g.writeln('if (${expr_str}${dot_or_ptr}state != 0) {') g.writeln2('\tpanic_option_not_set(_S("none"));', '}') g.write(cur_line) if is_unwrapped { @@ -5199,8 +5223,14 @@ fn (mut g Gen) ident(node ast.Ident) { if !g.is_assign_lhs && is_auto_heap { g.write('(*${name})') } else { - if node.obj is ast.Var && node.obj.is_inherited { - g.write(closure_ctx + '->') + if node.obj is ast.Var { + // mutable option var + if (g.is_assign_lhs || g.inside_struct_init) && node.obj.is_auto_deref { + g.write('*') + } + if node.obj.is_inherited { + g.write(closure_ctx + '->') + } } g.write(name) } @@ -5352,8 +5382,9 @@ fn (mut g Gen) ident(node ast.Ident) { } } if i == 0 && node.obj.ct_type_var != .smartcast && node.obj.is_unwrapped { - dot := if !node.obj.ct_type_unwrapped && !node.obj.orig_type.is_ptr() - && obj_sym.is_heap() { + dot := if (!node.obj.ct_type_unwrapped && !node.obj.orig_type.is_ptr() + && obj_sym.is_heap()) + || node.obj.orig_type.has_flag(.option_mut_param_t) { '->' } else { '.' @@ -7044,7 +7075,11 @@ fn (mut g Gen) gen_or_block_stmts(cvar_name string, cast_typ string, stmts []ast // Returns the type of the last stmt fn (mut g Gen) or_block(var_name string, or_block ast.OrExpr, return_type ast.Type) { cvar_name := c_name(var_name) - tmp_op := if var_name in g.tmp_var_ptr { '->' } else { '.' } + tmp_op := if var_name in g.tmp_var_ptr || return_type.has_flag(.option_mut_param_t) { + '->' + } else { + '.' + } if or_block.kind == .block && or_block.stmts.len == 0 { // generate nothing, block is empty g.write(';\n${util.tabs(g.indent)}(void)${cvar_name};') diff --git a/vlib/v/gen/c/dumpexpr.v b/vlib/v/gen/c/dumpexpr.v index 3fb80afe7..3774150fd 100644 --- a/vlib/v/gen/c/dumpexpr.v +++ b/vlib/v/gen/c/dumpexpr.v @@ -82,6 +82,9 @@ fn (mut g Gen) dump_expr(node ast.DumpExpr) { } else { old_inside_opt_or_res := g.inside_opt_or_res g.inside_opt_or_res = true + if expr_type.has_flag(.option_mut_param_t) { + g.write('*') + } g.expr(node.expr) g.inside_opt_or_res = old_inside_opt_or_res } diff --git a/vlib/v/gen/c/fn.v b/vlib/v/gen/c/fn.v index 5b1bf6bf4..cc48304ed 100644 --- a/vlib/v/gen/c/fn.v +++ b/vlib/v/gen/c/fn.v @@ -2631,7 +2631,11 @@ fn (mut g Gen) ref_or_deref_arg(arg ast.CallArg, expected_type ast.Type, lang as if (arg.expr is ast.Ident && arg.expr.kind in [.global, .variable]) || arg.expr is ast.SelectorExpr { g.write('&') - g.expr(arg.expr) + if expected_type.has_flag(.option_mut_param_t) { + g.expr_with_opt(arg.expr, arg_typ, expected_type) + } else { + g.expr(arg.expr) + } } else { // Special case for mutable arrays. We can't `&` function // results, have to use `(array[]){ expr }[0]` hack. @@ -2667,6 +2671,9 @@ fn (mut g Gen) ref_or_deref_arg(arg ast.CallArg, expected_type ast.Type, lang as && lang != .c { if arg.expr.is_lvalue() { if expected_type.has_flag(.option) { + if expected_type.has_flag(.option_mut_param_t) { + g.write('(${g.styp(expected_type)})&') + } g.expr_with_opt(arg.expr, arg_typ, expected_type) return } else if arg.expr is ast.Ident && arg.expr.language == .c { @@ -2729,6 +2736,11 @@ fn (mut g Gen) ref_or_deref_arg(arg ast.CallArg, expected_type ast.Type, lang as g.write('->val') return } else if expected_type.has_flag(.option) { + if expected_type.has_flag(.option_mut_param_t) + && arg_typ.nr_muls() <= expected_type.nr_muls() && !(arg.expr is ast.Ident + && (arg.expr.obj is ast.Var && arg.expr.obj.is_inherited)) { + g.write('&') + } if (arg_sym.info is ast.Alias || exp_sym.info is ast.Alias) && expected_type != arg_typ { g.expr_opt_with_alias(arg.expr, arg_typ, expected_type) } else { diff --git a/vlib/v/gen/c/if.v b/vlib/v/gen/c/if.v index 89f1f0ec1..4305bdcc7 100644 --- a/vlib/v/gen/c/if.v +++ b/vlib/v/gen/c/if.v @@ -330,7 +330,12 @@ fn (mut g Gen) if_expr(node ast.IfExpr) { g.write('if (${var_name} = ') g.expr(branch.cond.expr) if branch.cond.expr_type.has_flag(.option) { - g.writeln(', ${var_name}.state == 0) {') + dot_or_ptr := if !branch.cond.expr_type.has_flag(.option_mut_param_t) { + '.' + } else { + '-> ' + } + g.writeln(', ${var_name}${dot_or_ptr}state == 0) {') } else if branch.cond.expr_type.has_flag(.result) { g.writeln(', !${var_name}.is_error) {') } @@ -366,7 +371,12 @@ fn (mut g Gen) if_expr(node ast.IfExpr) { if is_auto_heap { g.writeln('\t${base_type}* ${left_var_name} = HEAP(${base_type}, *(${base_type}*)${var_name}.data);') } else { - g.writeln('\t${base_type} ${left_var_name} = *(${base_type}*)${var_name}.data;') + dot_or_ptr := if !branch.cond.expr_type.has_flag(.option_mut_param_t) { + '.' + } else { + '-> ' + } + g.writeln('\t${base_type} ${left_var_name} = *(${base_type}*)${var_name}${dot_or_ptr}data;') } } else if branch.cond.vars.len > 1 { sym := g.table.sym(branch.cond.expr_type) diff --git a/vlib/v/gen/c/infix.v b/vlib/v/gen/c/infix.v index c495c43f6..2605c6cf2 100644 --- a/vlib/v/gen/c/infix.v +++ b/vlib/v/gen/c/infix.v @@ -1174,14 +1174,16 @@ fn (mut g Gen) gen_is_none_check(node ast.InfixExpr) { g.expr(node.left) g.write(')') g.inside_opt_or_res = old_inside_opt_or_res - g.write('.state') + dot_or_ptr := if !node.left_type.has_flag(.option_mut_param_t) { '.' } else { '->' } + g.write('${dot_or_ptr}state') } else { stmt_str := g.go_before_last_stmt().trim_space() g.empty_line = true left_var := g.expr_with_opt(node.left, node.left_type, node.left_type) g.writeln(';') g.write2(stmt_str, ' ') - g.write('${left_var}.state') + dot_or_ptr := if !node.left_type.has_flag(.option_mut_param_t) { '.' } else { '->' } + g.write('${left_var}${dot_or_ptr}state') } g.write(' ${node.op.str()} 2') // none state } diff --git a/vlib/v/gen/c/str.v b/vlib/v/gen/c/str.v index 3b178c716..f8e1ba005 100644 --- a/vlib/v/gen/c/str.v +++ b/vlib/v/gen/c/str.v @@ -167,7 +167,11 @@ fn (mut g Gen) gen_expr_to_string(expr ast.Expr, etype ast.Type) { g.write('&') } } else if is_ptr && typ.has_flag(.option) { - g.write('*(${g.styp(typ)}*)&') + if typ.has_flag(.option_mut_param_t) { + g.write('*') + } else { + g.write('*(${g.styp(typ)}*)&') + } } else if !str_method_expects_ptr && !is_shared && (is_ptr || is_var_mut) { if sym.is_c_struct() { g.write(c_struct_ptr(sym, typ, str_method_expects_ptr)) diff --git a/vlib/v/gen/c/utils.v b/vlib/v/gen/c/utils.v index d41811164..eaa870dff 100644 --- a/vlib/v/gen/c/utils.v +++ b/vlib/v/gen/c/utils.v @@ -162,6 +162,8 @@ fn (mut g Gen) unwrap_option_type(typ ast.Type, name string, is_auto_heap bool) if parent_typ.has_flag(.option) { g.write('.data)') } + } else if typ.has_flag(.option_mut_param_t) { + g.write('(*(${styp}*)${name}->data)') } else { g.write('(*(${styp}*)${name}.data)') } diff --git a/vlib/v/parser/fn.v b/vlib/v/parser/fn.v index 0c12700b7..24b658093 100644 --- a/vlib/v/parser/fn.v +++ b/vlib/v/parser/fn.v @@ -1007,6 +1007,9 @@ fn (mut p Parser) fn_params() ([]ast.Param, bool, bool, bool) { } else { param_type = param_type.set_nr_muls(1) } + if param_type.has_flag(.option) { + param_type = param_type.set_flag(.option_mut_param_t) + } if is_shared { param_type = param_type.set_flag(.shared_f) } @@ -1135,6 +1138,9 @@ fn (mut p Parser) fn_params() ([]ast.Param, bool, bool, bool) { } else { typ = typ.set_nr_muls(1) } + if typ.has_flag(.option) { + typ = typ.set_flag(.option_mut_param_t) + } if is_shared { typ = typ.set_flag(.shared_f) } diff --git a/vlib/v/tests/options/option_mut_generic_array_test.v b/vlib/v/tests/options/option_mut_generic_array_test.v new file mode 100644 index 000000000..59fabaac2 --- /dev/null +++ b/vlib/v/tests/options/option_mut_generic_array_test.v @@ -0,0 +1,12 @@ +fn test_main() { + mut arr := [1, 2] + mut bbb := unwrap(mut arr) + bbb << 3 + dump(bbb) + assert bbb == [1, 2, 3] + assert arr == [1, 2] +} + +fn unwrap[T](mut t ?&T) T { + return t or { panic('unexpected `none`') } +} diff --git a/vlib/v/tests/options/option_mut_param_test.v b/vlib/v/tests/options/option_mut_param_test.v new file mode 100644 index 000000000..6fb858b5a --- /dev/null +++ b/vlib/v/tests/options/option_mut_param_test.v @@ -0,0 +1,23 @@ +struct Abc { + a int +} + +fn foo(mut baz ?Abc) { + baz = Abc{ + a: 3 + } + println(baz) + dump(baz) +} + +fn test_main() { + mut a := ?Abc{ + a: 2 + } + assert a?.a == 2 + dump(a) + foo(mut a) + println('--') + dump(a) + assert a?.a == 3 +} diff --git a/vlib/v/tests/options/option_mut_struct_init_test.v b/vlib/v/tests/options/option_mut_struct_init_test.v new file mode 100644 index 000000000..8357ce491 --- /dev/null +++ b/vlib/v/tests/options/option_mut_struct_init_test.v @@ -0,0 +1,39 @@ +struct Bar { +mut: + a int +} + +struct Foo { + field ?&Bar +} + +fn t(mut opt ?Bar) { + v := Foo{ + field: opt + } + if opt == none { + assert opt == none + assert v.field == none + } else { + assert opt.a == 123 + assert v.field != none + } + if mut opt != none { + opt.a = 321 + } +} + +fn test_main() { + mut var := ?&Bar(none) + t(mut var) + assert var == none +} + +fn test_not_none() { + mut var := ?&Bar(&Bar{ + a: 123 + }) + t(mut var) + assert var != none + assert var?.a == 321 +} -- 2.39.5