From 5bc6cc9512fb57c873eee8a6ca01b702815237cc Mon Sep 17 00:00:00 2001 From: Delyan Angelov Date: Wed, 15 Sep 2021 15:42:28 +0300 Subject: [PATCH] v.checker: fix spurious warning for `if x := map_of_sumtypes[k] {}` --- vlib/v/checker/checker.v | 29 +++++++++-------- .../require_or_block_sumtype_map.err.out | 10 +++--- .../tests/require_or_block_sumtype_map.err.vv | 3 ++ vlib/v/tests/map_sumtype_values_test.v | 31 +++++++++++++++++++ 4 files changed, 55 insertions(+), 18 deletions(-) create mode 100644 vlib/v/tests/map_sumtype_values_test.v diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index 9714362eff..87e38b6b99 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -73,16 +73,16 @@ pub mut: returns bool scope_returns bool mod string // current module name - is_builtin_mod bool // are we in `builtin`? - inside_unsafe bool - inside_const bool - inside_anon_fn bool - inside_ref_lit bool - inside_defer bool - inside_fn_arg bool // `a`, `b` in `a.f(b)` - inside_ct_attr bool // true inside [if expr] - skip_flags bool // should `#flag` and `#include` be skipped - fn_level int // 0 for the top level, 1 for `fn abc() {}`, 2 for a nested fn, etc + is_builtin_mod bool // true inside the 'builtin', 'os' or 'strconv' modules; TODO: remove the need for special casing this + inside_unsafe bool // true inside `unsafe {}` blocks + inside_const bool // true inside `const ( ... )` blocks + inside_anon_fn bool // true inside `fn() { ... }()` + inside_ref_lit bool // true inside `a := &something` + inside_defer bool // true inside `defer {}` blocks + inside_fn_arg bool // `a`, `b` in `a.f(b)` + inside_ct_attr bool // true inside `[if expr]` + skip_flags bool // should `#flag` and `#include` be skipped + fn_level int // 0 for the top level, 1 for `fn abc() {}`, 2 for a nested fn, etc ct_cond_stack []ast.Expr mut: files []ast.File @@ -104,6 +104,7 @@ mut: inside_selector_expr bool inside_println_arg bool inside_decl_rhs bool + inside_if_guard bool // true inside the guard condition of `if x := opt() {}` need_recheck_generic_fns bool // need recheck generic fns because there are cascaded nested generic fn } @@ -5480,7 +5481,10 @@ pub fn (mut c Checker) expr(node ast.Expr) ast.Type { return c.if_expr(mut node) } ast.IfGuardExpr { + old_inside_if_guard := c.inside_if_guard + c.inside_if_guard = true node.expr_type = c.expr(node.expr) + c.inside_if_guard = old_inside_if_guard if !node.expr_type.has_flag(.optional) { mut no_opt := true match mut node.expr { @@ -5997,8 +6001,7 @@ pub fn (mut c Checker) ident(mut node ast.Ident) ast.Type { if mut obj.expr is ast.IfGuardExpr { // new variable from if guard shouldn't have the optional flag for further use // a temp variable will be generated which unwraps it - if_guard_var_type := c.expr(obj.expr.expr) - typ = if_guard_var_type.clear_flag(.optional) + typ = obj.expr.expr_type.clear_flag(.optional) } else { typ = c.expr(obj.expr) } @@ -7462,7 +7465,7 @@ pub fn (mut c Checker) index_expr(mut node ast.IndexExpr) ast.Type { } value_sym := c.table.get_type_symbol(info.value_type) if !node.is_setter && value_sym.kind == .sum_type && node.or_expr.kind == .absent - && !c.inside_unsafe { + && !c.inside_unsafe && !c.inside_if_guard { c.warn('`or {}` block required when indexing a map with sum type value', node.pos) } diff --git a/vlib/v/checker/tests/require_or_block_sumtype_map.err.out b/vlib/v/checker/tests/require_or_block_sumtype_map.err.out index 7bfee678aa..2564654d08 100644 --- a/vlib/v/checker/tests/require_or_block_sumtype_map.err.out +++ b/vlib/v/checker/tests/require_or_block_sumtype_map.err.out @@ -1,6 +1,6 @@ -vlib/v/checker/tests/require_or_block_sumtype_map.err.vv:5:8: error: `or {}` block required when indexing a map with sum type value - 3 | fn main() { - 4 | x := map[string]Abc{} - 5 | _ := x['nonexisting'] +vlib/v/checker/tests/require_or_block_sumtype_map.err.vv:8:8: error: `or {}` block required when indexing a map with sum type value + 6 | println(y) + 7 | } + 8 | _ := x['nonexisting'] | ~~~~~~~~~~~~~~~ - 6 | } + 9 | } diff --git a/vlib/v/checker/tests/require_or_block_sumtype_map.err.vv b/vlib/v/checker/tests/require_or_block_sumtype_map.err.vv index ed4d806c71..a51e81a6cd 100644 --- a/vlib/v/checker/tests/require_or_block_sumtype_map.err.vv +++ b/vlib/v/checker/tests/require_or_block_sumtype_map.err.vv @@ -2,5 +2,8 @@ type Abc = string | int fn main() { x := map[string]Abc{} + if y := x['nonexisting'] { + println(y) + } _ := x['nonexisting'] } diff --git a/vlib/v/tests/map_sumtype_values_test.v b/vlib/v/tests/map_sumtype_values_test.v new file mode 100644 index 0000000000..01d27067a6 --- /dev/null +++ b/vlib/v/tests/map_sumtype_values_test.v @@ -0,0 +1,31 @@ +// NB: this test should be able to run without warnings/errors + +type SumType = bool | int | string + +fn test_reading_from_a_map_of_sumtype_values() { + mut values := map[string]SumType{} + values['abc'] = 'xyz' + values['xyz'] = 123 + values['xxx'] = true + println(unsafe { values['abcz'] }) // no warning/error, due to the unsafe{} + if value := values['abc'] { + eprintln('existing key `abc` is present, value: $value') + assert true + } + if (values['abc'] or { 'zzzz' }) is string { + eprintln('existing key `abc` is present, value is a string') + assert true + } + if (values['abc'] or { 123 }) is int { + eprintln('the existing keyed value is an int') + assert false + } + if (values['something else'] or { 'zzzz' }) is string { + eprintln('default value for non existing key is a string') + assert true + } + if (values['something else'] or { 123 }) is int { + eprintln('default value for non existing key is an int') + assert true + } +}