From ccb70b1292e6a7a3c0863273f8e6473bcea9deb5 Mon Sep 17 00:00:00 2001 From: Felipe Pena Date: Fri, 16 Jun 2023 03:46:33 -0300 Subject: [PATCH] checker: fix missing unwrap check for option to non-option argument (#18460) --- vlib/v/checker/check_types.v | 6 ++ vlib/v/checker/comptime.v | 9 +++ vlib/v/checker/tests/option_fn_err.out | 56 +++++++++++++++++++ .../tests/option_ptr_without_unwrapp_err.out | 7 +++ .../tests/option_ptr_without_unwrapp_err.vv | 17 ++++++ vlib/v/gen/c/struct.v | 3 +- vlib/v/tests/option_unwrap_test.v | 27 +++++++++ 7 files changed, 124 insertions(+), 1 deletion(-) create mode 100644 vlib/v/checker/tests/option_ptr_without_unwrapp_err.out create mode 100644 vlib/v/checker/tests/option_ptr_without_unwrapp_err.vv create mode 100644 vlib/v/tests/option_unwrap_test.v diff --git a/vlib/v/checker/check_types.v b/vlib/v/checker/check_types.v index b43a0d0238..a5bc629621 100644 --- a/vlib/v/checker/check_types.v +++ b/vlib/v/checker/check_types.v @@ -234,6 +234,12 @@ fn (mut c Checker) check_expected_call_arg(got ast.Type, expected_ ast.Type, lan got_typ_str, expected_typ_str := c.get_string_names_of(got, expected) return error('cannot use `${got_typ_str}` as `${expected_typ_str}`') } + + if !expected.has_flag(.option) && got.has_flag(.option) && (arg.expr !is ast.Ident + || (arg.expr is ast.Ident && c.get_ct_type_var(arg.expr) != .field_var)) { + got_typ_str, expected_typ_str := c.get_string_names_of(got, expected) + return error('cannot use `${got_typ_str}` as `${expected_typ_str}`, it must be unwrapped first') + } } // check int signed/unsigned mismatch if got == ast.int_literal_type_idx && expected in ast.unsigned_integer_type_idxs diff --git a/vlib/v/checker/comptime.v b/vlib/v/checker/comptime.v index 9e2068adbb..66a78996ad 100644 --- a/vlib/v/checker/comptime.v +++ b/vlib/v/checker/comptime.v @@ -9,6 +9,15 @@ import v.token import v.util import v.pkgconfig +[inline] +fn (mut c Checker) get_ct_type_var(node ast.Expr) ast.ComptimeVarKind { + return if node is ast.Ident && (node as ast.Ident).obj is ast.Var { + (node.obj as ast.Var).ct_type_var + } else { + .no_comptime + } +} + [inline] fn (mut c Checker) get_comptime_var_type(node ast.Expr) ast.Type { if node is ast.Ident && (node as ast.Ident).obj is ast.Var { diff --git a/vlib/v/checker/tests/option_fn_err.out b/vlib/v/checker/tests/option_fn_err.out index bd06ff0b54..8c1669be64 100644 --- a/vlib/v/checker/tests/option_fn_err.out +++ b/vlib/v/checker/tests/option_fn_err.out @@ -1,3 +1,17 @@ +vlib/v/checker/tests/option_fn_err.vv:34:16: error: cannot use `?int` as `int`, it must be unwrapped first in argument 1 to `twice` + 32 | foo() + 33 | _ := bar(0) + 34 | println(twice(bar(0))) + | ~~~~~~ + 35 | + 36 | // anon fn +vlib/v/checker/tests/option_fn_err.vv:37:16: error: cannot use `?int` as `int`, it must be unwrapped first in argument 1 to `anon` + 35 | + 36 | // anon fn + 37 | fn (_ int) {}(bar(0)) + | ~~~~~~ + 38 | + 39 | // assert vlib/v/checker/tests/option_fn_err.vv:40:9: error: assert can be used only with `bool` expressions, but found `bool` instead 38 | 39 | // assert @@ -12,6 +26,20 @@ vlib/v/checker/tests/option_fn_err.vv:45:3: error: cannot assign an Option value | ~~~~~~~~~~~~~ 46 | opt: bar(0) 47 | } +vlib/v/checker/tests/option_fn_err.vv:48:8: error: cannot use `?int` as `int`, it must be unwrapped first in argument 1 to `Data.add` + 46 | opt: bar(0) + 47 | } + 48 | v.add(bar(0)) // call method + | ~~~~~~ + 49 | v.f(bar(0)) // call fn field + 50 | +vlib/v/checker/tests/option_fn_err.vv:49:6: error: cannot use `?int` as `int`, it must be unwrapped first in argument 1 to `Data.f` + 47 | } + 48 | v.add(bar(0)) // call method + 49 | v.f(bar(0)) // call fn field + | ~~~~~~ + 50 | + 51 | // array vlib/v/checker/tests/option_fn_err.vv:56:27: error: cannot use unwrapped Option as initializer 54 | // init 55 | _ := [bar(0)] @@ -26,6 +54,34 @@ vlib/v/checker/tests/option_fn_err.vv:60:13: error: cannot use Option or Result | ~~~~~~~~ 61 | // array builtin methods 62 | arr.insert(0, bar(0)) +vlib/v/checker/tests/option_fn_err.vv:62:16: error: cannot use `?int` as `voidptr`, it must be unwrapped first in argument 2 to `[]int.insert` + 60 | println(arr[bar(0)]) + 61 | // array builtin methods + 62 | arr.insert(0, bar(0)) + | ~~~~~~ + 63 | arr.prepend(bar(0)) + 64 | arr.contains(bar(0)) +vlib/v/checker/tests/option_fn_err.vv:63:14: error: cannot use `?int` as `voidptr`, it must be unwrapped first in argument 1 to `[]int.prepend` + 61 | // array builtin methods + 62 | arr.insert(0, bar(0)) + 63 | arr.prepend(bar(0)) + | ~~~~~~ + 64 | arr.contains(bar(0)) + 65 | arr.index(bar(0)) +vlib/v/checker/tests/option_fn_err.vv:64:15: error: cannot use `?int` as `int`, it must be unwrapped first in argument 1 to `.contains()` + 62 | arr.insert(0, bar(0)) + 63 | arr.prepend(bar(0)) + 64 | arr.contains(bar(0)) + | ~~~~~~ + 65 | arr.index(bar(0)) + 66 | println(arr.map(bar(0))) +vlib/v/checker/tests/option_fn_err.vv:65:12: error: cannot use `?int` as `int`, it must be unwrapped first in argument 1 to `.index()` + 63 | arr.prepend(bar(0)) + 64 | arr.contains(bar(0)) + 65 | arr.index(bar(0)) + | ~~~~~~ + 66 | println(arr.map(bar(0))) + 67 | println(arr.filter(bar(true))) vlib/v/checker/tests/option_fn_err.vv:67:21: error: type mismatch, `bar` must return a bool 65 | arr.index(bar(0)) 66 | println(arr.map(bar(0))) diff --git a/vlib/v/checker/tests/option_ptr_without_unwrapp_err.out b/vlib/v/checker/tests/option_ptr_without_unwrapp_err.out new file mode 100644 index 0000000000..9281c74cb4 --- /dev/null +++ b/vlib/v/checker/tests/option_ptr_without_unwrapp_err.out @@ -0,0 +1,7 @@ +vlib/v/checker/tests/option_ptr_without_unwrapp_err.vv:7:13: error: cannot use `?&Node` as `Node`, it must be unwrapped first in argument 1 to `set_trace` + 5 | fn set_trace(n Node) { + 6 | if n.parent != none { + 7 | set_trace(n.parent) + | ~~~~~~~~ + 8 | } + 9 | } diff --git a/vlib/v/checker/tests/option_ptr_without_unwrapp_err.vv b/vlib/v/checker/tests/option_ptr_without_unwrapp_err.vv new file mode 100644 index 0000000000..107dc02e79 --- /dev/null +++ b/vlib/v/checker/tests/option_ptr_without_unwrapp_err.vv @@ -0,0 +1,17 @@ +struct Node { + parent ?&Node +} + +fn set_trace(n Node) { + if n.parent != none { + set_trace(n.parent) + } +} + +fn main() { + mut initial_node := Node{ + parent: none + } + set_trace(initial_node) + assert true +} diff --git a/vlib/v/gen/c/struct.v b/vlib/v/gen/c/struct.v index 5e10a9f3b0..917fdd3482 100644 --- a/vlib/v/gen/c/struct.v +++ b/vlib/v/gen/c/struct.v @@ -549,7 +549,8 @@ fn (mut g Gen) struct_init_field(sfield ast.StructInitField, language ast.Langua } else { if sfield.typ != ast.voidptr_type && sfield.typ != ast.nil_type && (sfield.expected_type.is_ptr() && !sfield.expected_type.has_flag(.shared_f)) - && !(sfield.typ.is_ptr() || sfield.typ.is_pointer()) && !sfield.typ.is_number() { + && !sfield.expected_type.has_flag(.option) && !(sfield.typ.is_ptr() + || sfield.typ.is_pointer()) && !sfield.typ.is_number() { g.write('/* autoref */&') } diff --git a/vlib/v/tests/option_unwrap_test.v b/vlib/v/tests/option_unwrap_test.v new file mode 100644 index 0000000000..15fb6a843b --- /dev/null +++ b/vlib/v/tests/option_unwrap_test.v @@ -0,0 +1,27 @@ +struct Node { +pub mut: + parent ?&Node + id int +} + +fn set_trace(n &Node) int { + if n.parent != none { + set_trace(n.parent or { &Node{} }) + assert n.id != 0 + } else { + assert n.id == 1 + } + return n.id +} + +fn test_main() { + mut initial_node := &Node{ + parent: none + id: 1 + } + mut child_node := &Node{ + parent: initial_node + id: 2 + } + assert set_trace(child_node) == 2 +}