Skip to content

Commit

Permalink
checker: check element and its children for ref uninitialized fields (f…
Browse files Browse the repository at this point in the history 
…ix #19864) (#19944)
  • Loading branch information
@shove70
shove70 committed Nov 22, 2023
1 parent 7519f91 commit 25e9016
Show file tree
Hide file tree
Showing 6 changed files with 176 additions and 8 deletions.
72 changes: 72 additions & 0 deletions vlib/v/checker/containers.v
View file
Expand Up @@ -102,6 +102,7 @@ fn (mut c Checker) array_init(mut node ast.ArrayInit) ast.Type {
c.warn('arrays of references need to be initialized right away, therefore `len:` cannot be used (unless inside `unsafe`)',
node.pos)
}
c.check_elements_ref_fields_initialized(node.elem_type, node.pos)
return node.typ
}

Expand All @@ -112,6 +113,7 @@ fn (mut c Checker) array_init(mut node ast.ArrayInit) ast.Type {
c.warn('fixed arrays of references need to be initialized right away (unless inside `unsafe`)',
node.pos)
}
c.check_elements_ref_fields_initialized(node.elem_type, node.pos)
}
// `a = []`
if node.exprs.len == 0 {
Expand Down Expand Up @@ -400,6 +402,7 @@ fn (mut c Checker) map_init(mut node ast.MapInit) ast.Type {
c.ensure_type_exists(info.value_type, node.pos)
node.key_type = info.key_type
node.value_type = info.value_type
c.check_elements_ref_fields_initialized(node.typ, node.pos)
return node.typ
}

Expand Down Expand Up @@ -499,3 +502,72 @@ fn (mut c Checker) map_init(mut node ast.MapInit) ast.Type {
}
return node.typ
}

// check the element, and its children for ref uninitialized fields
fn (mut c Checker) check_elements_ref_fields_initialized(typ ast.Type, pos &token.Pos) {
if typ == 0 || c.inside_const {
return
}
sym := c.table.sym(typ)
mut checked_types := []ast.Type{}
c.do_check_elements_ref_fields_initialized(sym, mut checked_types, pos)
}

// Recursively check the element, and its children for ref uninitialized fields
fn (mut c Checker) do_check_elements_ref_fields_initialized(sym &ast.TypeSymbol, mut checked_types []ast.Type, pos &token.Pos) {
if sym.info is ast.Struct {
linked_name := sym.name
// For now, let's call this method and give a notice instead of an error.
// After some time, we remove the check_ref_fields_initialized_note() method and
// simply call check_ref_fields_initialized()
c.check_ref_fields_initialized_note(sym, mut checked_types, linked_name, pos)
return
}
match sym.info {
ast.Array {
elem_type := sym.info.elem_type
if elem_type in checked_types {
return
}
checked_types << elem_type
elem_sym := c.table.sym(elem_type)
c.do_check_elements_ref_fields_initialized(elem_sym, mut checked_types, pos)
}
ast.ArrayFixed {
elem_type := sym.info.elem_type
if elem_type in checked_types {
return
}
checked_types << elem_type
elem_sym := c.table.sym(elem_type)
c.do_check_elements_ref_fields_initialized(elem_sym, mut checked_types, pos)
}
ast.Map {
key_type := sym.info.key_type
if key_type in checked_types {
return
}
checked_types << key_type
key_sym := c.table.sym(key_type)
c.do_check_elements_ref_fields_initialized(key_sym, mut checked_types, pos)
value_type := sym.info.value_type
if value_type in checked_types {
return
}
checked_types << value_type
value_sym := c.table.sym(value_type)
c.do_check_elements_ref_fields_initialized(value_sym, mut checked_types, pos)
}
ast.Alias {
parent_type := sym.info.parent_type
if parent_type in checked_types {
return
}
checked_types << parent_type
parent_sym := c.table.sym(parent_type)
c.do_check_elements_ref_fields_initialized(parent_sym, mut checked_types,
pos)
}
else {}
}
}
61 changes: 55 additions & 6 deletions vlib/v/checker/struct.v
View file
Expand Up @@ -4,6 +4,7 @@ module checker

import v.ast
import v.util
import v.token
import v.transformer

fn (mut c Checker) struct_decl(mut node ast.StructDecl) {
Expand Down Expand Up @@ -795,12 +796,12 @@ or use an explicit `unsafe{ a[..] }`, if you do not want a copy of the slice.',
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)
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)
'${type_sym.name}.${field.name}', node.pos)
}
}
}
Expand Down Expand Up @@ -896,7 +897,7 @@ or use an explicit `unsafe{ a[..] }`, if you do not want a copy of the slice.',
}

// 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, node &ast.StructInit) {
fn (mut c Checker) check_ref_fields_initialized(struct_sym &ast.TypeSymbol, mut checked_types []ast.Type, linked_name string, pos &token.Pos) {
if c.pref.translated || c.file.is_translated {
return
}
Expand All @@ -915,7 +916,55 @@ fn (mut c Checker) check_ref_fields_initialized(struct_sym &ast.TypeSymbol, mut
if field.typ.is_ptr() && !field.typ.has_flag(.shared_f) && !field.typ.has_flag(.option)
&& !field.has_default_expr {
c.error('reference field `${linked_name}.${field.name}` must be initialized (part of struct `${struct_sym.name}`)',
node.pos)
pos)
continue
}
if sym.kind == .struct_ {
if sym.language == .c && (sym.info as ast.Struct).is_typedef {
continue
}
if field.typ in checked_types {
continue
}
checked_types << field.typ
c.check_ref_fields_initialized(sym, mut checked_types, '${linked_name}.${field.name}',
pos)
} else if sym.kind == .alias {
psym := c.table.sym((sym.info as ast.Alias).parent_type)
if psym.kind == .struct_ {
checked_types << field.typ
c.check_ref_fields_initialized(psym, mut checked_types, '${linked_name}.${field.name}',
pos)
}
}
}
}

