Skip to content

Commit

Permalink
checker: fix missing or-block check for callexpr (fix #22835) (#22840)
Browse files Browse the repository at this point in the history
  • Loading branch information
felipensp authored Nov 13, 2024
1 parent 7c3c589 commit 62bdf99
Show file tree
Hide file tree
Showing 23 changed files with 294 additions and 37 deletions.
2 changes: 1 addition & 1 deletion vlib/net/websocket/utils.v
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ fn htonl64(payload_len u64) []u8 {

// create_masking_key returns a new masking key to use when masking websocket messages
fn create_masking_key() []u8 {
return rand.bytes(4) or { [0, 0, 0, 0] }
return rand.bytes(4) or { [u8(0), 0, 0, 0] }
}

// create_key_challenge_response creates a key challenge response from security key
Expand Down
10 changes: 8 additions & 2 deletions vlib/picohttpparser/misc_test.v
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ pub fn test_u64toa_large_values() {
mut buf := [20]u8{}

len := unsafe {
u64toa(&buf[0], v) or { assert err.msg() == 'Maximum size of 100MB exceeded!' }
u64toa(&buf[0], v) or {
assert err.msg() == 'Maximum size of 100MB exceeded!'
0
}
}

if v < 100_000_000 {
Expand All @@ -38,7 +41,10 @@ pub fn test_u64toa_edge_cases() {

// Test zero value
len := unsafe {
u64toa(&buf[0], 0) or { assert false }
u64toa(&buf[0], 0) or {
assert false
0
}
}

assert len == 1
Expand Down
30 changes: 23 additions & 7 deletions vlib/v/checker/checker.v
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ mut:
generic_fns map[string]bool // register generic fns that needs recheck once
inside_sql bool // to handle sql table fields pseudo variables
inside_selector_expr bool
inside_or_block_value bool // true inside or-block where its value is used `f(g() or { true })`
inside_interface_deref bool
inside_decl_rhs bool
inside_if_guard bool // true inside the guard condition of `if x := opt() {}`
Expand Down Expand Up @@ -1354,16 +1355,25 @@ fn (mut c Checker) check_or_expr(node ast.OrExpr, ret_type ast.Type, expr_return
return
}
if node.stmts.len == 0 {
if ret_type != ast.void_type {
// x := f() or {}
c.error('assignment requires a non empty `or {}` block', node.pos)
if expr is ast.CallExpr && expr.is_return_used {
// x := f() or {}, || f() or {} etc
c.error('expression requires a non empty `or {}` block', node.pos)
} else if expr !is ast.CallExpr && ret_type != ast.void_type {
// _ := sql db {... } or { }
c.error('expression requires a non empty `or {}` block', node.pos)
}
// allow `f() or {}`
return
}
mut valid_stmts := node.stmts.filter(it !is ast.SemicolonStmt)
mut last_stmt := if valid_stmts.len > 0 { valid_stmts.last() } else { node.stmts.last() }
c.check_or_last_stmt(mut last_stmt, ret_type, expr_return_type.clear_option_and_result())
if expr !is ast.CallExpr || (expr is ast.CallExpr && expr.is_return_used) {
// requires a block returning an unwrapped type of expr return type
c.check_or_last_stmt(mut last_stmt, ret_type, expr_return_type.clear_option_and_result())
} else {
// allow f() or { var = 123 }
c.check_or_last_stmt(mut last_stmt, ast.void_type, expr_return_type.clear_option_and_result())
}
}

fn (mut c Checker) check_or_last_stmt(mut stmt ast.Stmt, ret_type ast.Type, expr_return_type ast.Type) {
Expand All @@ -1372,6 +1382,10 @@ fn (mut c Checker) check_or_last_stmt(mut stmt ast.Stmt, ret_type ast.Type, expr
ast.ExprStmt {
c.expected_type = ret_type
c.expected_or_type = ret_type.clear_option_and_result()
if c.inside_or_block_value && stmt.expr is ast.None && ret_type.has_flag(.option) {
// return call() or { none } where fn returns an Option type
return
}
last_stmt_typ := c.expr(mut stmt.expr)
if last_stmt_typ.has_flag(.option) || last_stmt_typ == ast.none_type {
if stmt.expr in [ast.Ident, ast.SelectorExpr, ast.CallExpr, ast.None, ast.CastExpr] {
Expand Down Expand Up @@ -1437,9 +1451,11 @@ fn (mut c Checker) check_or_last_stmt(mut stmt ast.Stmt, ret_type ast.Type, expr
}
ast.Return {}
else {
expected_type_name := c.table.type_to_str(ret_type.clear_option_and_result())
c.error('last statement in the `or {}` block should be an expression of type `${expected_type_name}` or exit parent scope',
stmt.pos)
if stmt !is ast.AssertStmt || c.inside_or_block_value {
expected_type_name := c.table.type_to_str(ret_type.clear_option_and_result())
c.error('last statement in the `or {}` block should be an expression of type `${expected_type_name}` or exit parent scope',
stmt.pos)
}
}
}
} else if mut stmt is ast.ExprStmt {
Expand Down
10 changes: 9 additions & 1 deletion vlib/v/checker/fn.v
Original file line number Diff line number Diff line change
Expand Up @@ -737,9 +737,17 @@ fn (mut c Checker) call_expr(mut node ast.CallExpr) ast.Type {
}
}
}
old_expected_or_type := c.expected_or_type
c.expected_or_type = node.return_type.clear_flag(.result)
c.stmts_ending_with_expression(mut node.or_block.stmts, c.expected_or_type)
c.expected_or_type = ast.void_type

if node.or_block.kind == .block {
old_inside_or_block_value := c.inside_or_block_value
c.inside_or_block_value = true
c.check_or_expr(node.or_block, typ, c.expected_or_type, node)
c.inside_or_block_value = old_inside_or_block_value
}
c.expected_or_type = old_expected_or_type

if !c.inside_const && c.table.cur_fn != unsafe { nil } && !c.table.cur_fn.is_main
&& !c.table.cur_fn.is_test {
Expand Down
3 changes: 3 additions & 0 deletions vlib/v/checker/lambda_expr.v
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ pub fn (mut c Checker) lambda_expr(mut node ast.LambdaExpr, exp_typ ast.Type) as
is_expr: false
typ: return_type
}
if mut node.expr is ast.CallExpr && node.expr.is_return_used {
node.expr.is_return_used = false
}
} else {
stmts << ast.Return{
pos: node.pos
Expand Down
12 changes: 12 additions & 0 deletions vlib/v/checker/or_block_assert_err.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
vlib/v/checker/or_block_assert_err.vv:10:22: error: last statement in the `or {}` block should be an expression of type `int` or exit parent scope
8 |
9 | f() or { assert true }
10 | a := f() or { assert true }
| ~~~~~~
11 | dump(a)
12 | g(f() or { assert true })
vlib/v/checker/or_block_assert_err.vv:12:19: error: last statement in the `or {}` block should be an expression of type `int` or exit parent scope
10 | a := f() or { assert true }
11 | dump(a)
12 | g(f() or { assert true })
| ~~~~~~
12 changes: 12 additions & 0 deletions vlib/v/checker/or_block_assert_err.vv
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
fn g(x int) {
dump(x)
}

fn f() ?int {
return none
}

f() or { assert true }
a := f() or { assert true }
dump(a)
g(f() or { assert true })
56 changes: 56 additions & 0 deletions vlib/v/checker/tests/call_empty_or_block_err.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
vlib/v/checker/tests/call_empty_or_block_err.vv:9:2: warning: unused variable: `a`
7 |
8 | fn main() {
9 | a := foo() or { foo() or {} }
| ^
10 |
11 | // must be error
vlib/v/checker/tests/call_empty_or_block_err.vv:12:2: warning: unused variable: `y`
10 |
11 | // must be error
12 | y := if c := foo() {
| ^
13 | dump(c)
14 | bar() or {}
vlib/v/checker/tests/call_empty_or_block_err.vv:20:2: warning: unused variable: `z`
18 |
19 | // ok
20 | z := if d := foo() {
| ^
21 | dump(d)
22 | bar() or {}
vlib/v/checker/tests/call_empty_or_block_err.vv:29:2: warning: unused variable: `w`
27 |
28 | // ok
29 | w := foo() or {
| ^
30 | bar() or {}
31 | false
vlib/v/checker/tests/call_empty_or_block_err.vv:35:2: warning: unused variable: `b`
33 |
34 | // ok
35 | b := foo() or {
| ^
36 | foo() or {}
37 | false
vlib/v/checker/tests/call_empty_or_block_err.vv:9:24: error: expression requires a non empty `or {}` block
7 |
8 | fn main() {
9 | a := foo() or { foo() or {} }
| ~~~~~
10 |
11 | // must be error
vlib/v/checker/tests/call_empty_or_block_err.vv:14:9: error: expression requires a non empty `or {}` block
12 | y := if c := foo() {
13 | dump(c)
14 | bar() or {}
| ~~~~~
15 | } else {
16 | false
vlib/v/checker/tests/call_empty_or_block_err.vv:14:3: error: the final expression in `if` or `match`, must have a value of a non-void type
12 | y := if c := foo() {
13 | dump(c)
14 | bar() or {}
| ~~~~~
15 | } else {
16 | false
39 changes: 39 additions & 0 deletions vlib/v/checker/tests/call_empty_or_block_err.vv
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
fn foo() !bool {
return true
}

fn bar() ! {
}

fn main() {
a := foo() or { foo() or {} }

// must be error
y := if c := foo() {
dump(c)
bar() or {}
} else {
false
}

// ok
z := if d := foo() {
dump(d)
bar() or {}
true
} else {
false
}

// ok
w := foo() or {
bar() or {}
false
}

// ok
b := foo() or {
foo() or {}
false
}
}
6 changes: 6 additions & 0 deletions vlib/v/checker/tests/fn_call_or_block_err.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
vlib/v/checker/tests/fn_call_or_block_err.vv:7:18: error: wrong return type `int literal` in the `or {}` block, expected `string`
5 | }
6 |
7 | y := Aa(f() or { 2 })
| ^
8 | println(y)
8 changes: 8 additions & 0 deletions vlib/v/checker/tests/fn_call_or_block_err.vv
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
type Aa = string | int

fn f() !string {
return ''
}

y := Aa(f() or { 2 })
println(y)
14 changes: 14 additions & 0 deletions vlib/v/checker/tests/lambda_or_block_err.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
vlib/v/checker/tests/lambda_or_block_err.vv:10:10: error: cannot use `!int` as type `int` in return argument
8 |
9 | fn main() {
10 | foo(|i| bar(i))
| ~~~~~~
11 | foo(|i| bar(i) or {})
12 | foo(|i| bar(i) or { 0 })
vlib/v/checker/tests/lambda_or_block_err.vv:11:17: error: expression requires a non empty `or {}` block
9 | fn main() {
10 | foo(|i| bar(i))
11 | foo(|i| bar(i) or {})
| ~~~~~
12 | foo(|i| bar(i) or { 0 })
13 | }
13 changes: 13 additions & 0 deletions vlib/v/checker/tests/lambda_or_block_err.vv
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
fn foo(callback fn (int) int) {
dump([1, 2, 3].map(callback))
}

fn bar(a int) !int {
return a
}

fn main() {
foo(|i| bar(i))
foo(|i| bar(i) or {})
foo(|i| bar(i) or { 0 })
}
4 changes: 2 additions & 2 deletions vlib/v/checker/tests/or_block_check_err.out
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
vlib/v/checker/tests/or_block_check_err.vv:6:36: error: assignment requires a non empty `or {}` block
vlib/v/checker/tests/or_block_check_err.vv:6:36: error: expression requires a non empty `or {}` block
4 |
5 | fn main() {
6 | _ = callexpr_with_or_block_call() or {}.replace('a', 'b')
| ~~~~~
7 | _ = (callexpr_with_or_block_call() or {}).replace('a', 'b')
8 |
vlib/v/checker/tests/or_block_check_err.vv:7:37: error: assignment requires a non empty `or {}` block
vlib/v/checker/tests/or_block_check_err.vv:7:37: error: expression requires a non empty `or {}` block
5 | fn main() {
6 | _ = callexpr_with_or_block_call() or {}.replace('a', 'b')
7 | _ = (callexpr_with_or_block_call() or {}).replace('a', 'b')
Expand Down
2 changes: 1 addition & 1 deletion vlib/v/checker/tests/orm_no_default_value.out
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
vlib/v/checker/tests/orm_no_default_value.vv:11:4: error: assignment requires a non empty `or {}` block
vlib/v/checker/tests/orm_no_default_value.vv:11:4: error: expression requires a non empty `or {}` block
9 | _ := sql db {
10 | select from Person
11 | } or {
Expand Down
1 change: 1 addition & 0 deletions vlib/v/eval/eval.v
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@ pub fn (mut e Eval) register_symbol(stmt ast.Stmt, mod string, file string) {
}
}

@[noreturn]
fn (e &Eval) error(msg string) {
eprintln('> V interpreter backtrace:')
e.print_backtrace()
Expand Down
6 changes: 4 additions & 2 deletions vlib/v/gen/c/if.v
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ fn (mut g Gen) if_expr(node ast.IfExpr) {
// Always use this in -autofree, since ?: can have tmp expressions that have to be freed.
needs_tmp_var := g.need_tmp_var_in_if(node)
needs_conds_order := g.needs_conds_order(node)
tmp := if needs_tmp_var { g.new_tmp_var() } else { '' }
tmp := if node.typ != ast.void_type && needs_tmp_var { g.new_tmp_var() } else { '' }
mut cur_line := ''
mut raw_state := false
if needs_tmp_var {
Expand All @@ -210,7 +210,9 @@ fn (mut g Gen) if_expr(node ast.IfExpr) {
}
cur_line = g.go_before_last_stmt()
g.empty_line = true
g.writeln('${styp} ${tmp}; /* if prepend */')
if tmp != '' {
g.writeln('${styp} ${tmp}; /* if prepend */')
}
if g.infix_left_var_name.len > 0 {
g.writeln('if (${g.infix_left_var_name}) {')
g.indent++
Expand Down
4 changes: 3 additions & 1 deletion vlib/v/parser/expr.v
Original file line number Diff line number Diff line change
Expand Up @@ -787,7 +787,9 @@ fn (mut p Parser) infix_expr(left ast.Expr) ast.Expr {

right_op_pos := p.tok.pos()
old_assign_rhs := p.inside_assign_rhs
p.inside_assign_rhs = true
if op in [.decl_assign, .assign] {
p.inside_assign_rhs = true
}
right = p.expr(precedence)
p.inside_assign_rhs = old_assign_rhs
if op in [.plus, .minus, .mul, .div, .mod, .lt, .eq] && mut right is ast.PrefixExpr {
Expand Down
Loading

0 comments on commit 62bdf99

Please sign in to comment.