From 0024ff848d8ad265bdc4688331fac8a810dac9f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20D=C3=A4schle?= Date: Tue, 7 Apr 2020 16:36:00 +0200 Subject: [PATCH] parser: check if the last or {} block expression is valid These checks allows for: a) `x := call() or { 'abc' }` b) `x := call() or { panic('abc') }` c) `x := call() or { exit(123) }` d) `x := call() or { continue }` e) `x := call() or { break }` f) `x := call() or { return }` ... but produce errors for: g) `x := call() or { println('an error') }` , etc. --- vlib/os/os_test.v | 3 ++- vlib/v/ast/ast.v | 2 ++ vlib/v/checker/checker.v | 55 ++++++++++++++++++++++++++++++++++++++- vlib/v/parser/fn.v | 7 +++-- vlib/v/parser/parser.v | 9 +++++-- vlib/v/tests/defer_test.v | 7 ++--- 6 files changed, 72 insertions(+), 11 deletions(-) diff --git a/vlib/os/os_test.v b/vlib/os/os_test.v index ec32546bc0..f448f4b7b0 100644 --- a/vlib/os/os_test.v +++ b/vlib/os/os_test.v @@ -27,6 +27,7 @@ fn test_open_file() { hello := 'hello world!' os.open_file(filename, 'r+', 0o666) or { assert err == 'No such file or directory' + os.File{} } mut file := os.open_file(filename, 'w+', 0o666) or { panic(err) @@ -212,7 +213,7 @@ fn test_is_writable_folder() { tmp := os.temp_dir() f := os.is_writable_folder(tmp) or { eprintln('err: $err') - assert false + false } assert f } diff --git a/vlib/v/ast/ast.v b/vlib/v/ast/ast.v index fe8d197d4c..9b47e0e78b 100644 --- a/vlib/v/ast/ast.v +++ b/vlib/v/ast/ast.v @@ -44,6 +44,7 @@ pub struct ExprStmt { pub: expr Expr typ table.Type + pos token.Position } pub struct IntegerLiteral { @@ -647,6 +648,7 @@ mut: pub struct OrExpr { pub: stmts []Stmt + is_used bool // if the or{} block is written down or left out // var_name string // expr Expr } diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index 38c30bed88..ef8789e520 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -394,6 +394,57 @@ pub fn (c mut Checker) call_expr(call_expr mut ast.CallExpr) table.Type { } } +pub fn (c mut Checker) check_or_block(call_expr mut ast.CallExpr, ret_type table.Type) { + if call_expr.or_block.is_used { + stmts_len := call_expr.or_block.stmts.len + if stmts_len != 0 { + last_stmt := call_expr.or_block.stmts[stmts_len - 1] + + if c.is_or_block_stmt_valid(last_stmt) { + match last_stmt { + ast.ExprStmt { + type_fits := c.table.check(c.expr(it.expr), ret_type) + is_panic_or_exit := is_expr_panic_or_exit(it.expr) + if !type_fits && !is_panic_or_exit { + type_name := c.table.get_type_symbol(c.expr(it.expr)).name + expected_type_name := c.table.get_type_symbol(ret_type).name + c.error('wrong return type ‘$type_name‘ in or{} block, expected ‘$expected_type_name‘', it.pos) + } + } + ast.BranchStmt { + if !(it.tok.kind in [.key_continue, .key_break]) { + c.error('only break and continue are allowed as a final branch statement in an or{} block', it.tok.position()) + } + } + else {} + } + } else { + expected_type_name := c.table.get_type_symbol(ret_type).name + c.error('last statement in or{} block should return ‘$expected_type_name‘', call_expr.pos) + } + } else { + c.error('require a statement in or{} block', call_expr.pos) + } + } +} + +fn is_expr_panic_or_exit(expr ast.Expr) bool { + match expr { + ast.CallExpr { return it.name == 'panic' || it.name == 'exit' } + else { return false } + } +} + +// TODO: merge to check_or_block when v can handle it +pub fn (c mut Checker) is_or_block_stmt_valid(stmt ast.Stmt) bool { + return match stmt { + ast.Return { true } + ast.BranchStmt { true } + ast.ExprStmt { true } + else { false } + } +} + pub fn (c mut Checker) selector_expr(selector_expr mut ast.SelectorExpr) table.Type { typ := c.expr(selector_expr.expr) if typ == table.void_type_idx { @@ -795,7 +846,9 @@ pub fn (c mut Checker) expr(node ast.Expr) table.Type { return it.typ } ast.CallExpr { - return c.call_expr(mut it) + call_ret_type := c.call_expr(mut it) + c.check_or_block(mut it, call_ret_type) + return call_ret_type } ast.CharLiteral { return table.byte_type diff --git a/vlib/v/parser/fn.v b/vlib/v/parser/fn.v index 285cf91333..b84244ebbb 100644 --- a/vlib/v/parser/fn.v +++ b/vlib/v/parser/fn.v @@ -22,6 +22,7 @@ pub fn (p mut Parser) call_expr(is_c bool, mod string) ast.CallExpr { p.check(.lpar) args := p.call_args() mut or_stmts := []ast.Stmt + mut is_or_block_used := false if p.tok.kind == .key_orelse { p.next() p.open_scope() @@ -33,6 +34,7 @@ pub fn (p mut Parser) call_expr(is_c bool, mod string) ast.CallExpr { name: 'errcode' typ: table.int_type }) + is_or_block_used = true or_stmts = p.parse_block_no_scope() p.close_scope() } @@ -43,8 +45,9 @@ pub fn (p mut Parser) call_expr(is_c bool, mod string) ast.CallExpr { pos: tok.position() is_c: is_c or_block: ast.OrExpr{ - stmts: or_stmts - } + stmts: or_stmts + is_used: is_or_block_used + } } return node } diff --git a/vlib/v/parser/parser.v b/vlib/v/parser/parser.v index cf3d55c181..eeb11d0bb8 100644 --- a/vlib/v/parser/parser.v +++ b/vlib/v/parser/parser.v @@ -434,9 +434,11 @@ pub fn (p mut Parser) stmt() ast.Stmt { name: name } } + epos := p.tok.position() expr := p.expr(0) return ast.ExprStmt{ expr: expr + pos: epos } } } @@ -943,6 +945,7 @@ fn (p mut Parser) dot_expr(left ast.Expr) ast.Expr { p.next() args := p.call_args() mut or_stmts := []ast.Stmt + mut is_or_block_used := false if p.tok.kind == .key_orelse { p.next() p.open_scope() @@ -954,6 +957,7 @@ fn (p mut Parser) dot_expr(left ast.Expr) ast.Expr { name: 'err' typ: table.string_type }) + is_or_block_used = true or_stmts = p.parse_block_no_scope() p.close_scope() } @@ -964,8 +968,9 @@ fn (p mut Parser) dot_expr(left ast.Expr) ast.Expr { pos: pos is_method: true or_block: ast.OrExpr{ - stmts: or_stmts - } + stmts: or_stmts + is_used: is_or_block_used + } } mut node := ast.Expr{} node = mcall_expr diff --git a/vlib/v/tests/defer_test.v b/vlib/v/tests/defer_test.v index 1b2b7b4419..ee2cb21d94 100644 --- a/vlib/v/tests/defer_test.v +++ b/vlib/v/tests/defer_test.v @@ -56,10 +56,7 @@ fn test_defer_early_exit() { } fn test_defer_option() { - mut ok := Num{ - 0} - set_num_opt(mut ok) or { - assert false - } + mut ok := Num{0} + set_num_opt(mut ok) assert ok.val == 1 }