diff --git a/vlib/v/ast/ast.v b/vlib/v/ast/ast.v index 8da1abe720..d0b56201cb 100644 --- a/vlib/v/ast/ast.v +++ b/vlib/v/ast/ast.v @@ -720,11 +720,17 @@ pub mut: expr_type table.Type } +pub enum OrKind { + absent + block + propagate +} + // `or { ... }` pub struct OrExpr { pub: - stmts []Stmt - is_used bool // if the or{} block is written down or left out + stmts []Stmt + kind OrKind } pub struct Assoc { diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index 4839af11fc..fb0edb4b4d 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -24,7 +24,7 @@ pub mut: warnings []errors.Warning error_lines []int // to avoid printing multiple errors for the same line expected_type table.Type - fn_return_type table.Type // current function's return type + cur_fn &ast.FnDecl // current function const_decl string const_deps []string const_names []string @@ -44,6 +44,7 @@ pub fn new_checker(table &table.Table, pref &pref.Preferences) Checker { return Checker{ table: table pref: pref + cur_fn: 0 } } @@ -1014,24 +1015,34 @@ fn (mut c Checker) type_implements(typ, inter_typ table.Type, pos token.Position } } -pub fn (mut c Checker) check_expr_opt_call(expr ast.Expr, xtype table.Type, is_return_used bool) { +pub fn (mut c Checker) check_expr_opt_call(expr ast.Expr, ret_type table.Type, is_return_used bool) { if expr is ast.CallExpr { call_expr := expr as ast.CallExpr if call_expr.return_type.flag_is(.optional) { - c.check_or_block(call_expr, xtype, is_return_used) - } else if call_expr.or_block.is_used { + c.check_or_block(call_expr, ret_type, is_return_used) + } else if call_expr.or_block.kind == .block { c.error('unexpected `or` block, the function `$call_expr.name` does not return an optional', call_expr.pos) + } else if call_expr.or_block.kind == .propagate { + c.error('unexpected `?`, the function `$call_expr.name`, does not return an optional', + call_expr.pos) } } } pub fn (mut c Checker) check_or_block(mut call_expr ast.CallExpr, ret_type table.Type, is_ret_used bool) { - if !call_expr.or_block.is_used { + if call_expr.or_block.kind == .absent { c.error('${call_expr.name}() returns an option, but you missed to add an `or {}` block to it', call_expr.pos) return } + if call_expr.or_block.kind == .propagate { + if !c.cur_fn.return_type.flag_is(.optional) && c.cur_fn.name != 'main' { + c.error('to propagate the optional call, `${c.cur_fn.name}` must itself return an optional', + call_expr.pos) + } + return + } stmts_len := call_expr.or_block.stmts.len if stmts_len == 0 { if is_ret_used { @@ -1044,7 +1055,7 @@ pub fn (mut c Checker) check_or_block(mut call_expr ast.CallExpr, ret_type table } last_stmt := call_expr.or_block.stmts[stmts_len - 1] if is_ret_used { - if !c.is_last_or_block_stmt_valid(last_stmt) { + if !(last_stmt is ast.Return || last_stmt is ast.BranchStmt || last_stmt is ast.ExprStmt) { expected_type_name := c.table.get_type_symbol(ret_type).name c.error('last statement in the `or {}` block should return `$expected_type_name`', call_expr.pos) @@ -1084,16 +1095,6 @@ fn is_expr_panic_or_exit(expr ast.Expr) bool { } } -// TODO: merge to check_or_block when v can handle it -pub fn (mut c Checker) is_last_or_block_stmt_valid(stmt ast.Stmt) bool { - return match stmt { - ast.Return { true } - ast.BranchStmt { true } - ast.ExprStmt { true } - else { false } - } -} - pub fn (mut c Checker) selector_expr(mut selector_expr ast.SelectorExpr) table.Type { typ := c.expr(selector_expr.expr) if typ == table.void_type_idx { @@ -1126,19 +1127,19 @@ pub fn (mut c Checker) selector_expr(mut selector_expr ast.SelectorExpr) table.T // TODO: non deferred pub fn (mut c Checker) return_stmt(mut return_stmt ast.Return) { - c.expected_type = c.fn_return_type - if return_stmt.exprs.len > 0 && c.fn_return_type == table.void_type { + c.expected_type = c.cur_fn.return_type + if return_stmt.exprs.len > 0 && c.expected_type == table.void_type { c.error('too many arguments to return, current function does not return anything', return_stmt.pos) return - } else if return_stmt.exprs.len == 0 && c.fn_return_type != table.void_type { + } else if return_stmt.exprs.len == 0 && c.expected_type != table.void_type { c.error('too few arguments to return', return_stmt.pos) return } if return_stmt.exprs.len == 0 { return } - expected_type := c.fn_return_type + expected_type := c.expected_type expected_type_sym := c.table.get_type_symbol(expected_type) exp_is_optional := expected_type.flag_is(.optional) mut expected_types := [expected_type] @@ -1646,10 +1647,10 @@ fn (mut c Checker) stmts(stmts []ast.Stmt) { pub fn (mut c Checker) expr(node ast.Expr) table.Type { match mut node { ast.AnonFn { - keep_ret_type := c.fn_return_type - c.fn_return_type = it.decl.return_type + keep_fn := c.cur_fn + c.cur_fn = &it.decl c.stmts(it.decl.stmts) - c.fn_return_type = keep_ret_type + c.cur_fn = keep_fn return it.typ } ast.ArrayInit { @@ -2361,7 +2362,7 @@ fn (mut c Checker) fn_decl(it ast.FnDecl) { } } c.expected_type = table.void_type - c.fn_return_type = it.return_type + c.cur_fn = &it c.stmts(it.stmts) if it.language == .v && !it.no_body && it.return_type != table.void_type && !c.returns && it.name !in ['panic', 'exit'] { diff --git a/vlib/v/checker/tests/unexpected_or.out b/vlib/v/checker/tests/unexpected_or.out new file mode 100644 index 0000000000..050750c9bf --- /dev/null +++ b/vlib/v/checker/tests/unexpected_or.out @@ -0,0 +1,6 @@ +vlib/v/checker/tests/unexpected_or.v:6:7: error: unexpected `or` block, the function `ret_zero` does not return an optional + 4 | + 5 | fn main() { + 6 | _ := ret_zero() or { 1 } + | ~~~~~~~~~~ + 7 | } diff --git a/vlib/v/checker/tests/unexpected_or.vv b/vlib/v/checker/tests/unexpected_or.vv new file mode 100644 index 0000000000..2ced3c9309 --- /dev/null +++ b/vlib/v/checker/tests/unexpected_or.vv @@ -0,0 +1,7 @@ +fn ret_zero() int { + return 0 +} + +fn main() { + _ := ret_zero() or { 1 } +} diff --git a/vlib/v/checker/tests/unexpected_or_propagate.out b/vlib/v/checker/tests/unexpected_or_propagate.out new file mode 100644 index 0000000000..b5e6852581 --- /dev/null +++ b/vlib/v/checker/tests/unexpected_or_propagate.out @@ -0,0 +1,7 @@ +vlib/v/checker/tests/unexpected_or_propagate.v:6:7: error: unexpected `?`, the function `ret_zero`, does not return an optional + 4 | + 5 | fn opt_fn() ?int { + 6 | a := ret_zero()? + | ~~~~~~~~~~ + 7 | return a + 8 | } diff --git a/vlib/v/checker/tests/unexpected_or_propagate.vv b/vlib/v/checker/tests/unexpected_or_propagate.vv new file mode 100644 index 0000000000..16ef7b0d34 --- /dev/null +++ b/vlib/v/checker/tests/unexpected_or_propagate.vv @@ -0,0 +1,12 @@ +fn ret_zero() int { + return 0 +} + +fn opt_fn() ?int { + a := ret_zero()? + return a +} + +fn main() { + opt_fn() or {} +} diff --git a/vlib/v/checker/tests/wrong_propagate_ret_type.out b/vlib/v/checker/tests/wrong_propagate_ret_type.out new file mode 100644 index 0000000000..c4ef1ed49e --- /dev/null +++ b/vlib/v/checker/tests/wrong_propagate_ret_type.out @@ -0,0 +1,7 @@ +vlib/v/checker/tests/wrong_propagate_ret_type.v:6:7: error: to propagate the optional call, `opt_call` must itself return an optional + 4 | + 5 | fn opt_call() int { + 6 | a := ret_none()? + | ~~~~~~~~~~ + 7 | return a + 8 | } diff --git a/vlib/v/checker/tests/wrong_propagate_ret_type.vv b/vlib/v/checker/tests/wrong_propagate_ret_type.vv new file mode 100644 index 0000000000..82ef06c493 --- /dev/null +++ b/vlib/v/checker/tests/wrong_propagate_ret_type.vv @@ -0,0 +1,8 @@ +fn ret_none() ?int { + return none +} + +fn opt_call() int { + a := ret_none()? + return a +} diff --git a/vlib/v/fmt/fmt.v b/vlib/v/fmt/fmt.v index 8e8c5b9076..59ae7c4612 100644 --- a/vlib/v/fmt/fmt.v +++ b/vlib/v/fmt/fmt.v @@ -725,10 +725,16 @@ pub fn (mut f Fmt) call_args(args []ast.CallArg) { } pub fn (mut f Fmt) or_expr(or_block ast.OrExpr) { - if or_block.is_used { - f.writeln(' or {') - f.stmts(or_block.stmts) - f.write('}') + match or_block.kind { + .absent {} + .block { + f.writeln(' or {') + f.stmts(or_block.stmts) + f.write('}') + } + .propagate { + f.write('?') + } } } diff --git a/vlib/v/fmt/tests/optional_propagate_keep.vv b/vlib/v/fmt/tests/optional_propagate_keep.vv new file mode 100644 index 0000000000..bf287bd4d9 --- /dev/null +++ b/vlib/v/fmt/tests/optional_propagate_keep.vv @@ -0,0 +1,3 @@ +fn opt_propagate() ?int { + eventual_wrong_int()? +} diff --git a/vlib/v/gen/cgen.v b/vlib/v/gen/cgen.v index d8154127b7..8f58106028 100644 --- a/vlib/v/gen/cgen.v +++ b/vlib/v/gen/cgen.v @@ -95,6 +95,7 @@ mut: is_builtin_mod bool hotcode_fn_names []string fn_main &ast.FnDecl // the FnDecl of the main function. Needed in order to generate the main function code *last* + cur_fn &ast.FnDecl cur_generic_type table.Type // `int`, `string`, etc in `foo()` } @@ -124,6 +125,7 @@ pub fn cgen(files []ast.File, table &table.Table, pref &pref.Preferences) string pref: pref fn_decl: 0 fn_main: 0 + cur_fn: 0 autofree: true indent: -1 module_built: pref.path.after('vlib/') @@ -895,7 +897,6 @@ fn (mut g Gen) gen_assign_stmt(assign_stmt ast.AssignStmt) { 1 { // multi return // TODO Handle in if_expr - mut or_stmts := []ast.Stmt{} is_optional := return_type.flag_is(.optional) mr_var_name := 'mr_$assign_stmt.pos.pos' mr_styp := g.typ(return_type) @@ -905,9 +906,8 @@ fn (mut g Gen) gen_assign_stmt(assign_stmt ast.AssignStmt) { g.is_assign_rhs = false if is_optional && assign_stmt.right[0] is ast.CallExpr { val := assign_stmt.right[0] as ast.CallExpr - or_stmts = val.or_block.stmts return_type = val.return_type - g.or_block(mr_var_name, or_stmts, return_type) + g.or_block(mr_var_name, val.or_block, return_type) } g.writeln(';') for i, ident in assign_stmt.left { @@ -936,12 +936,10 @@ fn (mut g Gen) gen_assign_stmt(assign_stmt ast.AssignStmt) { ident_var_info := ident.var_info() styp := g.typ(ident_var_info.typ) mut is_call := false - mut or_stmts := []ast.Stmt{} blank_assign := ident.kind == .blank_ident match val { ast.CallExpr { is_call = true - or_stmts = it.or_block.stmts return_type = it.return_type } // TODO: no buffer fiddling @@ -1454,12 +1452,12 @@ fn (mut g Gen) enum_expr(node ast.Expr) { fn (mut g Gen) assign_expr(node ast.AssignExpr) { // g.write('/*assign_expr*/') mut is_call := false - mut or_stmts := []ast.Stmt{} + mut or_block := ast.OrExpr{} mut return_type := table.void_type match node.val { ast.CallExpr { is_call = true - or_stmts = it.or_block.stmts + or_block = it.or_block return_type = it.return_type } else {} @@ -1538,7 +1536,7 @@ fn (mut g Gen) assign_expr(node ast.AssignExpr) { } if gen_or { // g.write('/*777 $tmp_opt*/') - g.or_block(tmp_opt, or_stmts, return_type) + g.or_block(tmp_opt, or_block, return_type) unwrapped_type_str := g.typ(return_type.set_flag(.unset)) ident := node.left as ast.Ident if ident.kind != .blank_ident && ident.info is ast.IdentVar { @@ -2936,40 +2934,49 @@ fn (mut g Gen) insert_before_stmt(s string) { // to access its fields (`.ok`, `.error` etc) // `os.cp(...)` => `Option bool tmp = os__cp(...); if (!tmp.ok) { ... }` // Returns the type of the last stmt -fn (mut g Gen) or_block(var_name string, stmts []ast.Stmt, return_type table.Type) { +fn (mut g Gen) or_block(var_name string, or_block ast.OrExpr, return_type table.Type) { cvar_name := c_name(var_name) mr_styp := g.base_type(return_type) g.writeln(';') // or') g.writeln('if (!${cvar_name}.ok) {') - g.writeln('\tstring err = ${cvar_name}.v_error;') - g.writeln('\tint errcode = ${cvar_name}.ecode;') - if stmts.len > 0 && stmts[stmts.len - 1] is ast.ExprStmt && (stmts[stmts.len - 1] as ast.ExprStmt).typ != - table.void_type { - g.indent++ - for i, stmt in stmts { - if i == stmts.len - 1 { - expr_stmt := stmt as ast.ExprStmt - g.stmt_path_pos << g.out.len - g.write('*(${mr_styp}*) ${cvar_name}.data = ') - is_opt_call := expr_stmt.expr is ast.CallExpr && expr_stmt.typ.flag_is(.optional) - if is_opt_call { - g.write('*(${mr_styp}*) ') + if or_block.kind == .block { + g.writeln('\tstring err = ${cvar_name}.v_error;') + g.writeln('\tint errcode = ${cvar_name}.ecode;') + stmts := or_block.stmts + if stmts.len > 0 && stmts[or_block.stmts.len - 1] is ast.ExprStmt && (stmts[stmts.len - + 1] as ast.ExprStmt).typ != table.void_type { + g.indent++ + for i, stmt in stmts { + if i == stmts.len - 1 { + expr_stmt := stmt as ast.ExprStmt + g.stmt_path_pos << g.out.len + g.write('*(${mr_styp}*) ${cvar_name}.data = ') + is_opt_call := expr_stmt.expr is ast.CallExpr && expr_stmt.typ.flag_is(.optional) + if is_opt_call { + g.write('*(${mr_styp}*) ') + } + g.expr(expr_stmt.expr) + if is_opt_call { + g.write('.data') + } + if g.inside_ternary == 0 && !(expr_stmt.expr is ast.IfExpr) { + g.writeln(';') + } + g.stmt_path_pos.delete(g.stmt_path_pos.len - 1) + } else { + g.stmt(stmt) } - g.expr(expr_stmt.expr) - if is_opt_call { - g.write('.data') - } - if g.inside_ternary == 0 && !(expr_stmt.expr is ast.IfExpr) { - g.writeln(';') - } - g.stmt_path_pos.delete(g.stmt_path_pos.len - 1) - } else { - g.stmt(stmt) } + g.indent-- + } else { + g.stmts(stmts) + } + } else if or_block.kind == .propagate { + if g.file.mod.name == 'main' && g.cur_fn.name == 'main' { + g.writeln('\tv_panic(${cvar_name}.v_error);') + } else { + g.writeln('\treturn $cvar_name;') } - g.indent-- - } else { - g.stmts(stmts) } g.write('}') } diff --git a/vlib/v/gen/fn.v b/vlib/v/gen/fn.v index 77d02b9b0f..13879af772 100644 --- a/vlib/v/gen/fn.v +++ b/vlib/v/gen/fn.v @@ -12,6 +12,11 @@ fn (mut g Gen) gen_fn_decl(it ast.FnDecl) { // || it.no_body { return } + former_cur_fn := g.cur_fn + g.cur_fn = &it + defer { + g.cur_fn = former_cur_fn + } is_main := it.name == 'main' if it.is_generic && g.cur_generic_type == 0 { // need the cur_generic_type check to avoid inf. recursion // loop thru each generic type and generate a function @@ -284,7 +289,7 @@ fn (mut g Gen) call_expr(node ast.CallExpr) { if node.should_be_skipped { return } - gen_or := node.or_block.stmts.len > 0 + gen_or := node.or_block.kind != .absent cur_line := if gen_or && g.is_assign_rhs { line := g.go_before_stmt(0) g.out.write(tabs[g.indent]) @@ -303,7 +308,7 @@ fn (mut g Gen) call_expr(node ast.CallExpr) { g.fn_call(node) } if gen_or { - g.or_block(tmp_opt, node.or_block.stmts, node.return_type) + g.or_block(tmp_opt, node.or_block, node.return_type) g.write('\n${cur_line}${tmp_opt}') } } diff --git a/vlib/v/parser/fn.v b/vlib/v/parser/fn.v index 574446aa90..85709bec03 100644 --- a/vlib/v/parser/fn.v +++ b/vlib/v/parser/fn.v @@ -19,11 +19,11 @@ pub fn (mut p Parser) call_expr(language table.Language, mod string) ast.CallExp } else { p.check_name() } - mut is_or_block_used := false + mut or_kind := ast.OrKind.absent if fn_name == 'json.decode' { p.expecting_type = true // Makes name_expr() parse the type `User` in `json.decode(User, txt)` p.expr_mod = '' - is_or_block_used = true + or_kind = .block } mut generic_type := table.void_type if p.tok.kind == .lt { @@ -42,9 +42,9 @@ pub fn (mut p Parser) call_expr(language table.Language, mod string) ast.CallExp pos: first_pos.pos len: last_pos.pos - first_pos.pos + last_pos.len } - // `foo() or {}`` mut or_stmts := []ast.Stmt{} if p.tok.kind == .key_orelse { + // `foo() or {}`` was_inside_or_expr := p.inside_or_expr p.inside_or_expr = true p.next() @@ -61,7 +61,7 @@ pub fn (mut p Parser) call_expr(language table.Language, mod string) ast.CallExp pos: p.tok.position() is_used: true }) - is_or_block_used = true + or_kind = .block or_stmts = p.parse_block_no_scope() p.close_scope() p.inside_or_expr = was_inside_or_expr @@ -69,10 +69,7 @@ pub fn (mut p Parser) call_expr(language table.Language, mod string) ast.CallExp if p.tok.kind == .question { // `foo()?` p.next() - is_or_block_used = true - // mut s := ast.Stmt{} - // s = ast.ReturnStmt{} - or_stmts << ast.Return{} + or_kind = .propagate } node := ast.CallExpr{ name: fn_name @@ -82,7 +79,7 @@ pub fn (mut p Parser) call_expr(language table.Language, mod string) ast.CallExp language: language or_block: ast.OrExpr{ stmts: or_stmts - is_used: is_or_block_used + kind: or_kind } generic_type: generic_type } diff --git a/vlib/v/parser/parser.v b/vlib/v/parser/parser.v index 86f8931850..1dbe12584f 100644 --- a/vlib/v/parser/parser.v +++ b/vlib/v/parser/parser.v @@ -195,7 +195,7 @@ pub fn parse_files(paths []string, table &table.Table, pref &pref.Preferences, g } if false { // TODO: remove this; it just prevents warnings about unused time and runtime - time.sleep_ms(1) + time.sleep_ms(1) println(runtime.nr_cpus()) } // /////////////// @@ -986,7 +986,7 @@ fn (mut p Parser) dot_expr(left ast.Expr) ast.Expr { } p.check(.rpar) mut or_stmts := []ast.Stmt{} - mut is_or_block_used := false + mut or_kind := ast.OrKind.absent if p.tok.kind == .key_orelse { p.next() p.open_scope() @@ -1002,10 +1002,15 @@ fn (mut p Parser) dot_expr(left ast.Expr) ast.Expr { pos: p.tok.position() is_used: true }) - is_or_block_used = true + or_kind = .block or_stmts = p.parse_block_no_scope() p.close_scope() } + if p.tok.kind == .question { + // `foo()?` + p.next() + or_kind = .propagate + } end_pos := p.tok.position() pos := token.Position{ line_nr: name_pos.line_nr @@ -1020,7 +1025,7 @@ fn (mut p Parser) dot_expr(left ast.Expr) ast.Expr { is_method: true or_block: ast.OrExpr{ stmts: or_stmts - is_used: is_or_block_used + kind: or_kind } } if is_filter { diff --git a/vlib/v/tests/option_test.v b/vlib/v/tests/option_test.v index 1d0a479de6..8b0619a6e5 100644 --- a/vlib/v/tests/option_test.v +++ b/vlib/v/tests/option_test.v @@ -93,6 +93,33 @@ fn foo_str() ?string { return 'something' } +fn propagate_optional(b bool) ?int { + a := err_call(b)? + return a +} + +fn propagate_different_type(b bool) ?bool { + err_call(b)? + return true +} + +fn test_propagation() { + a := propagate_optional(true) or { + 0 + } + assert a == 42 + if _ := propagate_optional(false) { + assert false + } + b := propagate_different_type(true) or { + false + } + assert b == true + if _ := propagate_different_type(false) { + assert false + } +} + fn test_q() { // assert foo_ok()? == true }