Skip to content

Commit

Permalink
checker, orm: add error for unchecked option multi return types, fix …
Browse files Browse the repository at this point in the history
…undefined behavior (#21106)
  • Loading branch information
ttytm committed Mar 28, 2024
1 parent f172a04 commit b98dca5
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 32 deletions.
11 changes: 6 additions & 5 deletions vlib/orm/orm.v
Original file line number Diff line number Diff line change
Expand Up @@ -508,12 +508,13 @@ pub fn orm_table_gen(table string, q string, defaults bool, def_unique_len int,
return error("references attribute needs to be in the format [references], [references: 'tablename'], or [references: 'tablename(field_id)']")
}
if attr.arg.contains('(') {
ref_table, ref_field := attr.arg.split_once('(')
if !ref_field.ends_with(')') {
return error("explicit references attribute should be written as [references: 'tablename(field_id)']")
if ref_table, ref_field := attr.arg.split_once('(') {
if !ref_field.ends_with(')') {
return error("explicit references attribute should be written as [references: 'tablename(field_id)']")
}
references_table = ref_table
references_field = ref_field[..ref_field.len - 1]
}
references_table = ref_table
references_field = ref_field[..ref_field.len - 1]
} else {
references_table = attr.arg
references_field = 'id'
Expand Down
21 changes: 12 additions & 9 deletions vlib/v/checker/checker.v
Original file line number Diff line number Diff line change
Expand Up @@ -1161,17 +1161,20 @@ fn (mut c Checker) check_expr_option_or_result_call(expr ast.Expr, ret_type ast.
}
}
if expr_ret_type.has_option_or_result() {
if expr.or_block.kind == .absent && expr_ret_type.has_flag(.result) {
if expr.or_block.kind == .absent {
ret_sym := c.table.sym(expr_ret_type)
ret_typ_tok := if expr_ret_type.has_flag(.option) { '?' } else { '!' }
if c.inside_defer {
c.error('${expr.name}() returns `${ret_typ_tok}${ret_sym.name}`, so it should have an `or {}` block at the end',
expr.pos)
} else {
c.error('${expr.name}() returns `${ret_typ_tok}${ret_sym.name}`, so it should have either an `or {}` block, or `${ret_typ_tok}` at the end',
expr.pos)
if expr_ret_type.has_flag(.result)
|| (expr_ret_type.has_flag(.option) && ret_sym.kind == .multi_return) {
ret_typ_tok := if expr_ret_type.has_flag(.option) { '?' } else { '!' }
if c.inside_defer {
c.error('${expr.name}() returns `${ret_typ_tok}${ret_sym.name}`, so it should have an `or {}` block at the end',
expr.pos)
} else {
c.error('${expr.name}() returns `${ret_typ_tok}${ret_sym.name}`, so it should have either an `or {}` block, or `${ret_typ_tok}` at the end',
expr.pos)
}
}
} else if expr.or_block.kind != .absent {
} else {
c.check_or_expr(expr.or_block, ret_type, expr_ret_type, expr)
}
return ret_type.clear_flag(.result)
Expand Down
7 changes: 7 additions & 0 deletions vlib/v/checker/tests/option_multi_return_err.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
vlib/v/checker/tests/option_multi_return_err.vv:6:14: error: split() returns `?(string, string)`, so it should have either an `or {}` block, or `?` at the end
4 |
5 | fn main() {
6 | foo, bar := split('foo/bar')
| ~~~~~~~~~~~~~~~~
7 | println('${foo}.${bar}')
8 | }
8 changes: 8 additions & 0 deletions vlib/v/checker/tests/option_multi_return_err.vv
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
fn split(str string) ?(string, string) {
return none
}

fn main() {
foo, bar := split('foo/bar')
println('${foo}.${bar}')
}
41 changes: 27 additions & 14 deletions vlib/v/tests/option_multi_return_test.v
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
struct MmapRangeLocal {}

fn addr2range() ?(&MmapRangeLocal, ?u64, u64) {
return none
fn addr2range(none_ bool) ?(&MmapRangeLocal, ?u64, u64) {
if none_ {
return none
} else {
return &MmapRangeLocal{}, none, 1
}
}

fn foo(val ?int) (?int, ?int) {
Expand All @@ -23,39 +27,48 @@ fn tuple2() ?(string, int) {
}

fn tuple3() ?(?int, ?int) {
return none, none
return none
}

fn tuple4() ?(?int, ?int) {
return none
fn tuple4(none_ bool) ?(?int, ?int) {
if none_ {
return none, none
} else {
return 1, none
}
}

fn test_tuple_1() {
a, b := tuple()
a, b := tuple()?
assert a == 1
assert b == 2
}

fn test_tuple_2() {
a, b := tuple2()
a, b := tuple2()?
assert a == ''
assert b == 2
}

fn test_tuple_3() {
a, b := tuple3()
assert a == none
assert b == none
a, b := tuple3() or { return }
assert false
}

fn test_tuple_4() {
a, b := tuple4()
assert a == none
if _, _ := tuple4(true) {
assert false
}
a, b := tuple4(false)?
assert a? == 1
assert b == none
}

fn test_none_ret() {
_, b, c := addr2range()
if _, _, _ := addr2range(true) {
assert false
}
_, b, c := addr2range(false)?
assert b == none
assert c == 0
assert c == 1
}
9 changes: 5 additions & 4 deletions vlib/v/tests/return_option_call_test.v
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ fn wrapper(data string) ?(int, string) {
}

fn test_return_option_call() {
dump(wrapper('issue'))
dump(wrapper('foobar'))
dump(wrapper(''))
assert true
dump(wrapper('issue')?)
dump(wrapper('foobar')?)
if a, b := wrapper('') {
assert false, '${a}, ${b}'
}
}

0 comments on commit b98dca5

Please sign in to comment.