From 8c241cb7451591f1fb1b07c2ab04d5dea776e644 Mon Sep 17 00:00:00 2001 From: Delyan Angelov Date: Wed, 11 Nov 2020 17:23:57 +0200 Subject: [PATCH] checker: check that fns with return values, and matches, do return --- vlib/builtin/map.v | 2 + vlib/sync/channels.v | 6 ++- vlib/v/checker/checker.v | 48 ++++++++++++------- .../tests/return_missing_match_simple.out | 5 ++ .../tests/return_missing_match_simple.vv | 12 +++++ vlib/v/scanner/scanner.v | 1 + 6 files changed, 55 insertions(+), 19 deletions(-) create mode 100644 vlib/v/checker/tests/return_missing_match_simple.out create mode 100644 vlib/v/checker/tests/return_missing_match_simple.vv diff --git a/vlib/builtin/map.v b/vlib/builtin/map.v index 33bb510151..9347fab3c2 100644 --- a/vlib/builtin/map.v +++ b/vlib/builtin/map.v @@ -407,6 +407,8 @@ fn (mut m map) get_and_set(key string, zero voidptr) voidptr { // Key not found, insert key with zero-value m.set(key, zero) } + assert false + return voidptr(0) } // If `key` matches the key of an element in the container, diff --git a/vlib/sync/channels.v b/vlib/sync/channels.v index 67c5e02cce..fcf5438b90 100644 --- a/vlib/sync/channels.v +++ b/vlib/sync/channels.v @@ -325,6 +325,9 @@ fn (mut ch Channel) try_push_priv(src voidptr, no_block bool) ChanState { } } } + // this should not happen + assert false + return .success } [inline] @@ -494,8 +497,9 @@ fn (mut ch Channel) try_pop_priv(dest voidptr, no_block bool) ChanState { dest2 = dest } } - return .success + break } + return .success } // Wait `timeout` on any of `channels[i]` until one of them can push (`is_push[i] = true`) or pop (`is_push[i] = false`) diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index 0ba5ab149c..f64397d088 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -3357,8 +3357,8 @@ pub fn (mut c Checker) match_expr(mut node ast.MatchExpr) table.Type { c.match_exprs(mut node, cond_type_sym) c.expected_type = cond_type mut ret_type := table.void_type - mut require_return := false - mut branch_without_return := false + mut nbranches_with_return := 0 + mut nbranches_without_return := 0 for branch in node.branches { c.stmts(branch.stmts) if node.is_expr && branch.stmts.len > 0 { @@ -3392,17 +3392,21 @@ pub fn (mut c Checker) match_expr(mut node ast.MatchExpr) table.Type { } if has_return := c.has_return(branch.stmts) { if has_return { - require_return = true + nbranches_with_return++ } else { - branch_without_return = true + nbranches_without_return++ } } } - if require_return && branch_without_return { - c.returns = false - } else { - // if inner if branch has not covered all branches but this one - c.returns = true + if nbranches_with_return > 0 { + if nbranches_with_return == node.branches.len { + // an exhaustive match, and all branches returned + c.returns = true + } + if nbranches_without_return > 0 { + // some of the branches did not return + c.returns = false + } } // if ret_type != table.void_type { // node.is_expr = c.expected_type != table.void_type @@ -3692,8 +3696,8 @@ pub fn (mut c Checker) if_expr(mut node ast.IfExpr) table.Type { expr_required := c.expected_type != table.void_type former_expected_type := c.expected_type node.typ = table.void_type - mut require_return := false - mut branch_without_return := false + mut nbranches_with_return := 0 + mut nbranches_without_return := 0 mut should_skip := false // Whether the current branch should be skipped mut found_branch := false // Whether a matching branch was found- skip the rest for i in 0 .. node.branches.len { @@ -3859,17 +3863,25 @@ pub fn (mut c Checker) if_expr(mut node ast.IfExpr) table.Type { // Also check for returns inside a comp.if's statements, even if its contents aren't parsed if has_return := c.has_return(branch.stmts) { if has_return { - require_return = true + nbranches_with_return++ } else { - branch_without_return = true + nbranches_without_return++ } } } - if require_return && (!node.has_else || branch_without_return) { - c.returns = false - } else { - // if inner if branch has not covered all branches but this one - c.returns = true + if nbranches_with_return > 0 { + if nbranches_with_return == node.branches.len { + // if/else... where all branches returned + c.returns = true + } + if !node.has_else { + // `if cond { return ... }` means that when cond is false, execution continues + c.returns = false + } + if nbranches_without_return > 0 { + // some of the branches did not return + c.returns = false + } } // if only untyped literals were given default to int/f64 if node.typ == table.any_int_type { diff --git a/vlib/v/checker/tests/return_missing_match_simple.out b/vlib/v/checker/tests/return_missing_match_simple.out new file mode 100644 index 0000000000..89eb3ccda9 --- /dev/null +++ b/vlib/v/checker/tests/return_missing_match_simple.out @@ -0,0 +1,5 @@ +vlib/v/checker/tests/return_missing_match_simple.vv:1:1: error: missing return at end of function `h` + 1 | fn h() int { + | ~~~~~~~~~~ + 2 | a := 1 + 3 | match a { diff --git a/vlib/v/checker/tests/return_missing_match_simple.vv b/vlib/v/checker/tests/return_missing_match_simple.vv new file mode 100644 index 0000000000..463c2b20f0 --- /dev/null +++ b/vlib/v/checker/tests/return_missing_match_simple.vv @@ -0,0 +1,12 @@ +fn h() int { + a := 1 + match a { + else { println('abc') } + } + println('hello') +} + +fn main() { + d := h() + println('h returned: $d') +} diff --git a/vlib/v/scanner/scanner.v b/vlib/v/scanner/scanner.v index 8a556aa494..b56d2a3ac0 100644 --- a/vlib/v/scanner/scanner.v +++ b/vlib/v/scanner/scanner.v @@ -500,6 +500,7 @@ pub fn (mut s Scanner) buffer_scan() token.Token { } return s.all_tokens[cidx] } + return s.new_token(.eof, '', 1) } [inline]