From 3d302a6dad1d1d9ac7b9492ffda488c762e766e4 Mon Sep 17 00:00:00 2001 From: Felipe Pena Date: Tue, 19 Nov 2024 03:41:50 -0300 Subject: [PATCH] v: add selector option unwrapping inside `if tree.root != none {` (#22895) --- vlib/v/checker/checker.v | 5 +- vlib/v/checker/if.v | 3 +- ...assign_type_mismatch_with_generics_err.out | 7 +++ .../tests/option_ptr_without_unwrapp_err.out | 12 ++--- .../tests/option_ptr_without_unwrapp_err.vv | 4 +- vlib/v/gen/c/cgen.v | 16 +++++-- .../generics/generics_chans_select_test.v | 4 +- .../options/option_selector_unwrap_test.v | 31 ++++++++++++ .../option_selector_unwrap_types_test.v | 47 +++++++++++++++++++ vlib/v/tests/options/option_unwrap_test.v | 2 +- 10 files changed, 113 insertions(+), 18 deletions(-) create mode 100644 vlib/v/tests/options/option_selector_unwrap_test.v create mode 100644 vlib/v/tests/options/option_selector_unwrap_types_test.v diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index 6c03c5cf98033c..9fcadefba49685 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -1711,7 +1711,7 @@ fn (mut c Checker) selector_expr(mut node ast.SelectorExpr) ast.Type { if field.is_deprecated && is_used_outside { c.deprecate('field', field_name, field.attrs, node.pos) } - if field_sym.kind in [.sum_type, .interface] { + if field_sym.kind in [.sum_type, .interface] || field.typ.has_flag(.option) { if !prevent_sum_type_unwrapping_once { if scope_field := node.scope.find_struct_field(node.expr.str(), typ, field_name) { return scope_field.smartcasts.last() @@ -4204,7 +4204,8 @@ fn (mut c Checker) smartcast(mut expr ast.Expr, cur_type ast.Type, to_type_ ast. smartcasts << field.smartcasts } // smartcast either if the value is immutable or if the mut argument is explicitly given - if !is_mut || expr.is_mut { + if !is_mut || expr.is_mut + || (cur_type.has_flag(.option) && cur_type.clear_flag(.option) == to_type_) { smartcasts << to_type scope.register_struct_field(expr.expr.str(), ast.ScopeStructField{ struct_type: expr.expr_type diff --git a/vlib/v/checker/if.v b/vlib/v/checker/if.v index 95ef6a757dc5d2..aaa8884e65874d 100644 --- a/vlib/v/checker/if.v +++ b/vlib/v/checker/if.v @@ -568,7 +568,8 @@ fn (mut c Checker) smartcast_if_conds(mut node ast.Expr, mut scope ast.Scope, co if node.op == .and { c.smartcast_if_conds(mut node.left, mut scope, control_expr) c.smartcast_if_conds(mut node.right, mut scope, control_expr) - } else if node.left is ast.Ident && node.op == .ne && node.right is ast.None { + } else if node.left in [ast.Ident, ast.SelectorExpr] && node.op == .ne + && node.right is ast.None { if node.left is ast.Ident && c.comptime.get_ct_type_var(node.left) == .smartcast { node.left_type = c.comptime.get_type(node.left) c.smartcast(mut node.left, node.left_type, node.left_type.clear_flag(.option), mut diff --git a/vlib/v/checker/tests/assign_type_mismatch_with_generics_err.out b/vlib/v/checker/tests/assign_type_mismatch_with_generics_err.out index b16915eae474d0..cafb83d3838cea 100644 --- a/vlib/v/checker/tests/assign_type_mismatch_with_generics_err.out +++ b/vlib/v/checker/tests/assign_type_mismatch_with_generics_err.out @@ -1,3 +1,10 @@ +vlib/v/checker/tests/assign_type_mismatch_with_generics_err.vv:13:11: error: unexpected `or` block, the field `f` is not an Option or a Result + 11 | mut b := false + 12 | if f.f != none { + 13 | b = f.f or { panic(err) } + | ~~~~~~~~~~~~~~~~~ + 14 | } else { + 15 | b = true vlib/v/checker/tests/assign_type_mismatch_with_generics_err.vv:13:9: error: cannot assign to `b`: expected `bool`, not `fn (Bar) bool` 11 | mut b := false 12 | if f.f != none { diff --git a/vlib/v/checker/tests/option_ptr_without_unwrapp_err.out b/vlib/v/checker/tests/option_ptr_without_unwrapp_err.out index 9281c74cb43215..61910cab2e77ab 100644 --- a/vlib/v/checker/tests/option_ptr_without_unwrapp_err.out +++ b/vlib/v/checker/tests/option_ptr_without_unwrapp_err.out @@ -1,7 +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` +vlib/v/checker/tests/option_ptr_without_unwrapp_err.vv:6:12: error: cannot use `?&Node` as `Node`, it must be unwrapped first in argument 1 to `set_trace` + 4 | 5 | fn set_trace(n Node) { - 6 | if n.parent != none { - 7 | set_trace(n.parent) - | ~~~~~~~~ - 8 | } - 9 | } + 6 | set_trace(n.parent) + | ~~~~~~~~ + 7 | } + 8 | diff --git a/vlib/v/checker/tests/option_ptr_without_unwrapp_err.vv b/vlib/v/checker/tests/option_ptr_without_unwrapp_err.vv index 107dc02e794d01..dc711c547c791b 100644 --- a/vlib/v/checker/tests/option_ptr_without_unwrapp_err.vv +++ b/vlib/v/checker/tests/option_ptr_without_unwrapp_err.vv @@ -3,9 +3,7 @@ struct Node { } fn set_trace(n Node) { - if n.parent != none { - set_trace(n.parent) - } + set_trace(n.parent) } fn main() { diff --git a/vlib/v/gen/c/cgen.v b/vlib/v/gen/c/cgen.v index a3ab73f1882aa6..52980eabc57639 100644 --- a/vlib/v/gen/c/cgen.v +++ b/vlib/v/gen/c/cgen.v @@ -4022,23 +4022,30 @@ fn (mut g Gen) selector_expr(node ast.SelectorExpr) { mut sum_type_deref_field := '' mut sum_type_dot := '.' mut field_typ := ast.void_type + mut is_option_unwrap := false if f := g.table.find_field_with_embeds(sym, node.field_name) { field_sym := g.table.sym(f.typ) field_typ = f.typ if sym.kind in [.interface, .sum_type] { g.write('(*(') } - if field_sym.kind in [.sum_type, .interface] { + is_option := field_typ.has_flag(.option) + if field_sym.kind in [.sum_type, .interface] || is_option { if !prevent_sum_type_unwrapping_once { // check first if field is sum type because scope searching is expensive scope := g.file.scope.innermost(node.pos.pos) if field := scope.find_struct_field(node.expr.str(), node.expr_type, node.field_name) { + is_option_unwrap = is_option && field.smartcasts.len > 0 + && field.typ.clear_flag(.option) == field.smartcasts.last() if field.orig_type.is_ptr() { sum_type_dot = '->' } for i, typ in field.smartcasts { + if i == 0 && is_option_unwrap { + g.write('(*(${g.styp(typ)}*)') + } g.write('(') - if field_sym.kind == .sum_type { + if field_sym.kind == .sum_type && !is_option { g.write('*') } cast_sym := g.table.sym(g.unwrap_generic(typ)) @@ -4047,7 +4054,7 @@ fn (mut g Gen) selector_expr(node ast.SelectorExpr) { dot := if node.expr_type.is_ptr() { '->' } else { '.' } g.write('I_${field_sym.cname}_as_I_${cast_sym.cname}(${ptr}${node.expr}${dot}${node.field_name}))') return - } else { + } else if !is_option_unwrap { if i != 0 { dot := if field.typ.is_ptr() { '->' } else { '.' } sum_type_deref_field += ')${dot}' @@ -4194,6 +4201,9 @@ fn (mut g Gen) selector_expr(node ast.SelectorExpr) { verror('cgen: SelectorExpr | expr_type: 0 | it.expr: `${node.expr}` | field: `${node.field_name}` | file: ${g.file.path} | line: ${node.pos.line_nr}') } g.write(field_name) + if is_option_unwrap { + g.write('.data))') + } if sum_type_deref_field != '' { g.write('${sum_type_dot}${sum_type_deref_field})') } diff --git a/vlib/v/tests/generics/generics_chans_select_test.v b/vlib/v/tests/generics/generics_chans_select_test.v index 92c6461c82d9a6..f7cc243b96d08e 100644 --- a/vlib/v/tests/generics/generics_chans_select_test.v +++ b/vlib/v/tests/generics/generics_chans_select_test.v @@ -35,10 +35,10 @@ pub fn (mut ec EventController[T]) emit(e T, options EmitOptions) { for i, w in ec.wait_fors { mut b := false if w.check != none { - func := w.check or { panic(err) } + func := w.check b = func(e) } else { - b = true + assert false } if b { w.c.c <- e diff --git a/vlib/v/tests/options/option_selector_unwrap_test.v b/vlib/v/tests/options/option_selector_unwrap_test.v new file mode 100644 index 00000000000000..78cd0b407c343b --- /dev/null +++ b/vlib/v/tests/options/option_selector_unwrap_test.v @@ -0,0 +1,31 @@ +import datatypes { DoublyLinkedList } + +pub type LayoutBoxId = usize + +pub struct LayoutBox { +} + +pub struct LayoutTree { +mut: + root ?LayoutBoxId + boxes []LayoutBox +} + +pub fn LayoutTree.new() LayoutTree { + return LayoutTree{ + root: ?LayoutBoxId(none) + boxes: []LayoutBox{} + } +} + +fn test_main() { + mut tree := LayoutTree.new() + tree.root = 1 + if tree.root != none { + mut parents := DoublyLinkedList[LayoutBoxId]{} + parents.push_back(tree.root) + assert parents.len == 1 + } else { + assert false + } +} diff --git a/vlib/v/tests/options/option_selector_unwrap_types_test.v b/vlib/v/tests/options/option_selector_unwrap_types_test.v new file mode 100644 index 00000000000000..f7a8f71c6b62bf --- /dev/null +++ b/vlib/v/tests/options/option_selector_unwrap_types_test.v @@ -0,0 +1,47 @@ +type SumType = int | string + +struct Struct { + a int +} + +struct Foo { + a ?int + b ?string + c ?SumType + d ?Struct +} + +fn test_main() { + w := Foo{ + a: 123 + b: 'foo' + c: SumType(123) + d: Struct{ + a: 123 + } + } + if w.a != none { + dump(w.a) + assert w.a == 123 + } else { + assert false + } + if w.b != none { + dump(w.b) + assert w.b == 'foo' + } else { + assert false + } + if w.c != none { + dump(w.c) + assert w.c is int + } else { + assert false + } + if w.d != none { + dump(w.d) + assert w.d.a == 123 + } else { + assert false + } +} diff --git a/vlib/v/tests/options/option_unwrap_test.v b/vlib/v/tests/options/option_unwrap_test.v index b827fc07862c21..15ecaf1382057a 100644 --- a/vlib/v/tests/options/option_unwrap_test.v +++ b/vlib/v/tests/options/option_unwrap_test.v @@ -6,7 +6,7 @@ pub mut: fn set_trace(n &Node) int { if n.parent != none { - set_trace(n.parent or { &Node{} }) + set_trace(n.parent) assert n.id != 0 } else { assert n.id == 1