// Recursively check whether the struct type field is initialized
// NOTE:
// This method is temporary and will only be called by the do_check_elements_ref_fields_initialized() method.
// The goal is to give only a notice, not an error, for now. After a while,
// when we change the notice to error, we can remove this temporary method.
fn (mut c Checker) check_ref_fields_initialized_note(struct_sym &ast.TypeSymbol, mut checked_types []ast.Type, linked_name string, pos &token.Pos) {
if c.pref.translated || c.file.is_translated {
return
}
if struct_sym.kind == .struct_ && struct_sym.language == .c
&& (struct_sym.info as ast.Struct).is_typedef {
return
}
fields := c.table.struct_fields(struct_sym)
for field in fields {
sym := c.table.sym(field.typ)
if field.name.len > 0 && field.name[0].is_capital() && sym.info is ast.Struct
&& sym.language == .v {
// an embedded struct field
continue
}
if field.typ.is_ptr() && !field.typ.has_flag(.shared_f) && !field.typ.has_flag(.option)
&& !field.has_default_expr {
c.note('reference field `${linked_name}.${field.name}` must be initialized (part of struct `${struct_sym.name}`)',
pos)
continue
}
if sym.kind == .struct_ {
Expand All @@ -927,13 +976,13 @@ fn (mut c Checker) check_ref_fields_initialized(struct_sym &ast.TypeSymbol, mut
}
checked_types << field.typ
c.check_ref_fields_initialized(sym, mut checked_types, '${linked_name}.${field.name}',
node)
pos)
} else if sym.kind == .alias {
psym := c.table.sym((sym.info as ast.Alias).parent_type)
if psym.kind == .struct_ {
checked_types << field.typ
c.check_ref_fields_initialized(psym, mut checked_types, '${linked_name}.${field.name}',
node)
pos)
}
}
}
Expand Down
34 changes: 34 additions & 0 deletions vlib/v/checker/tests/array_map_elements_ref_fields_uninitialized_err.out
View file
@@ -0,0 +1,34 @@
vlib/v/checker/tests/array_map_elements_ref_fields_uninitialized_err.vv:8:6: notice: reference field `Foo.n` must be initialized (part of struct `Foo`)
6 |
7 | fn main() {
8 | _ = []Foo{}
| ~~~~~~
9 | _ = [1]Foo{}
10 | _ = map[string]Foo{}
vlib/v/checker/tests/array_map_elements_ref_fields_uninitialized_err.vv:9:6: notice: reference field `Foo.n` must be initialized (part of struct `Foo`)
7 | fn main() {
8 | _ = []Foo{}
9 | _ = [1]Foo{}
| ~~~~~~~~
10 | _ = map[string]Foo{}
11 | _ = map[string][]Foo{}
vlib/v/checker/tests/array_map_elements_ref_fields_uninitialized_err.vv:10:6: notice: reference field `Foo.n` must be initialized (part of struct `Foo`)
8 | _ = []Foo{}
9 | _ = [1]Foo{}
10 | _ = map[string]Foo{}
| ~~~~~~~~~~~~~~~~
11 | _ = map[string][]Foo{}
12 | _ = []AliasFoo{}
vlib/v/checker/tests/array_map_elements_ref_fields_uninitialized_err.vv:11:6: notice: reference field `Foo.n` must be initialized (part of struct `Foo`)
9 | _ = [1]Foo{}
10 | _ = map[string]Foo{}
11 | _ = map[string][]Foo{}
| ~~~~~~~~~~~~~~~~~~
12 | _ = []AliasFoo{}
13 | }
vlib/v/checker/tests/array_map_elements_ref_fields_uninitialized_err.vv:12:6: notice: reference field `Foo.n` must be initialized (part of struct `Foo`)
10 | _ = map[string]Foo{}
11 | _ = map[string][]Foo{}
12 | _ = []AliasFoo{}
| ~~~~~~~~~~~
13 | }
13 changes: 13 additions & 0 deletions vlib/v/checker/tests/array_map_elements_ref_fields_uninitialized_err.vv
View file
@@ -0,0 +1,13 @@
struct Foo {
n &int
}

type AliasFoo = Foo

fn main() {
_ = []Foo{}
_ = [1]Foo{}
_ = map[string]Foo{}
_ = map[string][]Foo{}
_ = []AliasFoo{}
}
2 changes: 1 addition & 1 deletion vlib/v/tests/const_fixed_array_containing_references_to_itself_test.v
View file
@@ -1,5 +1,5 @@
struct Abc {
prev &Abc
prev &Abc = unsafe { nil }
}

const a = [Abc{unsafe { nil }}, Abc{unsafe { &a[0] }}, Abc{unsafe { &a[1] }}]!
Expand Down
2 changes: 1 addition & 1 deletion vlib/v/tests/str_circular_test.v
View file
Expand Up @@ -6,7 +6,7 @@ mut:

struct Bb {
mut:
a &Aa
a &Aa = unsafe { nil }
}

fn test_circular() {
Expand Down

0 comments on commit 25e9016

Please sign in to comment.