From a200c4540a5bac23f9ef5f308a2e0c079c54e12c Mon Sep 17 00:00:00 2001 From: Felipe Pena Date: Mon, 16 Dec 2024 18:17:32 -0300 Subject: [PATCH] checker,cgen: fix missing validation for selector unwrapping + fix default `return none` for unwrapping (#23183) --- vlib/v/checker/checker.v | 4 ++-- vlib/v/checker/tests/cast_to_concrete_mut_err.vv | 2 +- .../tests/option_selector_fn_unwrap_err.out | 14 ++++++++++++++ .../checker/tests/option_selector_fn_unwrap_err.vv | 13 +++++++++++++ vlib/v/gen/c/cgen.v | 6 +++++- vlib/v/tests/options/option_ptr_unwrap_test.v | 2 +- .../v/tests/options/option_selector_fn_none_test.v | 13 +++++++++++++ 7 files changed, 49 insertions(+), 5 deletions(-) create mode 100644 vlib/v/checker/tests/option_selector_fn_unwrap_err.out create mode 100644 vlib/v/checker/tests/option_selector_fn_unwrap_err.vv create mode 100644 vlib/v/tests/options/option_selector_fn_none_test.v diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index 8472b9606e..142835f85e 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -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) @@ -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) diff --git a/vlib/v/checker/tests/cast_to_concrete_mut_err.vv b/vlib/v/checker/tests/cast_to_concrete_mut_err.vv index f911d85582..2aa05f7d69 100644 --- a/vlib/v/checker/tests/cast_to_concrete_mut_err.vv +++ b/vlib/v/checker/tests/cast_to_concrete_mut_err.vv @@ -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 } diff --git a/vlib/v/checker/tests/option_selector_fn_unwrap_err.out b/vlib/v/checker/tests/option_selector_fn_unwrap_err.out new file mode 100644 index 0000000000..54f5b09dbe --- /dev/null +++ b/vlib/v/checker/tests/option_selector_fn_unwrap_err.out @@ -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 | } diff --git a/vlib/v/checker/tests/option_selector_fn_unwrap_err.vv b/vlib/v/checker/tests/option_selector_fn_unwrap_err.vv new file mode 100644 index 0000000000..e41e10b80d --- /dev/null +++ b/vlib/v/checker/tests/option_selector_fn_unwrap_err.vv @@ -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) +} diff --git a/vlib/v/gen/c/cgen.v b/vlib/v/gen/c/cgen.v index 65828c3b76..5143ec90f4 100644 --- a/vlib/v/gen/c/cgen.v +++ b/vlib/v/gen/c/cgen.v @@ -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(';') } } } diff --git a/vlib/v/tests/options/option_ptr_unwrap_test.v b/vlib/v/tests/options/option_ptr_unwrap_test.v index cf1a589976..d0334e8b47 100644 --- a/vlib/v/tests/options/option_ptr_unwrap_test.v +++ b/vlib/v/tests/options/option_ptr_unwrap_test.v @@ -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 { diff --git a/vlib/v/tests/options/option_selector_fn_none_test.v b/vlib/v/tests/options/option_selector_fn_none_test.v new file mode 100644 index 0000000000..72c4fe26bc --- /dev/null +++ b/vlib/v/tests/options/option_selector_fn_none_test.v @@ -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 +}