From e86b526947718221093690cd05af88d230cacea5 Mon Sep 17 00:00:00 2001 From: Felipe Pena Date: Sun, 24 Nov 2024 09:58:07 -0300 Subject: [PATCH] checker: fix mut var option unwrap with `!= none`, support `if mut x != none {` too (fix #22936) (#22943) --- vlib/v/ast/ast.v | 2 ++ vlib/v/checker/checker.v | 8 +++---- vlib/v/checker/for.v | 2 +- vlib/v/checker/if.v | 10 ++++---- vlib/v/checker/infix.v | 7 ++++-- vlib/v/checker/match.v | 2 +- vlib/v/gen/c/cgen.v | 13 +++++++---- vlib/v/tests/generics/generic_selector_test.v | 6 ++--- vlib/v/tests/options/option_var_mut_test.v | 23 +++++++++++++++++++ vlib/v/tests/options/option_var_test.v | 2 +- 10 files changed, 54 insertions(+), 21 deletions(-) create mode 100644 vlib/v/tests/options/option_var_mut_test.v diff --git a/vlib/v/ast/ast.v b/vlib/v/ast/ast.v index 90d7d992f84d85..3a0e66a333e961 100644 --- a/vlib/v/ast/ast.v +++ b/vlib/v/ast/ast.v @@ -1086,6 +1086,7 @@ pub fn (mut i Ident) full_name() string { return i.full_name } +@[inline] pub fn (i &Ident) is_auto_heap() bool { return match i.obj { Var { i.obj.is_auto_heap } @@ -1093,6 +1094,7 @@ pub fn (i &Ident) is_auto_heap() bool { } } +@[inline] pub fn (i &Ident) is_mut() bool { match i.obj { Var { return i.obj.is_mut } diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index 9da0900d9d9004..50a6604e082dd3 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -4172,7 +4172,7 @@ fn (mut c Checker) concat_expr(mut node ast.ConcatExpr) ast.Type { // smartcast takes the expression with the current type which should be smartcasted to the target type in the given scope fn (mut c Checker) smartcast(mut expr ast.Expr, cur_type ast.Type, to_type_ ast.Type, mut scope ast.Scope, - is_comptime bool) { + is_comptime bool, is_option_unwrap bool) { sym := c.table.sym(cur_type) to_type := if sym.kind == .interface && c.table.sym(to_type_).kind != .interface { to_type_.ref() @@ -4201,8 +4201,7 @@ fn (mut c Checker) smartcast(mut expr ast.Expr, cur_type ast.Type, to_type_ ast. smartcasts << field.smartcasts } // smartcast either if the value is immutable or if the mut argument is explicitly given - if !is_mut || expr.is_mut - || (cur_type.has_flag(.option) && cur_type.clear_flag(.option) == to_type_) { + if !is_mut || expr.is_mut || is_option_unwrap { smartcasts << to_type scope.register_struct_field(expr.expr.str(), ast.ScopeStructField{ struct_type: expr.expr_type @@ -4238,7 +4237,7 @@ fn (mut c Checker) smartcast(mut expr ast.Expr, cur_type ast.Type, to_type_ ast. } } // smartcast either if the value is immutable or if the mut argument is explicitly given - if (!is_mut || expr.is_mut) && !is_already_casted { + if (!is_mut || expr.is_mut || is_option_unwrap) && !is_already_casted { smartcasts << to_type if var := scope.find_var(expr.name) { if is_comptime && var.ct_type_var == .smartcast { @@ -4272,6 +4271,7 @@ fn (mut c Checker) smartcast(mut expr ast.Expr, cur_type ast.Type, to_type_ ast. is_used: true is_mut: expr.is_mut is_inherited: is_inherited + is_unwrapped: is_option_unwrap smartcasts: smartcasts orig_type: orig_type ct_type_var: ct_type_var diff --git a/vlib/v/checker/for.v b/vlib/v/checker/for.v index 779d0f731d2894..5efa621845bb23 100644 --- a/vlib/v/checker/for.v +++ b/vlib/v/checker/for.v @@ -289,7 +289,7 @@ fn (mut c Checker) for_stmt(mut node ast.ForStmt) { if node.cond.right is ast.TypeNode && node.cond.left in [ast.Ident, ast.SelectorExpr] { if c.table.type_kind(node.cond.left_type) in [.sum_type, .interface] { c.smartcast(mut node.cond.left, node.cond.left_type, node.cond.right_type, mut - node.scope, false) + node.scope, false, false) } } } diff --git a/vlib/v/checker/if.v b/vlib/v/checker/if.v index aaa8884e65874d..9832ee2a639a33 100644 --- a/vlib/v/checker/if.v +++ b/vlib/v/checker/if.v @@ -573,10 +573,10 @@ fn (mut c Checker) smartcast_if_conds(mut node ast.Expr, mut scope ast.Scope, co if node.left is ast.Ident && c.comptime.get_ct_type_var(node.left) == .smartcast { node.left_type = c.comptime.get_type(node.left) c.smartcast(mut node.left, node.left_type, node.left_type.clear_flag(.option), mut - scope, true) + scope, true, true) } else { c.smartcast(mut node.left, node.left_type, node.left_type.clear_flag(.option), mut - scope, false) + scope, false, true) } } else if node.op == .key_is { if node.left is ast.Ident && c.comptime.is_comptime_var(node.left) { @@ -646,7 +646,7 @@ fn (mut c Checker) smartcast_if_conds(mut node ast.Expr, mut scope ast.Scope, co } if left_sym.kind in [.interface, .sum_type] { c.smartcast(mut node.left, node.left_type, right_type, mut - scope, is_comptime) + scope, is_comptime, false) } } } @@ -666,10 +666,10 @@ fn (mut c Checker) smartcast_if_conds(mut node ast.Expr, mut scope ast.Scope, co && c.comptime.get_ct_type_var(first_cond.left) == .smartcast { first_cond.left_type = c.comptime.get_type(first_cond.left) c.smartcast(mut first_cond.left, first_cond.left_type, first_cond.left_type.clear_flag(.option), mut - scope, true) + scope, true, true) } else { c.smartcast(mut first_cond.left, first_cond.left_type, first_cond.left_type.clear_flag(.option), mut - scope, false) + scope, false, true) } } } diff --git a/vlib/v/checker/infix.v b/vlib/v/checker/infix.v index 8370984c8ed1bd..f4e27fa7084ddd 100644 --- a/vlib/v/checker/infix.v +++ b/vlib/v/checker/infix.v @@ -140,8 +140,11 @@ fn (mut c Checker) infix_expr(mut node ast.InfixExpr) ast.Type { if node.op != .key_is { match mut node.left { ast.Ident, ast.SelectorExpr { - if node.left.is_mut { - c.error('the `mut` keyword is invalid here', node.left.mut_pos) + // mut foo != none is allowed for unwrapping option + if !(node.op == .ne && node.right is ast.None) { + if node.left.is_mut { + c.error('the `mut` keyword is invalid here', node.left.mut_pos) + } } } else {} diff --git a/vlib/v/checker/match.v b/vlib/v/checker/match.v index 40bf3f274be902..b9d125494ca7ea 100644 --- a/vlib/v/checker/match.v +++ b/vlib/v/checker/match.v @@ -506,7 +506,7 @@ fn (mut c Checker) match_exprs(mut node ast.MatchExpr, cond_type_sym ast.TypeSym } c.smartcast(mut node.cond, node.cond_type, expr_type, mut branch.scope, - false) + false, false) } } } diff --git a/vlib/v/gen/c/cgen.v b/vlib/v/gen/c/cgen.v index 33e11919e1a6d9..8352c95e06bedc 100644 --- a/vlib/v/gen/c/cgen.v +++ b/vlib/v/gen/c/cgen.v @@ -4983,7 +4983,9 @@ fn (mut g Gen) ident(node ast.Ident) { // `x = new_opt()` => `x = new_opt()` (g.right_is_opt == true) // `println(x)` => `println(*(int*)x.data)` if node.info.is_option && !(g.is_assign_lhs && g.right_is_opt) { - if node.obj is ast.Var && node.obj.smartcasts.len > 0 { + has_smartcast := node.obj is ast.Var && node.obj.smartcasts.len > 0 + && !(node.obj.ct_type_var == .no_comptime && node.obj.is_unwrapped) + if has_smartcast { g.write('*(') } if (g.inside_opt_or_res || g.left_is_opt) && node.or_expr.kind == .absent { @@ -5030,7 +5032,7 @@ fn (mut g Gen) ident(node ast.Ident) { g.write(stmt_str) } if node.obj is ast.Var { - if node.obj.smartcasts.len > 0 { + if has_smartcast { obj_sym := g.table.sym(g.unwrap_generic(node.obj.typ)) if node.obj.ct_type_var == .smartcast { ctyp := g.unwrap_generic(g.comptime.get_type(node)) @@ -5068,7 +5070,7 @@ fn (mut g Gen) ident(node ast.Ident) { for i, typ in node.obj.smartcasts { is_option_unwrap := is_option && typ == node.obj.typ.clear_flag(.option) g.write('(') - if i == 0 && node.obj.is_unwrapped { + if i == 0 && node.obj.is_unwrapped && node.obj.ct_type_var == .smartcast { ctyp := g.unwrap_generic(g.comptime.get_type(node)) g.write('*(${g.base_type(ctyp)}*)(') } @@ -5104,7 +5106,7 @@ fn (mut g Gen) ident(node ast.Ident) { g.write(closure_ctx + '->') } if node.obj.typ.nr_muls() > 1 { - g.write2('(', '*'.repeat(node.obj.typ.nr_muls() - 1)) + g.write2('/**/(', '*'.repeat(node.obj.typ.nr_muls() - 1)) g.write2(name, ')') } else { g.write(name) @@ -5142,6 +5144,9 @@ fn (mut g Gen) ident(node ast.Ident) { } } } + if node.obj.ct_type_var != .smartcast && node.obj.is_unwrapped { + g.write('.data') + } g.write(')') } if is_auto_heap { diff --git a/vlib/v/tests/generics/generic_selector_test.v b/vlib/v/tests/generics/generic_selector_test.v index 4ba0226f22b89a..e86eb27403487e 100644 --- a/vlib/v/tests/generics/generic_selector_test.v +++ b/vlib/v/tests/generics/generic_selector_test.v @@ -26,11 +26,11 @@ pub fn (mut l List[T]) append(mut node Node[T]) ?int { mut curr_node := l.head for { - if curr_node != none { - if next_node := curr_node?.next { + if mut curr_node != none { + if next_node := curr_node.next { curr_node = next_node } else { - curr_node?.next = &node + curr_node.next = &node l.size = l.size + 1 break } diff --git a/vlib/v/tests/options/option_var_mut_test.v b/vlib/v/tests/options/option_var_mut_test.v new file mode 100644 index 00000000000000..e4936a3cc8c97b --- /dev/null +++ b/vlib/v/tests/options/option_var_mut_test.v @@ -0,0 +1,23 @@ +fn test_main() { + mut entrykey := ?string(none) + mut res := false + if entrykey != none { + println('entrykey is a string') + println(entrykey.len) + println(entrykey) + res = entrykey.len > 0 + } else { + assert true + } + assert !res +} + +fn test_non_none() { + mut entrykey := ?string('foobar') + if entrykey != none { + println(entrykey) + assert entrykey.len == 6 + } else { + assert false + } +} diff --git a/vlib/v/tests/options/option_var_test.v b/vlib/v/tests/options/option_var_test.v index c9ae4341afd546..d62f467dd8ecce 100644 --- a/vlib/v/tests/options/option_var_test.v +++ b/vlib/v/tests/options/option_var_test.v @@ -205,7 +205,7 @@ fn test_opt_none() { t7 := { 'foo': 'bar' } - t6 := if t5 != none { t5?.clone() } else { t7 } + t6 := if t5 != none { t5.clone() } else { t7 } assert t5 == none assert t6.len == 1 }