Skip to content

Commit

Permalink
v: add selector option unwrapping inside if tree.root != none { (#2…
Browse files Browse the repository at this point in the history
  • Loading branch information
felipensp authored Nov 19, 2024
1 parent ae0fdbd commit 3d302a6
Show file tree
Hide file tree
Showing 10 changed files with 113 additions and 18 deletions.
5 changes: 3 additions & 2 deletions vlib/v/checker/checker.v
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion vlib/v/checker/if.v
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down
12 changes: 6 additions & 6 deletions vlib/v/checker/tests/option_ptr_without_unwrapp_err.out
Original file line number Diff line number Diff line change
@@ -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 |
4 changes: 1 addition & 3 deletions vlib/v/checker/tests/option_ptr_without_unwrapp_err.vv
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
16 changes: 13 additions & 3 deletions vlib/v/gen/c/cgen.v
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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}'
Expand Down Expand Up @@ -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})')
}
Expand Down
4 changes: 2 additions & 2 deletions vlib/v/tests/generics/generics_chans_select_test.v
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 31 additions & 0 deletions vlib/v/tests/options/option_selector_unwrap_test.v
Original file line number Diff line number Diff line change
@@ -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
}
}
47 changes: 47 additions & 0 deletions vlib/v/tests/options/option_selector_unwrap_types_test.v
Original file line number Diff line number Diff line change
@@ -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
}
}
2 changes: 1 addition & 1 deletion vlib/v/tests/options/option_unwrap_test.v
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 3d302a6

Please sign in to comment.