From e253256c304b032a9dbc04867d40707ea6e34b8b Mon Sep 17 00:00:00 2001 From: Felipe Pena Date: Wed, 8 Mar 2023 16:52:24 -0300 Subject: [PATCH] cgen: fix option unwrap on assignment (#17551) --- vlib/v/checker/checker.v | 5 +++-- vlib/v/checker/tests/wrong_option_unwrap_err.out | 7 +++++++ vlib/v/checker/tests/wrong_option_unwrap_err.vv | 5 +++++ vlib/v/gen/c/assign.v | 6 +++--- vlib/v/gen/c/cgen.v | 10 ++++++---- vlib/v/tests/option_unwrap_assign_test.v | 15 +++++++++++++++ 6 files changed, 39 insertions(+), 9 deletions(-) create mode 100644 vlib/v/checker/tests/wrong_option_unwrap_err.out create mode 100644 vlib/v/checker/tests/wrong_option_unwrap_err.vv create mode 100644 vlib/v/tests/option_unwrap_assign_test.v diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index 9bd36b90dd..c3bd4f2005 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -1119,8 +1119,9 @@ fn (mut c Checker) check_or_last_stmt(stmt ast.Stmt, ret_type ast.Type, expr_ret c.expected_or_type = ret_type.clear_flag(.option).clear_flag(.result) last_stmt_typ := c.expr(stmt.expr) - if ret_type.has_flag(.option) && last_stmt_typ.has_flag(.option) { - if stmt.expr in [ast.Ident, ast.SelectorExpr, ast.CallExpr] { + if ret_type.has_flag(.option) + && (last_stmt_typ.has_flag(.option) || last_stmt_typ == ast.none_type) { + if stmt.expr in [ast.Ident, ast.SelectorExpr, ast.CallExpr, ast.None] { expected_type_name := c.table.type_to_str(ret_type.clear_flag(.option).clear_flag(.result)) got_type_name := c.table.type_to_str(last_stmt_typ) c.error('`or` block must provide a value of type `${expected_type_name}`, not `${got_type_name}`', diff --git a/vlib/v/checker/tests/wrong_option_unwrap_err.out b/vlib/v/checker/tests/wrong_option_unwrap_err.out new file mode 100644 index 0000000000..611c135764 --- /dev/null +++ b/vlib/v/checker/tests/wrong_option_unwrap_err.out @@ -0,0 +1,7 @@ +vlib/v/checker/tests/wrong_option_unwrap_err.vv:3:14: error: `or` block must provide a value of type `string`, not `none` + 1 | fn main() { + 2 | a := ?string('c') + 3 | b := a or { none } + | ~~~~ + 4 | println(b) + 5 | } diff --git a/vlib/v/checker/tests/wrong_option_unwrap_err.vv b/vlib/v/checker/tests/wrong_option_unwrap_err.vv new file mode 100644 index 0000000000..5189e6c360 --- /dev/null +++ b/vlib/v/checker/tests/wrong_option_unwrap_err.vv @@ -0,0 +1,5 @@ +fn main() { + a := ?string('c') + b := a or { none } + println(b) +} diff --git a/vlib/v/gen/c/assign.v b/vlib/v/gen/c/assign.v index 8a1c7d3753..998044df7c 100644 --- a/vlib/v/gen/c/assign.v +++ b/vlib/v/gen/c/assign.v @@ -10,8 +10,8 @@ import v.token fn (mut g Gen) expr_with_opt_or_block(expr ast.Expr, expr_typ ast.Type, var_expr ast.Expr, ret_typ ast.Type) { gen_or := expr is ast.Ident && (expr as ast.Ident).or_expr.kind != .absent if gen_or { - old_inside_opt_data := g.inside_opt_data - g.inside_opt_data = true + old_inside_opt_or_res := g.inside_opt_or_res + g.inside_opt_or_res = true g.expr_with_cast(expr, expr_typ, ret_typ) g.writeln(';') g.writeln('if (${expr}.state != 0) {') @@ -40,7 +40,7 @@ fn (mut g Gen) expr_with_opt_or_block(expr ast.Expr, expr_typ ast.Type, var_expr } } g.writeln('}') - g.inside_opt_data = old_inside_opt_data + g.inside_opt_or_res = old_inside_opt_or_res } else { g.expr_with_opt(expr, expr_typ, ret_typ) } diff --git a/vlib/v/gen/c/cgen.v b/vlib/v/gen/c/cgen.v index 44895fa51e..2dc55a70c7 100644 --- a/vlib/v/gen/c/cgen.v +++ b/vlib/v/gen/c/cgen.v @@ -4093,7 +4093,7 @@ fn (mut g Gen) ident(node ast.Ident) { if node.obj is ast.Var { if !g.is_assign_lhs && node.obj.is_comptime_field { if g.comptime_for_field_type.has_flag(.option) { - if (g.inside_opt_or_res && node.or_expr.kind == .absent) || g.left_is_opt { + if (g.inside_opt_or_res || g.left_is_opt) && node.or_expr.kind == .absent { g.write('${name}') } else { g.write('/*opt*/') @@ -4103,7 +4103,8 @@ fn (mut g Gen) ident(node ast.Ident) { } else { g.write('${name}') } - if node.or_expr.kind != .absent { + if node.or_expr.kind != .absent && !(g.inside_opt_or_res && g.inside_assign + && !g.is_assign_lhs) { stmt_str := g.go_before_stmt(0).trim_space() g.empty_line = true g.or_block(name, node.or_expr, g.comptime_for_field_type) @@ -4117,14 +4118,15 @@ fn (mut g Gen) ident(node ast.Ident) { // `x = new_opt()` => `x = new_opt()` (g.right_is_opt == true) // `println(x)` => `println(*(int*)x.data)` if node.info.is_option && !(g.is_assign_lhs && g.right_is_opt) { - if (g.inside_opt_or_res && node.or_expr.kind == .absent) || g.left_is_opt { + if (g.inside_opt_or_res || g.left_is_opt) && node.or_expr.kind == .absent { g.write('${name}') } else { g.write('/*opt*/') styp := g.base_type(node.info.typ) g.write('(*(${styp}*)${name}.data)') } - if node.or_expr.kind != .absent && !(g.inside_assign && !g.is_assign_lhs) { + if node.or_expr.kind != .absent && !(g.inside_opt_or_res && g.inside_assign + && !g.is_assign_lhs) { stmt_str := g.go_before_stmt(0).trim_space() g.empty_line = true g.or_block(name, node.or_expr, node.info.typ) diff --git a/vlib/v/tests/option_unwrap_assign_test.v b/vlib/v/tests/option_unwrap_assign_test.v new file mode 100644 index 0000000000..a36571ee80 --- /dev/null +++ b/vlib/v/tests/option_unwrap_assign_test.v @@ -0,0 +1,15 @@ +struct Foo { +mut: + x string + y ?string +} + +fn test_main() { + a := ?string(none) + mut foo := Foo{} + foo.x = a or { 'test' } + foo.y = a or { 'test' } + + assert foo.x == 'test' + assert foo.y? == 'test' +}