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.

Sign up for GitHub

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

Jump to bottom

v: fix mutable option #19100

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
+85 −13
Draft

v: fix mutable option #19100

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7d84ddf
fix
felipensp Aug 9, 2023
20ef235
fmt
felipensp Aug 9, 2023
c80ac0c
fix
felipensp Aug 20, 2023
c2cc39e
Merge remote-tracking branch 'origin/master' into fix_opt_mut
felipensp Aug 22, 2023
675ebc0
fix
felipensp Aug 22, 2023
ead3ab3
fix param validation
felipensp Sep 9, 2023
4d2d3c9
Merge remote-tracking branch 'origin/master' into fix_opt_mut
felipensp Sep 9, 2023
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
1 change: 1 addition & 0 deletions vlib/v/ast/types.v
View file
Open in desktop
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
16 changes: 13 additions & 3 deletions vlib/v/gen/c/assign.v
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -610,10 +610,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 @@ -630,7 +636,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 @@ -739,6 +746,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
View file
Open in desktop
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
13 changes: 9 additions & 4 deletions vlib/v/gen/c/cgen.v
View file
Open in desktop
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,12 @@ 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('*', '')
g.writeln('${g.typ(ret_typ).replace('*', '')} ${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 +1974,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
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2210,6 +2210,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 @@ -2250,6 +2253,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
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -913,6 +913,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 @@ -1025,6 +1028,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