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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

v: fix mutable option #19100

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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion vlib/v/ast/types.v
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ pub enum TypeFlag {
generic
shared_f
atomic_f
option_mut_param_t
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you really need that? mut is usually just a modifier, not something that is part of the type.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TypeFlags are limited to just 3 bits, i.e. just 8 combinations :-|

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joe-conigliaro can you please also review this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you really need that? mut is usually just a modifier, not something that is part of the type.

Yes. I was thinking a way to determine when an option ptr must be used as struct _option_*_ptr or a real pointer.

}

/*
Expand Down Expand Up @@ -1401,7 +1402,7 @@ pub fn (t &Table) type_to_str_using_aliases(typ Type, import_aliases map[string]
nr_muls--
res = 'atomic ' + res
}
if nr_muls > 0 && !typ.has_flag(.variadic) {
if nr_muls > 0 && !typ.has_flag(.variadic) && !typ.has_flag(.option_mut_param_t) {
res = strings.repeat(`&`, nr_muls) + res
}
if typ.has_flag(.option) {
Expand Down
6 changes: 5 additions & 1 deletion vlib/v/checker/check_types.v
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,11 @@ fn (mut c Checker) check_expected_call_arg(got ast.Type, expected_ ast.Type, lan
if language != .v || expected.is_ptr() == got.is_ptr() || arg.is_mut
|| arg.expr.is_auto_deref_var() || got.has_flag(.shared_f)
|| c.table.sym(expected_).kind !in [.array, .map] {
return
if !expected.has_flag(.option) || !arg.is_mut
|| (arg.is_mut && expected.has_flag(.option)
&& (expected.has_flag(.option_mut_param_t) || (expected.is_ptr() && got.is_ptr()))) {
return
}
}
} else {
got_typ_sym := c.table.sym(c.unwrap_generic(got))
Expand Down
16 changes: 13 additions & 3 deletions vlib/v/gen/c/assign.v
Original file line number Diff line number Diff line change
Expand Up @@ -611,10 +611,16 @@ 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 && left.is_auto_deref_var() {
if !is_decl && left.is_auto_deref_var() && !var_type.has_flag(.option) {
g.write('*')
}
g.expr(left)
if var_type.has_flag(.option_mut_param_t) {
g.write('memcpy(&')
g.expr(left)
g.write('->data, &')
} 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
}
Expand All @@ -631,7 +637,8 @@ fn (mut g Gen) assign_stmt(node_ ast.AssignStmt) {
if is_decl {
g.writeln(';')
}
} else if !g.is_arraymap_set && !str_add && !op_overloaded {
} else if !var_type.has_flag(.option_mut_param_t) && !g.is_arraymap_set && !str_add
&& !op_overloaded {
g.write(' ${op} ')
} else if str_add || op_overloaded {
g.write(', ')
Expand Down Expand Up @@ -740,6 +747,9 @@ fn (mut g Gen) assign_stmt(node_ ast.AssignStmt) {
if str_add || op_overloaded {
g.write(')')
}
if var_type.has_flag(.option_mut_param_t) {
g.write('.data, sizeof(${g.base_type(val_type)}))')
}
if g.is_arraymap_set {
g.write(' })')
g.is_arraymap_set = false
Expand Down
9 changes: 7 additions & 2 deletions vlib/v/gen/c/auto_str_methods.v
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,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) {
Expand Down Expand Up @@ -185,7 +186,11 @@ 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() { '**(${sym.cname}**)&' } else { '*(${sym.cname}*)' }
deref := if typ.is_ptr() && !typ.has_flag(.option_mut_param_t) {
'**(${sym.cname}**)&'
} else {
'*(${sym.cname}*)'
}
if sym.kind == .string {
if typ.nr_muls() > 1 {
g.auto_str_funcs.writeln('\t\tres = ptr_str(*(${sym.cname}**)&it.data);')
Expand Down
14 changes: 10 additions & 4 deletions vlib/v/gen/c/cgen.v
Original file line number Diff line number Diff line change
Expand Up @@ -1076,7 +1076,7 @@ fn (mut g Gen) option_type_name(t ast.Type) (string, string) {
} else {
styp = '${c.option_name}_${base}'
}
if t.is_ptr() {
if t.is_ptr() && !t.has_flag(.option_mut_param_t) {
styp = styp.replace('*', '_ptr')
}
return styp, base
Expand Down Expand Up @@ -1160,7 +1160,7 @@ fn (mut g Gen) write_options() {
done = g.done_options.clone()
}
for base, styp in g.options {
if base in done {
if base in done || styp.ends_with('*') {
continue
}
done << base
Expand Down Expand Up @@ -1947,7 +1947,13 @@ fn (mut g Gen) expr_with_tmp_var(expr ast.Expr, expr_typ ast.Type, ret_typ ast.T
ret_styp := g.typ(g.unwrap_generic(ret_typ)).replace('*', '_ptr')
g.writeln('${ret_styp} ${tmp_var};')
} else {
g.writeln('${g.typ(ret_typ)} ${tmp_var};')
if ret_typ.has_flag(.option_mut_param_t) {
styp = styp.replace('*', '')
ret_styp := g.typ(ret_typ).replace('*', '')
g.writeln('${ret_styp} ${tmp_var};')
} else {
g.writeln('${g.typ(ret_typ)} ${tmp_var};')
}
}
if ret_typ.has_flag(.option) {
if expr_typ.has_flag(.option) && expr in [ast.StructInit, ast.ArrayInit, ast.MapInit] {
Expand All @@ -1969,7 +1975,7 @@ fn (mut g Gen) expr_with_tmp_var(expr ast.Expr, expr_typ ast.Type, ret_typ ast.T
} else {
g.write('_option_ok(&(${styp}[]) { ')
}
if !expr_typ.is_ptr() && ret_typ.is_ptr() {
if !expr_typ.is_ptr() && ret_typ.is_ptr() && !ret_typ.has_flag(.option_mut_param_t) {
g.write('&/*ref*/')
}
}
Expand Down
9 changes: 8 additions & 1 deletion vlib/v/gen/c/dumpexpr.v
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,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
}
Expand Down Expand Up @@ -106,7 +109,11 @@ fn (mut g Gen) dump_expr_definitions() {
} else {
if typ.has_flag(.option) {
str_dumparg_type += '_option_'
ptr_asterisk = ptr_asterisk.replace('*', '_ptr')
if typ.has_flag(.option_mut_param_t) {
ptr_asterisk = ptr_asterisk.replace('*', '')
} else {
ptr_asterisk = ptr_asterisk.replace('*', '_ptr')
}
}
str_dumparg_type += g.cc_type(dump_type, true) + ptr_asterisk
}
Expand Down
6 changes: 6 additions & 0 deletions vlib/v/gen/c/fn.v
Original file line number Diff line number Diff line change
Expand Up @@ -2232,6 +2232,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('&/*opt-mut*/')
}
g.expr_with_opt(arg.expr, arg_typ, expected_type)
return
} else {
Expand Down Expand Up @@ -2272,6 +2275,9 @@ 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) {
g.write('&/*opt-mut*/')
}
g.expr_with_opt(arg.expr, arg_typ, expected_type)
return
} else if arg.expr is ast.ArrayInit {
Expand Down
6 changes: 5 additions & 1 deletion vlib/v/gen/c/str.v
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,11 @@ fn (mut g Gen) gen_expr_to_string(expr ast.Expr, etype ast.Type) {
if str_method_expects_ptr && !is_ptr {
g.write('&')
} else if is_ptr && typ.has_flag(.option) {
g.write('*(${g.typ(typ)}*)&')
if typ.has_flag(.option_mut_param_t) {
g.write('*')
} else {
g.write('*(${g.typ(typ)}*)&')
}
} else if !str_method_expects_ptr && !is_shared && (is_ptr || is_var_mut) {
g.write('*'.repeat(typ.nr_muls()))
}
Expand Down
6 changes: 6 additions & 0 deletions vlib/v/parser/fn.v
Original file line number Diff line number Diff line change
Expand Up @@ -914,6 +914,9 @@ fn (mut p Parser) fn_params() ([]ast.Param, bool, bool) {
// p.error('cannot mut')
// }
// arg_type = arg_type.ref()
if param_type.has_flag(.option) && param_type.nr_muls() == 0 {
param_type = param_type.set_flag(.option_mut_param_t)
}
param_type = param_type.set_nr_muls(1)
spytheman marked this conversation as resolved.
Show resolved Hide resolved
if is_shared {
param_type = param_type.set_flag(.shared_f)
Expand Down Expand Up @@ -1026,6 +1029,9 @@ fn (mut p Parser) fn_params() ([]ast.Param, bool, bool) {
pos)
return []ast.Param{}, false, false
}
if typ.has_flag(.option) && typ.nr_muls() == 0 {
typ = typ.set_flag(.option_mut_param_t)
}
typ = typ.set_nr_muls(1)
if is_shared {
typ = typ.set_flag(.shared_f)
Expand Down
23 changes: 23 additions & 0 deletions vlib/v/tests/option_mut_param_test.v
Original file line number Diff line number Diff line change
@@ -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
}