From 00645aa246e8ebedd74a1534aa500c1d9776811f Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Sat, 30 May 2026 05:37:13 +0300 Subject: [PATCH] checker: preserve option index or values --- vlib/v/checker/checker.v | 60 ++++++++++---- vlib/v/gen/c/fn.v | 4 + vlib/v/gen/c/index.v | 37 +++++++-- .../index_or_block_option_context_test.v | 78 +++++++++++++++++++ 4 files changed, 158 insertions(+), 21 deletions(-) create mode 100644 vlib/v/tests/options/index_or_block_option_context_test.v diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index 981dbe439..91adbfcca 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -2642,11 +2642,9 @@ fn (mut c Checker) check_expr_option_or_result_call(expr ast.Expr, ret_type ast. // since it was already validated. This prevents the duplicate call // from assign.v from rejecting valid `none` in option map or blocks. if expr.typ != 0 { - if ret_type.has_flag(.option) && expr.or_expr.stmts.len > 0 { - last := expr.or_expr.stmts.last() - if last is ast.ExprStmt && last.expr is ast.None { - return ret_type - } + if ret_type.has_flag(.option) + && index_or_block_returns_option_value(expr.or_expr) { + return ret_type } return ret_type.clear_option_and_result() } @@ -2667,13 +2665,10 @@ fn (mut c Checker) check_expr_option_or_result_call(expr ast.Expr, ret_type ast. c.check_or_expr(expr.or_expr, ret_type, ret_type.set_flag(.result), expr) c.cur_or_expr = last_cur_or_expr } - // When the map value type is itself optional and the or block - // returns none, preserve the option type so the result is ?T. - if ret_type.has_flag(.option) && expr.or_expr.stmts.len > 0 { - last := expr.or_expr.stmts.last() - if last is ast.ExprStmt && last.expr is ast.None { - return ret_type - } + // When the index expression is expected to produce an option and the + // or block returns `none`/`?T`, preserve the option type. + if ret_type.has_flag(.option) && index_or_block_returns_option_value(expr.or_expr) { + return ret_type } return ret_type.clear_option_and_result() } else if expr.left is ast.SelectorExpr && expr.left_type.has_option_or_result() { @@ -2708,6 +2703,30 @@ fn (mut c Checker) check_expr_option_or_result_call(expr ast.Expr, ret_type ast. return ret_type } +fn index_or_block_returns_option_value(or_expr ast.OrExpr) bool { + if or_expr.stmts.len == 0 { + return false + } + last := or_expr.stmts.last() + if last is ast.ExprStmt { + return last.expr is ast.None || last.typ.has_flag(.option) + } + return false +} + +fn (mut c Checker) index_or_expr_expected_type(value_type ast.Type, expected_type ast.Type) ast.Type { + expected_unwrapped_type := c.unwrap_generic(expected_type) + if expected_unwrapped_type.has_flag(.option) { + expected_value_type := + c.table.fully_unaliased_type(expected_unwrapped_type).clear_option_and_result() + unwrapped_value_type := c.table.fully_unaliased_type(value_type).clear_option_and_result() + if expected_value_type == unwrapped_value_type { + return expected_type + } + } + return value_type +} + fn (mut c Checker) expr_unhandled_option_type(expr ast.Expr) ast.Type { match expr { ast.Ident { @@ -2831,8 +2850,10 @@ fn (mut c Checker) check_or_last_stmt(mut stmt ast.Stmt, ret_type ast.Type, defa if last_stmt_typ.has_flag(.option) || last_stmt_typ == ast.none_type { if stmt.expr in [ast.Ident, ast.SelectorExpr, ast.CallExpr, ast.None, ast.CastExpr] && !(last_stmt_typ == ast.none_type && allow_none_as_option_value - && default_or_type.has_flag(.option)) && !(last_stmt_typ == ast.none_type - && c.inside_return && ret_type.has_flag(.option)) { + && default_or_type.has_flag(.option)) && !(last_stmt_typ.has_flag(.option) + && allow_none_as_option_value && default_or_type.has_flag(.option)) + && !(last_stmt_typ == ast.none_type && c.inside_return + && ret_type.has_flag(.option)) { expected_type_name := c.table.type_to_str(default_or_type) got_type_name := c.table.type_to_str(last_stmt_typ) c.error('`or` block must provide a value of type `${expected_type_name}`, not `${got_type_name}`', @@ -5379,6 +5400,8 @@ fn (mut c Checker) cast_expr(mut node ast.CastExpr) ast.Type { c.expected_type = ast.void_type } else if node.expr is ast.Ident && node.expr.language == .c { c.expected_type = base_to_type + } else if node.expr is ast.IndexExpr && to_type.has_flag(.option) { + c.expected_type = to_type } expr_is_ident_or_cast := node.expr is ast.Ident || node.expr is ast.CastExpr node.expr_type = c.expr(mut node.expr) // type to be casted @@ -8071,6 +8094,7 @@ fn (c &Checker) map_key_expected_msg(got ast.Type, expected ast.Type, key_expr a } fn (mut c Checker) index_expr(mut node ast.IndexExpr) ast.Type { + expected_type := c.expected_type mut typ := c.expr(mut node.left) if typ == 0 { c.error('unknown type for expression `${node.left}`', node.pos) @@ -8303,11 +8327,15 @@ fn (mut c Checker) index_expr(mut node ast.IndexExpr) ast.Type { typ = c.unwrap_generic(value_type) } } + mut or_expr_type := typ if node.or_expr.stmts.len > 0 && node.or_expr.stmts.last() is ast.ExprStmt { - c.expected_or_type = typ + if !c.inside_return { + or_expr_type = c.index_or_expr_expected_type(typ, expected_type) + } + c.expected_or_type = or_expr_type } c.stmts_ending_with_expression(mut node.or_expr.stmts, c.expected_or_type) - typ = c.check_expr_option_or_result_call(node, typ) + typ = c.check_expr_option_or_result_call(node, or_expr_type) node.typ = typ return typ } diff --git a/vlib/v/gen/c/fn.v b/vlib/v/gen/c/fn.v index e412df817..ec92a84c1 100644 --- a/vlib/v/gen/c/fn.v +++ b/vlib/v/gen/c/fn.v @@ -6942,6 +6942,10 @@ fn (mut g Gen) ref_or_deref_arg_ex(arg ast.CallArg, expected_type_ ast.Type, lan if arg.expr is ast.IndexExpr && arg.expr.index is ast.RangeExpr && arg_typ.is_ptr() { arg_typ = arg_typ.deref() } + if arg.expr is ast.IndexExpr && expected_type.has_flag(.option) + && arg.expr.typ.has_flag(.option) { + arg_typ = g.unwrap_generic(g.recheck_concrete_type(arg.expr.typ)) + } arg_sym := g.table.sym(arg_typ) exp_is_ptr := expected_type.is_any_kind_of_pointer() arg_is_ptr := arg_typ.is_any_kind_of_pointer() diff --git a/vlib/v/gen/c/index.v b/vlib/v/gen/c/index.v index b8e4a092d..aeb9da224 100644 --- a/vlib/v/gen/c/index.v +++ b/vlib/v/gen/c/index.v @@ -221,6 +221,14 @@ fn (mut g Gen) index_expr(node ast.IndexExpr) { } } +fn (mut g Gen) index_or_expr_keeps_option(node ast.IndexExpr) bool { + if !node.typ.has_flag(.option) || node.or_expr.kind != .block { + return false + } + or_value_type := g.resolved_or_block_value_type(node.or_expr) + return or_value_type in [ast.none_type, ast.none_type_idx] || or_value_type.has_flag(.option) +} + fn (mut g Gen) index_range_expr(node ast.IndexExpr, range ast.RangeExpr) { mut resolved_left_type := ast.Type(0) if node.left is ast.Ident { @@ -363,7 +371,7 @@ fn (mut g Gen) index_of_array(node ast.IndexExpr, sym ast.TypeSymbol) { } else { resolved_elem_type_ } - elem_type := if resolved_elem_type != 0 && resolved_elem_type != ast.void_type { + mut elem_type := if resolved_elem_type != 0 && resolved_elem_type != ast.void_type { resolved_elem_type } else if info.elem_type == ast.int_literal_type { ast.int_type @@ -372,6 +380,9 @@ fn (mut g Gen) index_of_array(node ast.IndexExpr, sym ast.TypeSymbol) { } else { info.elem_type } + if gen_or && node.typ.has_flag(.option) && !info.elem_type.has_flag(.option) { + elem_type = elem_type.clear_option_and_result() + } elem_sym := g.table.final_sym(elem_type) mut left_is_ptr := array_left_type.is_ptr() || node.left.is_auto_deref_var() // Auto-heap idents are pointers in C. `g.expr` writes `(*(name))` to deref them, @@ -582,6 +593,7 @@ fn (mut g Gen) index_of_array(node ast.IndexExpr, sym ast.TypeSymbol) { } if gen_or { g.writeln(';') + keep_option_result := g.index_or_expr_keeps_option(node) opt_elem_type := g.styp(elem_type.set_flag(.option)) g.writeln('${opt_elem_type} ${tmp_opt} = {0};') g.writeln('if (${tmp_opt_ptr}) {') @@ -599,13 +611,19 @@ fn (mut g Gen) index_of_array(node ast.IndexExpr, sym ast.TypeSymbol) { g.writeln('\t${tmp_opt}.state = 2; ${tmp_opt}.err = builtin___v_error(_S("array index out of range"));') g.writeln('}') if !node.is_option { - g.or_block(tmp_opt, node.or_expr, elem_type) + if keep_option_result { + g.or_block_on_value(tmp_opt, node.or_expr, elem_type.set_flag(.option)) + } else { + g.or_block(tmp_opt, node.or_expr, elem_type) + } } if is_gen_or_and_assign_rhs { g.set_current_pos_as_last_stmt_pos() } if !g.is_amp { - if g.inside_opt_or_res && elem_type.has_flag(.option) && g.inside_assign { + if keep_option_result { + g.write('\n${cur_line}${tmp_opt}') + } else if g.inside_opt_or_res && elem_type.has_flag(.option) && g.inside_assign { g.write('\n${cur_line}(*(${elem_type_str}*)&${tmp_opt})') } else if elem_type.has_flag(.option) && !g.inside_opt_or_res { g.write('\n${cur_line}(*(${result_type_str}*)${tmp_opt}.data)') @@ -914,6 +932,7 @@ fn (mut g Gen) index_of_map(node ast.IndexExpr, sym ast.TypeSymbol) { g.write('\n${cur_line}(*(${plain_val_type_str}*)${tmp_opt}.data)') } } else { + keep_option_result := g.index_or_expr_keeps_option(node) opt_val_type := g.styp(val_type.set_flag(.option)) g.writeln('${opt_val_type} ${tmp_opt} = {0};') g.writeln('if (${tmp_opt_ptr}) {') @@ -926,12 +945,20 @@ fn (mut g Gen) index_of_map(node ast.IndexExpr, sym ast.TypeSymbol) { g.writeln('\t${tmp_opt}.state = 2; ${tmp_opt}.err = builtin___v_error(_S("map key does not exist"));') g.writeln('}') if !node.is_option { - g.or_block(tmp_opt, node.or_expr, val_type) + if keep_option_result { + g.or_block_on_value(tmp_opt, node.or_expr, val_type.set_flag(.option)) + } else { + g.or_block(tmp_opt, node.or_expr, val_type) + } } if is_gen_or_and_assign_rhs { g.set_current_pos_as_last_stmt_pos() } - g.write('\n${cur_line}(*(${val_type_str}*)${tmp_opt}.data)') + if keep_option_result { + g.write('\n${cur_line}${tmp_opt}') + } else { + g.write('\n${cur_line}(*(${val_type_str}*)${tmp_opt}.data)') + } } } } diff --git a/vlib/v/tests/options/index_or_block_option_context_test.v b/vlib/v/tests/options/index_or_block_option_context_test.v new file mode 100644 index 000000000..95948493c --- /dev/null +++ b/vlib/v/tests/options/index_or_block_option_context_test.v @@ -0,0 +1,78 @@ +struct Integration { + id int +} + +fn accepts_option(value ?Integration) bool { + return value == none +} + +fn test_array_index_or_none_assigned_to_option() { + integrations := []Integration{} + mut current := ?Integration{} + current = integrations[0] or { none } + assert current == none +} + +fn test_array_index_or_option_literal_assigned_to_option() { + integrations := []Integration{} + mut current := ?Integration{} + current = integrations[0] or { ?Integration{} } + assert current == none +} + +fn test_chained_array_index_or_none_assigned_to_option() { + integrations := [Integration{ + id: 1 + }, Integration{ + id: 2 + }] + mut current := ?Integration{} + current = integrations.filter(it.id == 1)[0] or { none } + if value := current { + assert value.id == 1 + } else { + assert false + } +} + +fn test_chained_array_index_or_option_literal_assigned_to_option() { + integrations := [Integration{ + id: 1 + }, Integration{ + id: 2 + }] + mut current := ?Integration{} + current = integrations.filter(it.id == 3)[0] or { ?Integration{} } + assert current == none +} + +fn test_array_index_or_none_keeps_present_value() { + integrations := [Integration{ + id: 7 + }] + mut current := ?Integration{} + current = integrations[0] or { none } + if value := current { + assert value.id == 7 + } else { + assert false + } +} + +fn test_map_index_or_none_assigned_to_option() { + integrations := map[string]Integration{} + mut current := ?Integration{} + current = integrations['missing'] or { none } + assert current == none +} + +fn test_index_or_none_in_option_cast() { + integrations := []Integration{} + current := ?Integration(integrations[0] or { none }) + assert current == none +} + +fn test_index_or_none_in_option_argument() { + integrations := []Integration{} + assert accepts_option(integrations[0] or { none }) +} -- 2.39.5