Skip to content

Commit

Permalink
checker,cgen: fix missing validation for selector unwrapping + fix de…
Browse files Browse the repository at this point in the history
…fault `return none` for unwrapping (vlang#23183)
  • Loading branch information
felipensp authored Dec 16, 2024
1 parent 75ff2e0 commit a200c45
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 5 deletions.
4 changes: 2 additions & 2 deletions vlib/v/checker/checker.v
Original file line number Diff line number Diff line change
Expand Up @@ -1412,7 +1412,7 @@ fn (mut c Checker) check_or_expr(node ast.OrExpr, ret_type ast.Type, expr_return
node.pos)
}
}
if expr !is ast.Ident && !expr_return_type.has_flag(.option) {
if expr !in [ast.Ident, ast.SelectorExpr] && !expr_return_type.has_flag(.option) {
if expr_return_type.has_flag(.result) {
c.error('propagating a Result like an Option is deprecated, use `foo()!` instead of `foo()?`',
node.pos)
Expand Down Expand Up @@ -1791,7 +1791,7 @@ fn (mut c Checker) selector_expr(mut node ast.SelectorExpr) ast.Type {
}
}
node.typ = field.typ
if node.or_block.kind == .block {
if node.or_block.kind != .absent {
unwrapped_typ := c.unwrap_generic(node.typ)
c.expected_or_type = unwrapped_typ.clear_option_and_result()
c.stmts_ending_with_expression(mut node.or_block.stmts, c.expected_or_type)
Expand Down
2 changes: 1 addition & 1 deletion vlib/v/checker/tests/cast_to_concrete_mut_err.vv
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ pub fn (mut m Message) update_text(new_text string) {
m.text = new_text
}

pub fn (mut m Message) update_ext_text(new_text string) {
pub fn (mut m Message) update_ext_text(new_text string) ? {
m.m2?.text = new_text
}

Expand Down
14 changes: 14 additions & 0 deletions vlib/v/checker/tests/option_selector_fn_unwrap_err.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
vlib/v/checker/tests/option_selector_fn_unwrap_err.vv:7:17: error: to propagate the call, `callback` must return an Option type
5 |
6 | fn callback(foo &Foo) bool {
7 | return foo.func? == callback
| ^
8 | }
9 |
Details: vlib/v/checker/tests/option_selector_fn_unwrap_err.vv:6:23: details: prepend ? before the declaration of the return type of `callback`
4 | }
5 |
6 | fn callback(foo &Foo) bool {
| ~~~~
7 | return foo.func? == callback
8 | }
13 changes: 13 additions & 0 deletions vlib/v/checker/tests/option_selector_fn_unwrap_err.vv
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
struct Foo {
mut:
func ?fn (voidptr) bool
}

fn callback(foo &Foo) bool {
return foo.func? == callback
}

fn main() {
t := Foo{}
assert callback(&t)
}
6 changes: 5 additions & 1 deletion vlib/v/gen/c/cgen.v
Original file line number Diff line number Diff line change
Expand Up @@ -7041,11 +7041,15 @@ fn (mut g Gen) or_block(var_name string, or_block ast.OrExpr, return_type ast.Ty
// Now that option types are distinct we need a cast here
if g.fn_decl == unsafe { nil } || g.fn_decl.return_type == ast.void_type {
g.writeln('\treturn;')
} else {
} else if g.fn_decl.return_type.clear_option_and_result() == return_type.clear_option_and_result() {
styp := g.styp(g.fn_decl.return_type).replace('*', '_ptr')
err_obj := g.new_tmp_var()
g.writeln2('\t${styp} ${err_obj};', '\tmemcpy(&${err_obj}, &${cvar_name}, sizeof(_option));')
g.writeln('\treturn ${err_obj};')
} else {
g.write('\treturn ')
g.gen_option_error(g.fn_decl.return_type, ast.None{})
g.writeln(';')
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion vlib/v/tests/options/option_ptr_unwrap_test.v
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ fn new_linked_list[T]() &LinkedList[T] {
return &LinkedList[T]{}
}

fn (mut li LinkedList[T]) add[T](data T) {
fn (mut li LinkedList[T]) add[T](data T) ? {
mut node := &Node[T]{data, none, none}

if li.head == none {
Expand Down
13 changes: 13 additions & 0 deletions vlib/v/tests/options/option_selector_fn_none_test.v
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
struct Foo {
mut:
func ?fn (voidptr) ?bool
}

fn callback(foo &Foo) ?bool {
return foo.func? == callback
}

fn test_main() {
t := Foo{}
assert callback(&t) == none
}

0 comments on commit a200c45

Please sign in to comment.