Skip to content

Commit

Permalink
cgen, checker: fix none checking (#17364)
Browse files Browse the repository at this point in the history
  • Loading branch information
felipensp committed Feb 22, 2023
1 parent c173104 commit 2879afa
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 31 deletions.
5 changes: 3 additions & 2 deletions vlib/v/checker/infix.v
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,7 @@ fn (mut c Checker) infix_expr(mut node ast.InfixExpr) ast.Type {
}
}
}
} else if node.left !is ast.Ident
} else if node.left !in [ast.Ident, ast.SelectorExpr, ast.ComptimeSelector]
&& (left_type.has_flag(.option) || right_type.has_flag(.option)) {
opt_comp_pos := if left_type.has_flag(.option) { left_pos } else { right_pos }
c.error('unwrapped option cannot be compared in an infix expression',
Expand Down Expand Up @@ -669,7 +669,8 @@ fn (mut c Checker) infix_expr(mut node ast.InfixExpr) ast.Type {
// TODO move this to symmetric_check? Right now it would break `return 0` for `fn()?int `
left_is_option := left_type.has_flag(.option)
right_is_option := right_type.has_flag(.option)
if node.left !is ast.Ident && (left_is_option || right_is_option) {
if node.left !in [ast.Ident, ast.SelectorExpr, ast.ComptimeSelector]
&& (left_is_option || right_is_option) {
opt_infix_pos := if left_is_option { left_pos } else { right_pos }
c.error('unwrapped option cannot be used in an infix expression', opt_infix_pos)
}
Expand Down
32 changes: 15 additions & 17 deletions vlib/v/gen/c/assign.v
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,10 @@ fn (mut g Gen) expr_opt_with_cast(expr ast.Expr, expr_typ ast.Type, ret_typ ast.
g.expr(expr)
g.write('.data)')
} else {
old_inside_opt_or_res := g.inside_opt_or_res
g.inside_opt_or_res = false
g.expr(expr)
g.inside_opt_or_res = old_inside_opt_or_res
}
g.writeln(' }, (${option_name}*)(&${tmp_var}), sizeof(${styp}));')
g.write(stmt_str)
Expand All @@ -65,32 +67,24 @@ fn (mut g Gen) expr_opt_with_cast(expr ast.Expr, expr_typ ast.Type, ret_typ ast.
// expr_with_opt is used in assigning an expression to an `option` variable
// e.g. x = y (option lhs and rhs), mut x = ?int(123), y = none
fn (mut g Gen) expr_with_opt(expr ast.Expr, expr_typ ast.Type, ret_typ ast.Type) string {
if expr_typ == ast.none_type {
old_inside_opt_data := g.inside_opt_data
g.inside_opt_or_res = true
defer {
g.inside_opt_data = old_inside_opt_data
}
old_inside_opt_or_res := g.inside_opt_or_res
g.inside_opt_or_res = true
defer {
g.inside_opt_or_res = old_inside_opt_or_res
}
if expr_typ.has_flag(.option) && ret_typ.has_flag(.option)&& (expr in [ast.Ident, ast.ComptimeSelector, ast.AsCast, ast.CallExpr, ast.MatchExpr, ast.IfExpr, ast.IndexExpr, ast.UnsafeExpr, ast.CastExpr]) {
if expr in [ast.Ident, ast.CastExpr] {
if expr_typ.idx() != ret_typ.idx() {
return g.expr_opt_with_cast(expr, expr_typ, ret_typ)
}
old_inside_opt_data := g.inside_opt_data
g.inside_opt_or_res = true
defer {
g.inside_opt_data = old_inside_opt_data
}
}
g.expr(expr)
return expr.str()
} else {
old_inside_opt_data := g.inside_opt_data
g.inside_opt_or_res = true
defer {
g.inside_opt_data = old_inside_opt_data
if expr is ast.ComptimeSelector {
return '${expr.left.str()}.${g.comptime_for_field_value.name}'
} else {
return expr.str()
}
} else {
tmp_out_var := g.new_tmp_var()
g.expr_with_tmp_var(expr, expr_typ, ret_typ, tmp_out_var)
return tmp_out_var
Expand Down Expand Up @@ -552,6 +546,10 @@ fn (mut g Gen) assign_stmt(node_ ast.AssignStmt) {
if !g.inside_comptime_for_field
&& ((var_type.has_flag(.option) && !val_type.has_flag(.option))
|| (var_type.has_flag(.result) && !val_type.has_flag(.result))) {
old_inside_opt_or_res := g.inside_opt_or_res
defer {
g.inside_opt_or_res = old_inside_opt_or_res
}
g.inside_opt_or_res = true
tmp_var := g.new_tmp_var()
g.expr_with_tmp_var(val, val_type, var_type, tmp_var)
Expand Down
14 changes: 10 additions & 4 deletions vlib/v/gen/c/cgen.v
Original file line number Diff line number Diff line change
Expand Up @@ -3492,7 +3492,7 @@ fn (mut g Gen) selector_expr(node ast.SelectorExpr) {
&& (node.expr_type.has_flag(.option) || node.expr_type.has_flag(.result))
if is_opt_or_res {
opt_base_typ := g.base_type(node.expr_type)
g.writeln('(*(${opt_base_typ}*)')
g.write('(*(${opt_base_typ}*)')
}
if sym.kind in [.interface_, .sum_type] {
g.write('(*(')
Expand Down Expand Up @@ -4142,7 +4142,7 @@ fn (mut g Gen) ident(node ast.Ident) {
stmt_str := g.go_before_stmt(0).trim_space()
g.empty_line = true
g.or_block(name, node.or_expr, node.info.typ)
g.writeln(stmt_str)
g.write(stmt_str)
}
return
}
Expand Down Expand Up @@ -4702,7 +4702,7 @@ fn (mut g Gen) return_stmt(node ast.Return) {
}
if g.table.sym(mr_info.types[i]).kind in [.sum_type, .interface_] {
g.expr_with_cast(expr, node.types[i], mr_info.types[i])
} else if mr_info.types[i].has_flag(.option) && !node.types[i].has_flag(.option) {
} else if mr_info.types[i].has_flag(.option) {
g.expr_with_opt(expr, node.types[i], mr_info.types[i])
} else {
g.expr(expr)
Expand Down Expand Up @@ -4790,7 +4790,11 @@ fn (mut g Gen) return_stmt(node ast.Return) {
}
}
for i, expr in node.exprs {
g.expr_with_cast(expr, node.types[i], fn_ret_type.clear_flag(.result))
if fn_ret_type.has_flag(.option) {
g.expr_with_opt(expr, node.types[i], fn_ret_type.clear_flag(.result))
} else {
g.expr_with_cast(expr, node.types[i], fn_ret_type.clear_flag(.result))
}
if i < node.exprs.len - 1 {
g.write(', ')
}
Expand Down Expand Up @@ -4837,6 +4841,8 @@ fn (mut g Gen) return_stmt(node ast.Return) {
if g.fn_decl.return_type.is_ptr() {
var_str := g.expr_string(expr0)
g.write(var_str.trim('&'))
} else if g.fn_decl.return_type.has_flag(.option) {
g.expr_with_opt(expr0, node.types[0], g.fn_decl.return_type)
} else if g.table.sym(g.fn_decl.return_type).kind in [.sum_type, .interface_] {
g.expr_with_cast(expr0, node.types[0], g.fn_decl.return_type)
} else {
Expand Down
5 changes: 5 additions & 0 deletions vlib/v/gen/c/for.v
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,12 @@ fn (mut g Gen) for_in_stmt(node_ ast.ForInStmt) {
cond_var = g.new_tmp_var()
g.write(g.typ(node.cond_type))
g.write(' ${cond_var} = ')
old_inside_opt_or_res := g.inside_opt_or_res
if node.cond_type.has_flag(.option) {
g.inside_opt_or_res = true
}
g.expr(node.cond)
g.inside_opt_or_res = old_inside_opt_or_res
g.writeln(';')
}
i := if node.key_var in ['', '_'] { g.new_tmp_var() } else { node.key_var }
Expand Down
23 changes: 15 additions & 8 deletions vlib/v/gen/c/infix.v
Original file line number Diff line number Diff line change
Expand Up @@ -955,14 +955,21 @@ fn (mut g Gen) infix_expr_and_or_op(node ast.InfixExpr) {
}

fn (mut g Gen) gen_is_none_check(node ast.InfixExpr) {
stmt_str := g.go_before_stmt(0).trim_space()
g.empty_line = true
left_var := g.expr_with_opt(node.left, node.left_type, node.left_type)
g.writeln(';')
g.write(stmt_str)
g.write(' ')

g.write('${left_var}.state')
if node.left in [ast.Ident, ast.SelectorExpr] {
old_inside_opt_or_res := g.inside_opt_or_res
g.inside_opt_or_res = true
g.expr(node.left)
g.inside_opt_or_res = old_inside_opt_or_res
g.write('.state')
} else {
stmt_str := g.go_before_stmt(0).trim_space()
g.empty_line = true
left_var := g.expr_with_opt(node.left, node.left_type, node.left_type)
g.writeln(';')
g.write(stmt_str)
g.write(' ')
g.write('${left_var}.state')
}
g.write(' ${node.op.str()} ')
g.write('2') // none state
}
Expand Down
42 changes: 42 additions & 0 deletions vlib/v/tests/none_checking_test.v
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
struct Test {
a ?string
b ?int
c ?u8
d ?fn (int)
}

fn test_main() {
a := ?int(none)

if a == none {
assert true
} else {
assert false
}

b := Test{}
if b.a == none {
assert true
} else {
assert false
}

c := ?Test(Test{})
if c?.a == none {
assert true
} else {
assert false
}
}

fn test_comptime() {
t := Test{}
$for f in Test.fields {
println('${f.name}')
if t.$(f.name) == none {
assert true
} else {
assert false
}
}
}

0 comments on commit 2879afa

Please sign in to comment.