Skip to content

Commit

Permalink
parser, checker, cgen: fix multidimensional fixed array size expressi…
Browse files Browse the repository at this point in the history 
…on evaluation (fix #20311) (#20458)
  • Loading branch information
@shove70
shove70 committed Jan 10, 2024
1 parent c1904ec commit b77e391
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 94 deletions.
141 changes: 81 additions & 60 deletions vlib/v/checker/containers.v
View file
Expand Up @@ -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
}
Expand All @@ -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)
}
}

Expand Down
63 changes: 34 additions & 29 deletions vlib/v/gen/c/cgen.v
View file
Expand Up @@ -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
}
}
}
}
Expand Down Expand Up @@ -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}];')
}
}
}
}
Expand Down
7 changes: 2 additions & 5 deletions vlib/v/parser/parse_type.v
View file
Expand Up @@ -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)

Expand Down Expand Up @@ -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,
Expand Down
15 changes: 15 additions & 0 deletions vlib/v/tests/constant_array_size_test.v
View file
Expand Up @@ -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

0 comments on commit b77e391

Please sign in to comment.