From b77e3914d82e2c2ca0400027cd125235c50ec231 Mon Sep 17 00:00:00 2001 From: shove Date: Wed, 10 Jan 2024 22:11:36 +0800 Subject: [PATCH] parser, checker, cgen: fix multidimensional fixed array size expression evaluation (fix #20311) (#20458) --- vlib/v/checker/containers.v | 141 ++++++++++++++---------- vlib/v/gen/c/cgen.v | 63 ++++++----- vlib/v/parser/parse_type.v | 7 +- vlib/v/tests/constant_array_size_test.v | 15 +++ 4 files changed, 132 insertions(+), 94 deletions(-) diff --git a/vlib/v/checker/containers.v b/vlib/v/checker/containers.v index 2bbe60107909aa..be6af09942594c 100644 --- a/vlib/v/checker/containers.v +++ b/vlib/v/checker/containers.v @@ -247,24 +247,71 @@ fn (mut c Checker) array_init(mut node ast.ArrayInit) ast.Type { node.elem_type = elem_type } else if node.is_fixed && node.exprs.len == 1 && node.elem_type != ast.void_type { // `[50]u8` - mut fixed_size := i64(0) mut init_expr := node.exprs[0] - c.expr(mut init_expr) - match mut init_expr { + node.typ = c.eval_array_fixed_sizes(mut init_expr, 0, node.elem_type) + node.elem_type = (c.table.sym(node.typ).info as ast.ArrayFixed).elem_type + if node.has_init { + c.check_array_init_default_expr(mut node) + } + } + return node.typ +} + +fn (mut c Checker) check_array_init_default_expr(mut node ast.ArrayInit) { + mut init_expr := node.init_expr + init_typ := c.check_expr_option_or_result_call(init_expr, c.expr(mut init_expr)) + node.init_type = init_typ + if !node.elem_type.has_flag(.option) && init_typ.has_flag(.option) { + c.error('cannot use unwrapped Option as initializer', init_expr.pos()) + } + c.check_expected(init_typ, node.elem_type) or { c.error(err.msg(), init_expr.pos()) } +} + +fn (mut c Checker) check_array_init_para_type(para string, mut expr ast.Expr, pos token.Pos) { + sym := c.table.sym(c.unwrap_generic(c.expr(mut expr))) + if sym.kind !in [.int, .int_literal] { + c.error('array ${para} needs to be an int', pos) + } + if expr is ast.IntegerLiteral { + lit := expr as ast.IntegerLiteral + if lit.val.int() < 0 { + c.error('array ${para} can not be negative', lit.pos) + } + } +} + +// When the fixed array has multiple dimensions, it needs to be evaluated recursively. +// `[const]int`, `[const][3]int`, `[3][const]int`, `[const + 1][3][const]int`... +fn (mut c Checker) eval_array_fixed_sizes(mut size_expr ast.Expr, size int, elem_type ast.Type) ast.Type { + elem_sym := c.table.sym(elem_type) + elem_info := elem_sym.info + + new_elem_typ := if elem_sym.kind == .array_fixed { + mut info := elem_info as ast.ArrayFixed + mut elem_size_expr := unsafe { info.size_expr } + c.eval_array_fixed_sizes(mut elem_size_expr, info.size, info.elem_type) + } else { + elem_type + } + + mut fixed_size := i64(size) + if fixed_size <= 0 { + c.expr(mut size_expr) + match mut size_expr { ast.IntegerLiteral { - fixed_size = init_expr.val.int() + fixed_size = size_expr.val.int() } ast.CastExpr { - if !init_expr.typ.is_pure_int() { - c.error('only integer types are allowed', init_expr.pos) + if !size_expr.typ.is_pure_int() { + c.error('only integer types are allowed', size_expr.pos) } - match mut init_expr.expr { + match mut size_expr.expr { ast.IntegerLiteral { - fixed_size = init_expr.expr.val.int() + fixed_size = size_expr.expr.val.int() } ast.EnumVal { - if val := c.table.find_enum_field_val(init_expr.expr.enum_name, - init_expr.expr.val) + if val := c.table.find_enum_field_val(size_expr.expr.enum_name, + size_expr.expr.val) { fixed_size = val } @@ -273,90 +320,64 @@ fn (mut c Checker) array_init(mut node ast.ArrayInit) ast.Type { } } ast.EnumVal { - c.error('${init_expr.enum_name}.${init_expr.val} has to be casted to integer to be used as size', - init_expr.pos) + c.error('${size_expr.enum_name}.${size_expr.val} has to be casted to integer to be used as size', + size_expr.pos) } ast.Ident { - if mut init_expr.obj is ast.ConstField { - if mut init_expr.obj.expr is ast.EnumVal { - c.error('${init_expr.obj.expr.enum_name}.${init_expr.obj.expr.val} has to be casted to integer to be used as size', - init_expr.pos) + if mut size_expr.obj is ast.ConstField { + if mut size_expr.obj.expr is ast.EnumVal { + c.error('${size_expr.obj.expr.enum_name}.${size_expr.obj.expr.val} has to be casted to integer to be used as size', + size_expr.pos) } - if mut init_expr.obj.expr is ast.CastExpr { - if !init_expr.obj.expr.typ.is_pure_int() { - c.error('only integer types are allowed', init_expr.pos) + if mut size_expr.obj.expr is ast.CastExpr { + if !size_expr.obj.expr.typ.is_pure_int() { + c.error('only integer types are allowed', size_expr.pos) } - if init_expr.obj.expr.expr is ast.IntegerLiteral { - if comptime_value := c.eval_comptime_const_expr(init_expr.obj.expr.expr, + if size_expr.obj.expr.expr is ast.IntegerLiteral { + if comptime_value := c.eval_comptime_const_expr(size_expr.obj.expr.expr, 0) { fixed_size = comptime_value.i64() or { fixed_size } } } - if init_expr.obj.expr.expr is ast.InfixExpr { - if comptime_value := c.eval_comptime_const_expr(init_expr.obj.expr.expr, + if size_expr.obj.expr.expr is ast.InfixExpr { + if comptime_value := c.eval_comptime_const_expr(size_expr.obj.expr.expr, 0) { fixed_size = comptime_value.i64() or { fixed_size } } } } - if comptime_value := c.eval_comptime_const_expr(init_expr.obj.expr, + if comptime_value := c.eval_comptime_const_expr(size_expr.obj.expr, 0) { fixed_size = comptime_value.i64() or { fixed_size } } } else { - c.error('non-constant array bound `${init_expr.name}`', init_expr.pos) + c.error('non-constant array bound `${size_expr.name}`', size_expr.pos) } } ast.InfixExpr { - if comptime_value := c.eval_comptime_const_expr(init_expr, 0) { + if comptime_value := c.eval_comptime_const_expr(size_expr, 0) { fixed_size = comptime_value.i64() or { fixed_size } } } else { - c.error('fixed array size cannot use non-constant value', init_expr.pos()) + c.error('fixed array size cannot use non-constant value', size_expr.pos()) } } if fixed_size <= 0 { c.error('fixed size cannot be zero or negative (fixed_size: ${fixed_size})', - init_expr.pos()) - } - idx := c.table.find_or_register_array_fixed(node.elem_type, int(fixed_size), init_expr, - false) - if node.elem_type.has_flag(.generic) { - node.typ = ast.new_type(idx).set_flag(.generic) - } else { - node.typ = ast.new_type(idx) - } - if node.has_init { - c.check_array_init_default_expr(mut node) + size_expr.pos()) } } - return node.typ -} - -fn (mut c Checker) check_array_init_default_expr(mut node ast.ArrayInit) { - mut init_expr := node.init_expr - init_typ := c.check_expr_option_or_result_call(init_expr, c.expr(mut init_expr)) - node.init_type = init_typ - if !node.elem_type.has_flag(.option) && init_typ.has_flag(.option) { - c.error('cannot use unwrapped Option as initializer', init_expr.pos()) - } - c.check_expected(init_typ, node.elem_type) or { c.error(err.msg(), init_expr.pos()) } -} -fn (mut c Checker) check_array_init_para_type(para string, mut expr ast.Expr, pos token.Pos) { - sym := c.table.sym(c.unwrap_generic(c.expr(mut expr))) - if sym.kind !in [.int, .int_literal] { - c.error('array ${para} needs to be an int', pos) - } - if expr is ast.IntegerLiteral { - lit := expr as ast.IntegerLiteral - if lit.val.int() < 0 { - c.error('array ${para} can not be negative', lit.pos) - } + idx := c.table.find_or_register_array_fixed(new_elem_typ, int(fixed_size), size_expr, + false) + return if elem_type.has_flag(.generic) { + ast.new_type(idx).set_flag(.generic) + } else { + ast.new_type(idx) } } diff --git a/vlib/v/gen/c/cgen.v b/vlib/v/gen/c/cgen.v index e6fc4a53a54702..2fb6f8ce3396c4 100644 --- a/vlib/v/gen/c/cgen.v +++ b/vlib/v/gen/c/cgen.v @@ -1455,23 +1455,25 @@ pub fn (mut g Gen) write_typedef_types() { styp := sym.cname // array_fixed_char_300 => char x[300] len := styp.after('_') - mut fixed := g.typ(info.elem_type) - if elem_sym.info is ast.FnType { - pos := g.out.len - // pos2:=g.out_parallel[g.out_idx].len - g.write_fn_ptr_decl(&elem_sym.info, '') - fixed = g.out.cut_to(pos) - // g.out_parallel[g.out_idx].cut_to(pos2) - mut def_str := 'typedef ${fixed};' - def_str = def_str.replace_once('(*)', '(*${styp}[${len}])') - g.type_definitions.writeln(def_str) - } else if !info.is_fn_ret && len.int() > 0 { - g.type_definitions.writeln('typedef ${fixed} ${styp} [${len}];') - base := g.typ(info.elem_type.clear_option_and_result()) - if info.elem_type.has_flag(.option) && base !in g.options_forward { - g.options_forward << base - } else if info.elem_type.has_flag(.result) && base !in g.results_forward { - g.results_forward << base + if len.int() > 0 { + mut fixed := g.typ(info.elem_type) + if elem_sym.info is ast.FnType { + pos := g.out.len + // pos2:=g.out_parallel[g.out_idx].len + g.write_fn_ptr_decl(&elem_sym.info, '') + fixed = g.out.cut_to(pos) + // g.out_parallel[g.out_idx].cut_to(pos2) + mut def_str := 'typedef ${fixed};' + def_str = def_str.replace_once('(*)', '(*${styp}[${len}])') + g.type_definitions.writeln(def_str) + } else if !info.is_fn_ret { + g.type_definitions.writeln('typedef ${fixed} ${styp} [${len}];') + base := g.typ(info.elem_type.clear_option_and_result()) + if info.elem_type.has_flag(.option) && base !in g.options_forward { + g.options_forward << base + } else if info.elem_type.has_flag(.result) && base !in g.results_forward { + g.results_forward << base + } } } } @@ -6181,18 +6183,21 @@ fn (mut g Gen) write_types(symbols []&ast.TypeSymbol) { fixed_elem_name += '*'.repeat(sym.info.elem_type.nr_muls()) } len := sym.info.size - if fixed_elem_name.starts_with('C__') { - fixed_elem_name = fixed_elem_name[3..] - } - if elem_sym.info is ast.FnType { - pos := g.out.len - g.write_fn_ptr_decl(&elem_sym.info, '') - fixed_elem_name = g.out.cut_to(pos) - mut def_str := 'typedef ${fixed_elem_name};' - def_str = def_str.replace_once('(*)', '(*${styp}[${len}])') - g.type_definitions.writeln(def_str) - } else if len > 0 { - g.type_definitions.writeln('typedef ${fixed_elem_name} ${styp} [${len}];') + if len > 0 { + if fixed_elem_name.starts_with('C__') { + fixed_elem_name = fixed_elem_name[3..] + } + if elem_sym.info is ast.FnType { + pos := g.out.len + g.write_fn_ptr_decl(&elem_sym.info, '') + fixed_elem_name = g.out.cut_to(pos) + mut def_str := 'typedef ${fixed_elem_name};' + def_str = def_str.replace_once('(*)', '(*${styp}[${len}])') + g.type_definitions.writeln(def_str) + } else if elem_sym.info !is ast.ArrayFixed + || (elem_sym.info as ast.ArrayFixed).size > 0 { + g.type_definitions.writeln('typedef ${fixed_elem_name} ${styp} [${len}];') + } } } } diff --git a/vlib/v/parser/parse_type.v b/vlib/v/parser/parse_type.v index a748abb1985c27..4d094b3c43856c 100644 --- a/vlib/v/parser/parse_type.v +++ b/vlib/v/parser/parse_type.v @@ -31,7 +31,6 @@ fn (mut p Parser) parse_array_type(expecting token.Kind, is_option bool) ast.Typ fixed_size = const_field.expr.val.int() size_unresolved = false } else if mut const_field.expr is ast.InfixExpr { - // QUESTION: this should most likely no be done in the parser, right? mut t := transformer.new_transformer_with_table(p.table, p.pref) folded_expr := t.infix_expr(mut const_field.expr) @@ -74,10 +73,8 @@ fn (mut p Parser) parse_array_type(expecting token.Kind, is_option bool) ast.Typ // error is handled by parse_type return 0 } - // TODO: - // For now, when a const variable or expression is temporarily unavailable to evaluate, - // only pending struct fields are deferred. - if fixed_size <= 0 && (!p.inside_struct_field_decl || !size_unresolved) { + // has been explicitly resolved, but size is 0 + if fixed_size <= 0 && !size_unresolved { p.error_with_pos('fixed size cannot be zero or negative', size_expr.pos()) } idx := p.table.find_or_register_array_fixed(elem_type, fixed_size, size_expr, diff --git a/vlib/v/tests/constant_array_size_test.v b/vlib/v/tests/constant_array_size_test.v index 47e56cd296a459..c06648676d3156 100644 --- a/vlib/v/tests/constant_array_size_test.v +++ b/vlib/v/tests/constant_array_size_test.v @@ -20,3 +20,18 @@ fn test_const_below_at_struct_fixed_array_fields() { foo := Foo{} assert foo.posts == [0, 0, 0, 0, 0]! } + +// for issue 20311 +// When using a static variable to define a fixed array size, +// if the static variable is defined below or in another module, the size value will not be calculated correctly. +fn test_const_below_at_fixed_array() { + arr := [width][2][width + 1]f64{} + assert arr.len == 2 + assert arr[0].len == 2 + assert arr[0][0].len == 3 + assert arr == [[[0.0, 0.0, 0.0]!, [0.0, 0.0, 0.0]!]!, [[0.0, 0.0, 0.0]!, + [0.0, 0.0, 0.0]!]!]! +} + +// Do not move this definition; it must be below `fn test_const_below_at_fixed_array()`. +const width = 2