Skip to content

Commit

Permalink
checker: fix mut var option unwrap with != none, support `if mut x …
Browse files Browse the repository at this point in the history
…!= none {` too (fix vlang#22936) (vlang#22943)
  • Loading branch information
felipensp authored Nov 24, 2024
1 parent cbfe1b8 commit e86b526
Show file tree
Hide file tree
Showing 10 changed files with 54 additions and 21 deletions.
2 changes: 2 additions & 0 deletions vlib/v/ast/ast.v
Original file line number Diff line number Diff line change
Expand Up @@ -1086,13 +1086,15 @@ 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 }
else { false }
}
}

@[inline]
pub fn (i &Ident) is_mut() bool {
match i.obj {
Var { return i.obj.is_mut }
Expand Down
8 changes: 4 additions & 4 deletions vlib/v/checker/checker.v
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion vlib/v/checker/for.v
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
Expand Down
10 changes: 5 additions & 5 deletions vlib/v/checker/if.v
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
}
}
}
Expand All @@ -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)
}
}
}
Expand Down
7 changes: 5 additions & 2 deletions vlib/v/checker/infix.v
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
Expand Down
2 changes: 1 addition & 1 deletion vlib/v/checker/match.v
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
Expand Down
13 changes: 9 additions & 4 deletions vlib/v/gen/c/cgen.v
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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)}*)(')
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions vlib/v/tests/generics/generic_selector_test.v
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
23 changes: 23 additions & 0 deletions vlib/v/tests/options/option_var_mut_test.v
Original file line number Diff line number Diff line change
@@ -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
}
}
2 changes: 1 addition & 1 deletion vlib/v/tests/options/option_var_test.v
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

0 comments on commit e86b526

Please sign in to comment.