From f3fb69935def86bb548e36db5e7168e47a800cfd Mon Sep 17 00:00:00 2001 From: Felipe Pena Date: Sat, 17 Aug 2024 09:03:28 -0300 Subject: [PATCH 1/3] fix --- vlib/v/checker/struct.v | 257 +++++++++--------- .../sumtype_init_with_ref_fields_err.out | 21 ++ .../tests/sumtype_init_with_ref_fields_err.vv | 11 + 3 files changed, 168 insertions(+), 121 deletions(-) create mode 100644 vlib/v/checker/tests/sumtype_init_with_ref_fields_err.out create mode 100644 vlib/v/checker/tests/sumtype_init_with_ref_fields_err.vv diff --git a/vlib/v/checker/struct.v b/vlib/v/checker/struct.v index 6c246b28da6c94..4d4e5d3903d924 100644 --- a/vlib/v/checker/struct.v +++ b/vlib/v/checker/struct.v @@ -777,128 +777,8 @@ or use an explicit `unsafe{ a[..] }`, if you do not want a copy of the slice.', // and the second part is all fields embedded in the structure // If the return value data composition form in `c.table.struct_fields()` is modified, // need to modify here accordingly. - mut fields := c.table.struct_fields(type_sym) - mut checked_types := []ast.Type{} + c.check_uninitialized_struct_fields(node, type_sym, mut inited_fields) - for i, mut field in fields { - if field.name in inited_fields { - if c.mod != type_sym.mod { - if !field.is_pub { - parts := type_sym.name.split('.') - for init_field in node.init_fields { - if field.name == init_field.name { - mod_type := if parts.len > 1 { - parts#[-2..].join('.') - } else { - parts.last() - } - c.error('cannot access private field `${field.name}` on `${mod_type}`', - init_field.pos) - break - } - } - } - if field.is_deprecated { - for init_field in node.init_fields { - if field.name == init_field.name { - c.deprecate('field', field.name, field.attrs, init_field.pos) - break - } - } - } - } - continue - } - sym := c.table.sym(field.typ) - if field.name != '' && field.name[0].is_capital() && sym.info is ast.Struct { - // struct embeds - continue - } - if field.has_default_expr { - if i < info.fields.len && field.default_expr_typ == 0 { - if mut field.default_expr is ast.StructInit { - idx := c.table.find_type_idx(field.default_expr.typ_str) - if idx != 0 { - info.fields[i].default_expr_typ = ast.new_type(idx) - } - } else if field.default_expr.is_nil() { - if field.typ.is_any_kind_of_pointer() { - info.fields[i].default_expr_typ = field.typ - } - } else if field.default_expr is ast.Ident - && field.default_expr.info is ast.IdentFn { - c.expr(mut field.default_expr) - } else { - if const_field := c.table.global_scope.find_const('${field.default_expr}') { - info.fields[i].default_expr_typ = const_field.typ - } else if type_sym.info is ast.Struct && type_sym.info.is_anon { - c.expected_type = field.typ - field.default_expr_typ = c.expr(mut field.default_expr) - info.fields[i].default_expr_typ = field.default_expr_typ - } - } - } - continue - } - if field.typ.is_ptr() && !field.typ.has_flag(.shared_f) - && !field.typ.has_flag(.option) && !node.has_update_expr && !c.pref.translated - && !c.file.is_translated { - c.error('reference field `${type_sym.name}.${field.name}` must be initialized', - node.pos) - continue - } - if !field.typ.has_flag(.option) { - if sym.kind == .struct_ { - c.check_ref_fields_initialized(sym, mut checked_types, '${type_sym.name}.${field.name}', - node.pos) - } else if sym.kind == .alias { - parent_sym := c.table.sym((sym.info as ast.Alias).parent_type) - if parent_sym.kind == .struct_ { - c.check_ref_fields_initialized(parent_sym, mut checked_types, - '${type_sym.name}.${field.name}', node.pos) - } - } - } - // Do not allow empty uninitialized interfaces - if sym.kind == .interface_ && !node.has_update_expr && !field.typ.has_flag(.option) - && sym.language != .js && !field.attrs.contains('noinit') { - // TODO: should be an error instead, but first `ui` needs updating. - c.note('interface field `${type_sym.name}.${field.name}` must be initialized', - node.pos) - } - // Do not allow empty uninitialized sum types - /* - sym := c.table.sym(field.typ) - if sym.kind == .sum_type { - c.warn('sum type field `${type_sym.name}.$field.name` must be initialized', - node.pos) - } - */ - // Check for `@[required]` struct attr - if !node.no_keys && !node.has_update_expr && field.attrs.contains('required') - && node.init_fields.all(it.name != field.name) { - c.error('field `${type_sym.name}.${field.name}` must be initialized', - node.pos) - } - if !node.has_update_expr && !field.has_default_expr && !field.typ.is_ptr() - && !field.typ.has_flag(.option) { - field_final_sym := c.table.final_sym(field.typ) - if field_final_sym.kind == .struct_ { - mut zero_struct_init := ast.StructInit{ - pos: node.pos - typ: field.typ - } - if field.is_part_of_union { - if field.name in inited_fields { - // fields that are part of an union, should only be checked, when they are explicitly initialised - c.struct_init(mut zero_struct_init, true, mut inited_fields) - } - } else { - c.struct_init(mut zero_struct_init, true, mut inited_fields) - } - } - } - } for embed in info.embeds { mut zero_struct_init := ast.StructInit{ pos: node.pos @@ -908,6 +788,18 @@ or use an explicit `unsafe{ a[..] }`, if you do not want a copy of the slice.', } // println('>> checked_types.len: $checked_types.len | checked_types: $checked_types | type_sym: $type_sym.name ') } + .sum_type { + first_typ := (type_sym.info as ast.SumType).variants[0] + first_sym := c.table.final_sym(first_typ) + if first_sym.kind == .struct_ { + // Check uninitialized refs/sum types + // The variable `fields` contains two parts, the first part is the same as info.fields, + // and the second part is all fields embedded in the structure + // If the return value data composition form in `c.table.struct_fields()` is modified, + // need to modify here accordingly. + c.check_uninitialized_struct_fields(node, first_sym, mut inited_fields) + } + } else {} } if node.has_update_expr { @@ -960,6 +852,129 @@ or use an explicit `unsafe{ a[..] }`, if you do not want a copy of the slice.', return node.typ } +fn (mut c Checker) check_uninitialized_struct_fields(node ast.StructInit, type_sym ast.TypeSymbol, mut inited_fields []string) { + mut fields := c.table.struct_fields(type_sym) + mut checked_types := []ast.Type{} + mut info := type_sym.info as ast.Struct + + for i, mut field in fields { + if field.name in inited_fields { + if c.mod != type_sym.mod { + if !field.is_pub { + parts := type_sym.name.split('.') + for init_field in node.init_fields { + if field.name == init_field.name { + mod_type := if parts.len > 1 { + parts#[-2..].join('.') + } else { + parts.last() + } + c.error('cannot access private field `${field.name}` on `${mod_type}`', + init_field.pos) + break + } + } + } + if field.is_deprecated { + for init_field in node.init_fields { + if field.name == init_field.name { + c.deprecate('field', field.name, field.attrs, init_field.pos) + break + } + } + } + } + continue + } + sym := c.table.sym(field.typ) + if field.name != '' && field.name[0].is_capital() && sym.info is ast.Struct { + // struct embeds + continue + } + if field.has_default_expr { + if i < info.fields.len && field.default_expr_typ == 0 { + if mut field.default_expr is ast.StructInit { + idx := c.table.find_type_idx(field.default_expr.typ_str) + if idx != 0 { + info.fields[i].default_expr_typ = ast.new_type(idx) + } + } else if field.default_expr.is_nil() { + if field.typ.is_any_kind_of_pointer() { + info.fields[i].default_expr_typ = field.typ + } + } else if field.default_expr is ast.Ident && field.default_expr.info is ast.IdentFn { + c.expr(mut field.default_expr) + } else { + if const_field := c.table.global_scope.find_const('${field.default_expr}') { + info.fields[i].default_expr_typ = const_field.typ + } else if type_sym.info is ast.Struct && type_sym.info.is_anon { + c.expected_type = field.typ + field.default_expr_typ = c.expr(mut field.default_expr) + info.fields[i].default_expr_typ = field.default_expr_typ + } + } + } + continue + } + if field.typ.is_ptr() && !field.typ.has_flag(.shared_f) && !field.typ.has_flag(.option) + && !node.has_update_expr && !c.pref.translated && !c.file.is_translated { + c.error('reference field `${type_sym.name}.${field.name}` must be initialized', + node.pos) + continue + } + if !field.typ.has_flag(.option) { + if sym.kind == .struct_ { + c.check_ref_fields_initialized(sym, mut checked_types, '${type_sym.name}.${field.name}', + node.pos) + } else if sym.kind == .alias { + parent_sym := c.table.sym((sym.info as ast.Alias).parent_type) + if parent_sym.kind == .struct_ { + c.check_ref_fields_initialized(parent_sym, mut checked_types, '${type_sym.name}.${field.name}', + node.pos) + } + } + } + // Do not allow empty uninitialized interfaces + if sym.kind == .interface_ && !node.has_update_expr && !field.typ.has_flag(.option) + && sym.language != .js && !field.attrs.contains('noinit') { + // TODO: should be an error instead, but first `ui` needs updating. + c.note('interface field `${type_sym.name}.${field.name}` must be initialized', + node.pos) + } + // Do not allow empty uninitialized sum types + /* + sym := c.table.sym(field.typ) + if sym.kind == .sum_type { + c.warn('sum type field `${type_sym.name}.$field.name` must be initialized', + node.pos) + } + */ + // Check for `@[required]` struct attr + if !node.no_keys && !node.has_update_expr && field.attrs.contains('required') + && node.init_fields.all(it.name != field.name) { + c.error('field `${type_sym.name}.${field.name}` must be initialized', node.pos) + } + if !node.has_update_expr && !field.has_default_expr && !field.typ.is_ptr() + && !field.typ.has_flag(.option) { + field_final_sym := c.table.final_sym(field.typ) + if field_final_sym.kind == .struct_ { + mut zero_struct_init := ast.StructInit{ + pos: node.pos + typ: field.typ + } + if field.is_part_of_union { + if field.name in inited_fields { + // fields that are part of an union, should only be checked, when they are explicitly initialised + c.struct_init(mut zero_struct_init, true, mut inited_fields) + } + } else { + c.struct_init(mut zero_struct_init, true, mut inited_fields) + } + } + } + } +} + // Recursively check whether the struct type field is initialized fn (mut c Checker) check_ref_fields_initialized(struct_sym &ast.TypeSymbol, mut checked_types []ast.Type, linked_name string, pos &token.Pos) { diff --git a/vlib/v/checker/tests/sumtype_init_with_ref_fields_err.out b/vlib/v/checker/tests/sumtype_init_with_ref_fields_err.out new file mode 100644 index 00000000000000..70da8509eab584 --- /dev/null +++ b/vlib/v/checker/tests/sumtype_init_with_ref_fields_err.out @@ -0,0 +1,21 @@ +vlib/v/checker/tests/sumtype_init_with_ref_fields_err.vv:9:10: error: reference field `Node.parent` must be initialized + 7 | + 8 | fn main() { + 9 | s := Sumtype{} + | ~~~~~~~~~ + 10 | dump(s) + 11 | } +vlib/v/checker/tests/sumtype_init_with_ref_fields_err.vv:9:10: error: reference field `Node.left` must be initialized + 7 | + 8 | fn main() { + 9 | s := Sumtype{} + | ~~~~~~~~~ + 10 | dump(s) + 11 | } +vlib/v/checker/tests/sumtype_init_with_ref_fields_err.vv:9:10: error: reference field `Node.right` must be initialized + 7 | + 8 | fn main() { + 9 | s := Sumtype{} + | ~~~~~~~~~ + 10 | dump(s) + 11 | } diff --git a/vlib/v/checker/tests/sumtype_init_with_ref_fields_err.vv b/vlib/v/checker/tests/sumtype_init_with_ref_fields_err.vv new file mode 100644 index 00000000000000..b7eaf5e5a74de7 --- /dev/null +++ b/vlib/v/checker/tests/sumtype_init_with_ref_fields_err.vv @@ -0,0 +1,11 @@ +struct Node { + parent &Node + left &Node + right &Node +} +type Sumtype = Node | int + +fn main() { + s := Sumtype{} + dump(s) +} \ No newline at end of file From 674b7b407e2fccacca94099a95b02fc948e085a2 Mon Sep 17 00:00:00 2001 From: Felipe Pena Date: Sat, 17 Aug 2024 09:25:49 -0300 Subject: [PATCH 2/3] fix --- vlib/v/checker/struct.v | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/vlib/v/checker/struct.v b/vlib/v/checker/struct.v index 4d4e5d3903d924..15cfa0c76f212e 100644 --- a/vlib/v/checker/struct.v +++ b/vlib/v/checker/struct.v @@ -777,15 +777,8 @@ or use an explicit `unsafe{ a[..] }`, if you do not want a copy of the slice.', // and the second part is all fields embedded in the structure // If the return value data composition form in `c.table.struct_fields()` is modified, // need to modify here accordingly. - c.check_uninitialized_struct_fields(node, type_sym, mut inited_fields) - - for embed in info.embeds { - mut zero_struct_init := ast.StructInit{ - pos: node.pos - typ: embed - } - c.struct_init(mut zero_struct_init, true, mut inited_fields) - } + c.check_uninitialized_struct_fields_and_embeds(node, type_sym, mut info, mut + inited_fields) // println('>> checked_types.len: $checked_types.len | checked_types: $checked_types | type_sym: $type_sym.name ') } .sum_type { @@ -797,7 +790,9 @@ or use an explicit `unsafe{ a[..] }`, if you do not want a copy of the slice.', // and the second part is all fields embedded in the structure // If the return value data composition form in `c.table.struct_fields()` is modified, // need to modify here accordingly. - c.check_uninitialized_struct_fields(node, first_sym, mut inited_fields) + mut info := first_sym.info as ast.Struct + c.check_uninitialized_struct_fields_and_embeds(node, first_sym, mut info, mut + inited_fields) } } else {} @@ -852,10 +847,9 @@ or use an explicit `unsafe{ a[..] }`, if you do not want a copy of the slice.', return node.typ } -fn (mut c Checker) check_uninitialized_struct_fields(node ast.StructInit, type_sym ast.TypeSymbol, mut inited_fields []string) { +fn (mut c Checker) check_uninitialized_struct_fields_and_embeds(node ast.StructInit, type_sym ast.TypeSymbol, mut info ast.Struct, mut inited_fields []string) { mut fields := c.table.struct_fields(type_sym) mut checked_types := []ast.Type{} - mut info := type_sym.info as ast.Struct for i, mut field in fields { if field.name in inited_fields { @@ -973,6 +967,14 @@ fn (mut c Checker) check_uninitialized_struct_fields(node ast.StructInit, type_s } } } + + for embed in info.embeds { + mut zero_struct_init := ast.StructInit{ + pos: node.pos + typ: embed + } + c.struct_init(mut zero_struct_init, true, mut inited_fields) + } } // Recursively check whether the struct type field is initialized From 336f621bf37557601e54099dd1873b1eb4b3e5b8 Mon Sep 17 00:00:00 2001 From: Felipe Pena Date: Sat, 17 Aug 2024 11:36:17 -0300 Subject: [PATCH 3/3] fix --- vlib/v/checker/struct.v | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/vlib/v/checker/struct.v b/vlib/v/checker/struct.v index 15cfa0c76f212e..20a05463249ffb 100644 --- a/vlib/v/checker/struct.v +++ b/vlib/v/checker/struct.v @@ -772,11 +772,6 @@ or use an explicit `unsafe{ a[..] }`, if you do not want a copy of the slice.', init_field.expr.pos.extend(init_field.expr.expr.pos())) } } - // Check uninitialized refs/sum types - // The variable `fields` contains two parts, the first part is the same as info.fields, - // and the second part is all fields embedded in the structure - // If the return value data composition form in `c.table.struct_fields()` is modified, - // need to modify here accordingly. c.check_uninitialized_struct_fields_and_embeds(node, type_sym, mut info, mut inited_fields) // println('>> checked_types.len: $checked_types.len | checked_types: $checked_types | type_sym: $type_sym.name ') @@ -785,11 +780,6 @@ or use an explicit `unsafe{ a[..] }`, if you do not want a copy of the slice.', first_typ := (type_sym.info as ast.SumType).variants[0] first_sym := c.table.final_sym(first_typ) if first_sym.kind == .struct_ { - // Check uninitialized refs/sum types - // The variable `fields` contains two parts, the first part is the same as info.fields, - // and the second part is all fields embedded in the structure - // If the return value data composition form in `c.table.struct_fields()` is modified, - // need to modify here accordingly. mut info := first_sym.info as ast.Struct c.check_uninitialized_struct_fields_and_embeds(node, first_sym, mut info, mut inited_fields) @@ -847,6 +837,11 @@ or use an explicit `unsafe{ a[..] }`, if you do not want a copy of the slice.', return node.typ } +// Check uninitialized refs/sum types +// The variable `fields` contains two parts, the first part is the same as info.fields, +// and the second part is all fields embedded in the structure +// If the return value data composition form in `c.table.struct_fields()` is modified, +// need to modify here accordingly. fn (mut c Checker) check_uninitialized_struct_fields_and_embeds(node ast.StructInit, type_sym ast.TypeSymbol, mut info ast.Struct, mut inited_fields []string) { mut fields := c.table.struct_fields(type_sym) mut checked_types := []ast.Type{}