From 867f4376d8013aad70e524a74a58804ea12ae4e5 Mon Sep 17 00:00:00 2001 From: Turiiya <34311583+tobealive@users.noreply.github.com> Date: Wed, 21 Jun 2023 11:13:18 +0200 Subject: [PATCH] checker: abstract repetitive error handling (#18507) --- vlib/v/checker/assign.v | 19 +--- vlib/v/checker/checker.v | 90 +++++++++++-------- vlib/v/checker/return.v | 19 +--- vlib/v/checker/struct.v | 19 +--- vlib/v/checker/tests/go_wait_or.out | 4 +- .../checker/tests/struct_field_option_err.out | 2 +- .../checker/tests/unexpected_or_propagate.out | 2 +- 7 files changed, 59 insertions(+), 96 deletions(-) diff --git a/vlib/v/checker/assign.v b/vlib/v/checker/assign.v index 7d6481ff9..4a064b049 100644 --- a/vlib/v/checker/assign.v +++ b/vlib/v/checker/assign.v @@ -250,24 +250,7 @@ fn (mut c Checker) assign_stmt(mut node ast.AssignStmt) { } if right_type.is_ptr() && left_type.is_ptr() { if mut right is ast.Ident { - if mut right.obj is ast.Var { - mut obj := unsafe { &right.obj } - if c.fn_scope != unsafe { nil } { - obj = c.fn_scope.find_var(right.obj.name) or { obj } - } - if obj.is_stack_obj && !c.inside_unsafe { - type_sym := c.table.sym(obj.typ.set_nr_muls(0)) - if !type_sym.is_heap() && !c.pref.translated && !c.file.is_translated { - suggestion := if type_sym.kind == .struct_ { - 'declaring `${type_sym.name}` as `[heap]`' - } else { - 'wrapping the `${type_sym.name}` object in a `struct` declared as `[heap]`' - } - c.error('`${right.name}` cannot be assigned outside `unsafe` blocks as it might refer to an object stored on stack. Consider ${suggestion}.', - right.pos) - } - } - } + c.fail_if_stack_struct_action_outside_unsafe(mut right, 'assigned') } } // Do not allow `a := 0; b := 0; a = &b` diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index 94eebc431..e65901c20 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -1026,6 +1026,28 @@ fn (mut c Checker) type_implements(typ ast.Type, interface_type ast.Type, pos to return true } +fn (mut c Checker) expr_or_block_err(kind ast.OrKind, expr_name string, pos token.Pos, is_field bool) { + obj_does_not_return_or_is_not := if is_field { + 'field `${expr_name}` is not' + } else { + 'function `${expr_name}` does not return' + } + match kind { + .absent {} + .block { + c.error('unexpected `or` block, the ${obj_does_not_return_or_is_not} an Option or a Result', + pos) + } + .propagate_option { + c.error('unexpected `?`, the ${obj_does_not_return_or_is_not} an Option', + pos) + } + .propagate_result { + c.error('unexpected `!`, the ${obj_does_not_return_or_is_not} a Result', pos) + } + } +} + // return the actual type of the expression, once the option is handled fn (mut c Checker) check_expr_opt_call(expr ast.Expr, ret_type ast.Type) ast.Type { if expr is ast.CallExpr { @@ -1057,15 +1079,8 @@ fn (mut c Checker) check_expr_opt_call(expr ast.Expr, ret_type ast.Type) ast.Typ } } return ret_type.clear_flag(.result) - } else if expr.or_block.kind == .block { - c.error('unexpected `or` block, the function `${expr.name}` does not return an Option or a Result', - expr.or_block.pos) - } else if expr.or_block.kind == .propagate_option { - c.error('unexpected `?`, the function `${expr.name}` does not return an Option', - expr.or_block.pos) - } else if expr.or_block.kind == .propagate_result { - c.error('unexpected `!`, the function `${expr.name}` does not return a Result', - expr.or_block.pos) + } else { + c.expr_or_block_err(expr.or_block.kind, expr.name, expr.or_block.pos, false) } } else if expr is ast.SelectorExpr && c.table.sym(ret_type).kind != .chan { if expr.typ.has_flag(.option) || expr.typ.has_flag(.result) { @@ -1089,14 +1104,9 @@ fn (mut c Checker) check_expr_opt_call(expr ast.Expr, ret_type ast.Type) ast.Typ } } return ret_type.clear_flag(.result) - } else if expr.or_block.kind == .block { - c.error('unexpected `or` block, the field `${expr.field_name}` is neither an Option, nor a Result', - expr.or_block.pos) - } else if expr.or_block.kind == .propagate_option { - c.error('unexpected `?`, the field `${expr.field_name}` is not an Option', - expr.or_block.pos) - } else if expr.or_block.kind == .propagate_result { - c.error('unexpected `!`, Result fields are not supported', expr.or_block.pos) + } else { + c.expr_or_block_err(expr.or_block.kind, expr.field_name, expr.or_block.pos, + true) } } else if expr is ast.IndexExpr { if expr.or_expr.kind != .absent { @@ -2511,16 +2521,8 @@ pub fn (mut c Checker) expr(node_ ast.Expr) ast.Type { } } if !ret_type.has_flag(.option) && !ret_type.has_flag(.result) { - if node.or_block.kind == .block { - c.error('unexpected `or` block, the function `${node.name}` does not return an Option or a Result', - node.or_block.pos) - } else if node.or_block.kind == .propagate_option { - c.error('unexpected `?`, the function `${node.name}` does not return an Option or a Result', - node.or_block.pos) - } else if node.or_block.kind == .propagate_result { - c.error('unexpected `!`, the function `${node.name}` does not return an Option or a Result', - node.or_block.pos) - } + c.expr_or_block_err(node.or_block.kind, node.name, node.or_block.pos, + false) } if node.or_block.kind != .absent { if ret_type.has_flag(.option) { @@ -2710,18 +2712,9 @@ pub fn (mut c Checker) expr(node_ ast.Expr) ast.Type { if c.table.sym(ret_type).kind == .chan { return ret_type } - if !ret_type.has_flag(.option) && !ret_type.has_flag(.result) { - if node.or_block.kind == .block { - c.error('unexpected `or` block, the field `${node.field_name}` is neither an Option, nor a Result', - node.or_block.pos) - } else if node.or_block.kind == .propagate_option { - c.error('unexpected `?`, the field `${node.field_name}` is neither an Option, nor a Result', - node.or_block.pos) - } else if node.or_block.kind == .propagate_result { - c.error('unexpected `!`, the field `${node.field_name}` is neither an Option, nor a Result', - node.or_block.pos) - } + c.expr_or_block_err(node.or_block.kind, node.field_name, node.or_block.pos, + true) } if node.or_block.kind != .absent { if ret_type.has_flag(.option) { @@ -4721,6 +4714,27 @@ fn (mut c Checker) fail_if_unreadable(expr ast.Expr, typ ast.Type, what string) } } +fn (mut c Checker) fail_if_stack_struct_action_outside_unsafe(mut ident ast.Ident, failed_action string) { + if mut ident.obj is ast.Var { + mut obj := unsafe { &ident.obj } + if c.fn_scope != unsafe { nil } { + obj = c.fn_scope.find_var(ident.obj.name) or { obj } + } + if obj.is_stack_obj && !c.inside_unsafe { + sym := c.table.sym(obj.typ.set_nr_muls(0)) + if !sym.is_heap() && !c.pref.translated && !c.file.is_translated { + suggestion := if sym.kind == .struct_ { + 'declaring `${sym.name}` as `[heap]`' + } else { + 'wrapping the `${sym.name}` object in a `struct` declared as `[heap]`' + } + c.error('`${ident.name}` cannot be ${failed_action} outside `unsafe` blocks as it might refer to an object stored on stack. Consider ${suggestion}.', + ident.pos) + } + } + } +} + fn (mut c Checker) goto_label(node ast.GotoLabel) { // Register a goto label if node.name !in c.goto_labels { diff --git a/vlib/v/checker/return.v b/vlib/v/checker/return.v index 868f7f3ea..c3f0e825a 100644 --- a/vlib/v/checker/return.v +++ b/vlib/v/checker/return.v @@ -252,24 +252,7 @@ fn (mut c Checker) return_stmt(mut node ast.Return) { if exp_type.is_ptr() && got_type.is_ptr() { mut r_expr := &node.exprs[expr_idxs[i]] if mut r_expr is ast.Ident { - if mut r_expr.obj is ast.Var { - mut obj := unsafe { &r_expr.obj } - if c.fn_scope != unsafe { nil } { - obj = c.fn_scope.find_var(r_expr.obj.name) or { obj } - } - if obj.is_stack_obj && !c.inside_unsafe { - type_sym := c.table.sym(obj.typ.set_nr_muls(0)) - if !type_sym.is_heap() && !c.pref.translated && !c.file.is_translated { - suggestion := if type_sym.kind == .struct_ { - 'declaring `${type_sym.name}` as `[heap]`' - } else { - 'wrapping the `${type_sym.name}` object in a `struct` declared as `[heap]`' - } - c.error('`${r_expr.name}` cannot be returned outside `unsafe` blocks as it might refer to an object stored on stack. Consider ${suggestion}.', - r_expr.pos) - } - } - } + c.fail_if_stack_struct_action_outside_unsafe(mut r_expr, 'returned') } } } diff --git a/vlib/v/checker/struct.v b/vlib/v/checker/struct.v index b4ef2f26d..c6ca8712e 100644 --- a/vlib/v/checker/struct.v +++ b/vlib/v/checker/struct.v @@ -551,24 +551,7 @@ fn (mut c Checker) struct_init(mut node ast.StructInit, is_field_zero_struct_ini if got_type.is_ptr() && exp_type.is_ptr() { if mut field.expr is ast.Ident { - if mut field.expr.obj is ast.Var { - mut obj := unsafe { &field.expr.obj } - if c.fn_scope != unsafe { nil } { - obj = c.fn_scope.find_var(obj.name) or { obj } - } - if obj.is_stack_obj && !c.inside_unsafe { - sym := c.table.sym(obj.typ.set_nr_muls(0)) - if !sym.is_heap() && !c.pref.translated && !c.file.is_translated { - suggestion := if sym.kind == .struct_ { - 'declaring `${sym.name}` as `[heap]`' - } else { - 'wrapping the `${sym.name}` object in a `struct` declared as `[heap]`' - } - c.error('`${field.expr.name}` cannot be assigned outside `unsafe` blocks as it might refer to an object stored on stack. Consider ${suggestion}.', - field.expr.pos) - } - } - } + c.fail_if_stack_struct_action_outside_unsafe(mut field.expr, 'assigned') } } if field_info.typ in ast.unsigned_integer_type_idxs { diff --git a/vlib/v/checker/tests/go_wait_or.out b/vlib/v/checker/tests/go_wait_or.out index 56131300e..55372c8da 100644 --- a/vlib/v/checker/tests/go_wait_or.out +++ b/vlib/v/checker/tests/go_wait_or.out @@ -1,4 +1,4 @@ -vlib/v/checker/tests/go_wait_or.vv:11:16: error: unexpected `?`, the function `wait` does not return an Option or a Result +vlib/v/checker/tests/go_wait_or.vv:11:16: error: unexpected `?`, the function `wait` does not return an Option 9 | spawn d(1) 10 | ] 11 | r := tg.wait()? @@ -19,7 +19,7 @@ vlib/v/checker/tests/go_wait_or.vv:19:13: error: unexpected `or` block, the func | ~~~~~~~~~~~~~~~~~~~~~~~ 20 | tg2[0].wait()? 21 | tg3 := [ -vlib/v/checker/tests/go_wait_or.vv:20:15: error: unexpected `?`, the function `wait` does not return an Option or a Result +vlib/v/checker/tests/go_wait_or.vv:20:15: error: unexpected `?`, the function `wait` does not return an Option 18 | ] 19 | tg2.wait() or { panic('problem') } 20 | tg2[0].wait()? diff --git a/vlib/v/checker/tests/struct_field_option_err.out b/vlib/v/checker/tests/struct_field_option_err.out index 6bfb51ffa..4b9ba67e3 100644 --- a/vlib/v/checker/tests/struct_field_option_err.out +++ b/vlib/v/checker/tests/struct_field_option_err.out @@ -26,7 +26,7 @@ vlib/v/checker/tests/struct_field_option_err.vv:16:19: error: last statement in | ^ 17 | 18 | _ = f.baz? -vlib/v/checker/tests/struct_field_option_err.vv:18:11: error: unexpected `?`, the field `baz` is neither an Option, nor a Result +vlib/v/checker/tests/struct_field_option_err.vv:18:11: error: unexpected `?`, the field `baz` is not an Option 16 | _ = f.bar or { _ = 1 } 17 | 18 | _ = f.baz? diff --git a/vlib/v/checker/tests/unexpected_or_propagate.out b/vlib/v/checker/tests/unexpected_or_propagate.out index 3519140c7..fcc9fbc90 100644 --- a/vlib/v/checker/tests/unexpected_or_propagate.out +++ b/vlib/v/checker/tests/unexpected_or_propagate.out @@ -1,4 +1,4 @@ -vlib/v/checker/tests/unexpected_or_propagate.vv:6:17: error: unexpected `?`, the function `ret_zero` does not return an Option or a Result +vlib/v/checker/tests/unexpected_or_propagate.vv:6:17: error: unexpected `?`, the function `ret_zero` does not return an Option 4 | 5 | fn opt_fn() ?int { 6 | a := ret_zero()? -- 2.39.5