Skip to content

Commit

Permalink
ast, checker: add a notice, for accessing by key, map values, that co…
Browse files Browse the repository at this point in the history
…ntain pointers (to use unsafe or an `or {}` block) (#20266)
  • Loading branch information
shove70 committed Dec 25, 2023
1 parent 8e47c21 commit 053a7a9
Show file tree
Hide file tree
Showing 10 changed files with 110 additions and 30 deletions.
2 changes: 1 addition & 1 deletion vlib/v/ast/table.v
Expand Up @@ -1489,7 +1489,7 @@ pub fn (t Table) does_type_implement_interface(typ Type, inter_typ Type) bool {
return false
}
if mut inter_sym.info is Interface {
attrs := t.interfaces[inter_typ].attrs
attrs := unsafe { t.interfaces[inter_typ].attrs }
for attr in attrs {
if attr.name == 'single_impl' {
return false
Expand Down
37 changes: 37 additions & 0 deletions vlib/v/checker/check_types.v
Expand Up @@ -1080,3 +1080,40 @@ fn (mut c Checker) infer_fn_generic_types(func ast.Fn, mut node ast.CallExpr) {
c.need_recheck_generic_fns = true
}
}

// is_contains_any_kind_of_pointer check that the type and submember types(arrays, fixed arrays, maps, struct fields, and so on)
// contain pointer types.
fn (mut c Checker) is_contains_any_kind_of_pointer(typ ast.Type, mut checked_types []ast.Type) bool {
if typ.is_any_kind_of_pointer() {
return true
}
if typ in checked_types {
return false
}
checked_types << typ
sym := c.table.sym(typ)
match sym.info {
ast.Array, ast.ArrayFixed {
return c.is_contains_any_kind_of_pointer(sym.info.elem_type, mut checked_types)
}
ast.Map {
return c.is_contains_any_kind_of_pointer(sym.info.value_type, mut checked_types)
}
ast.Alias {
return c.is_contains_any_kind_of_pointer(sym.info.parent_type, mut checked_types)
}
ast.Struct {
if sym.kind == .struct_ && sym.language == .v {
fields := c.table.struct_fields(sym)
for field in fields {
ret := c.is_contains_any_kind_of_pointer(field.typ, mut checked_types)
if ret {
return true
}
}
}
}
else {}
}
return false
}
13 changes: 11 additions & 2 deletions vlib/v/checker/checker.v
Expand Up @@ -708,8 +708,12 @@ fn (mut c Checker) expand_iface_embeds(idecl &ast.InterfaceDecl, level int, ifac
mut list := iface_decl.embeds.clone()
if !iface_decl.are_embeds_expanded {
list = c.expand_iface_embeds(idecl, level + 1, iface_decl.embeds)
c.table.interfaces[ie.typ].embeds = list
c.table.interfaces[ie.typ].are_embeds_expanded = true
unsafe {
c.table.interfaces[ie.typ].embeds = list
}
unsafe {
c.table.interfaces[ie.typ].are_embeds_expanded = true
}
}
for partial in list {
res[partial.typ] = partial
Expand Down Expand Up @@ -4356,6 +4360,11 @@ fn (mut c Checker) index_expr(mut node ast.IndexExpr) ast.Type {
c.note('accessing a pointer map value requires an `or {}` block outside `unsafe`',
node.pos)
}
mut checked_types := []ast.Type{}
if c.is_contains_any_kind_of_pointer(elem_type, mut checked_types) {
c.note('accessing map value that contain pointers requires an `or {}` block outside `unsafe`',
node.pos)
}
}

if (typ.is_ptr() && !typ.has_flag(.shared_f) && (!node.left.is_auto_deref_var()
Expand Down
24 changes: 14 additions & 10 deletions vlib/v/checker/fn.v
Expand Up @@ -426,10 +426,14 @@ fn (mut c Checker) fn_decl(mut node ast.FnDecl) {
}
}
if dep_names.len > 0 {
c.table.fns[node.name].dep_names = dep_names
unsafe {
c.table.fns[node.name].dep_names = dep_names
}
}
}
c.table.fns[node.name].source_fn = voidptr(node)
unsafe {
c.table.fns[node.name].source_fn = voidptr(node)
}
}

// vweb checks
Expand Down Expand Up @@ -810,7 +814,7 @@ fn (mut c Checker) fn_call(mut node ast.CallExpr, mut continue_check &bool) ast.
node.name = name_prefixed
found = true
func = f
c.table.fns[name_prefixed].usages++
unsafe { c.table.fns[name_prefixed].usages++ }
}
}
if !found && node.left is ast.IndexExpr {
Expand Down Expand Up @@ -868,7 +872,7 @@ fn (mut c Checker) fn_call(mut node ast.CallExpr, mut continue_check &bool) ast.
if f := c.table.find_fn(fn_name) {
found = true
func = f
c.table.fns[fn_name].usages++
unsafe { c.table.fns[fn_name].usages++ }
}
}
// already imported symbol (static Foo.new() in another module)
Expand All @@ -881,7 +885,7 @@ fn (mut c Checker) fn_call(mut node ast.CallExpr, mut continue_check &bool) ast.
found = true
func = f
node.name = qualified_name
c.table.fns[qualified_name].usages++
unsafe { c.table.fns[qualified_name].usages++ }
break
}
}
Expand Down Expand Up @@ -938,9 +942,9 @@ fn (mut c Checker) fn_call(mut node ast.CallExpr, mut continue_check &bool) ast.
mut is_native_builtin := false
if !found && c.pref.backend == .native {
if fn_name in ast.native_builtins {
c.table.fns[fn_name].usages++
unsafe { c.table.fns[fn_name].usages++ }
found = true
func = c.table.fns[fn_name]
func = unsafe { c.table.fns[fn_name] }
is_native_builtin = true
}
}
Expand All @@ -958,7 +962,7 @@ fn (mut c Checker) fn_call(mut node ast.CallExpr, mut continue_check &bool) ast.
node.name = os_name
found = true
func = f
c.table.fns[os_name].usages++
unsafe { c.table.fns[os_name].usages++ }
}
}
if is_native_builtin {
Expand Down Expand Up @@ -1040,8 +1044,8 @@ fn (mut c Checker) fn_call(mut node ast.CallExpr, mut continue_check &bool) ast.
if sym.info is ast.FnType {
// at this point, the const metadata should be already known,
// and we are sure that it is just a function
c.table.fns[qualified_const_name].usages++
c.table.fns[func.name].usages++
unsafe { c.table.fns[qualified_const_name].usages++ }
unsafe { c.table.fns[func.name].usages++ }
found = true
func = sym.info.func
node.is_fn_a_const = true
Expand Down
44 changes: 32 additions & 12 deletions vlib/v/checker/tests/map_index_reference_value.out
@@ -1,14 +1,34 @@
vlib/v/checker/tests/map_index_reference_value.vv:7:3: notice: accessing a pointer map value requires an `or {}` block outside `unsafe`
5 | fn main() {
6 | mut m := map[string]&Foo{}
7 | m['bar'].bar = 'bar'
vlib/v/checker/tests/map_index_reference_value.vv:11:3: notice: accessing a pointer map value requires an `or {}` block outside `unsafe`
9 | fn main() {
10 | mut m := map[string]&Foo{}
11 | m['bar'].bar = 'bar'
| ~~~~~~~
8 | // m['bar'] << 'baz' // etc
9 | }
vlib/v/checker/tests/map_index_reference_value.vv:7:11: error: field `bar` of struct `&Foo` is immutable
5 | fn main() {
6 | mut m := map[string]&Foo{}
7 | m['bar'].bar = 'bar'
12 | // m['bar'] << 'baz' // etc
13 |
vlib/v/checker/tests/map_index_reference_value.vv:11:3: notice: accessing map value that contain pointers requires an `or {}` block outside `unsafe`
9 | fn main() {
10 | mut m := map[string]&Foo{}
11 | m['bar'].bar = 'bar'
| ~~~~~~~
12 | // m['bar'] << 'baz' // etc
13 |
vlib/v/checker/tests/map_index_reference_value.vv:15:8: notice: accessing map value that contain pointers requires an `or {}` block outside `unsafe`
13 |
14 | mut m2 := map[string]Bar{}
15 | _ = m2['key']
| ~~~~~~~
16 |
17 | mut m3 := map[string][]Bar{}
vlib/v/checker/tests/map_index_reference_value.vv:18:8: notice: accessing map value that contain pointers requires an `or {}` block outside `unsafe`
16 |
17 | mut m3 := map[string][]Bar{}
18 | _ = m3['key']
| ~~~~~~~
19 | }
vlib/v/checker/tests/map_index_reference_value.vv:11:11: error: field `bar` of struct `&Foo` is immutable
9 | fn main() {
10 | mut m := map[string]&Foo{}
11 | m['bar'].bar = 'bar'
| ~~~
8 | // m['bar'] << 'baz' // etc
9 | }
12 | // m['bar'] << 'baz' // etc
13 |
10 changes: 10 additions & 0 deletions vlib/v/checker/tests/map_index_reference_value.vv
Expand Up @@ -2,8 +2,18 @@ struct Foo {
bar string
}

struct Bar {
field &int
}

fn main() {
mut m := map[string]&Foo{}
m['bar'].bar = 'bar'
// m['bar'] << 'baz' // etc

mut m2 := map[string]Bar{}
_ = m2['key']

mut m3 := map[string][]Bar{}
_ = m3['key']
}
2 changes: 1 addition & 1 deletion vlib/v/gen/c/cmain.v
Expand Up @@ -268,7 +268,7 @@ pub fn (mut g Gen) gen_c_main_for_tests() {
g.writeln('')
for tnumber, tname in all_tfuncs {
tcname := util.no_dots(tname)
testfn := g.table.fns[tname]
testfn := unsafe { g.table.fns[tname] }
lnum := testfn.pos.line_nr + 1
g.writeln('\tmain__VTestFnMetaInfo_free(test_runner.fn_test_info);')
g.writeln('\tstring tcname_${tnumber} = _SLIT("${tcname}");')
Expand Down
4 changes: 2 additions & 2 deletions vlib/v/gen/c/orm.v
Expand Up @@ -281,7 +281,7 @@ fn (mut g Gen) write_orm_insert_with_last_ids(node ast.SqlStmtLine, connection_v
for field in node.fields {
sym := g.table.sym(field.typ)
if sym.kind == .struct_ && sym.name != 'time.Time' {
subs << node.sub_structs[int(field.typ)]
subs << unsafe { node.sub_structs[int(field.typ)] }
unwrapped_c_typ := g.typ(field.typ.clear_flag(.option))
subs_unwrapped_c_typ << if field.typ.has_flag(.option) { unwrapped_c_typ } else { '' }
} else if sym.kind == .array {
Expand All @@ -293,7 +293,7 @@ fn (mut g Gen) write_orm_insert_with_last_ids(node ast.SqlStmtLine, connection_v
if field.typ.has_flag(.option) {
opt_fields << arrs.len
}
arrs << node.sub_structs[int(field.typ)]
arrs << unsafe { node.sub_structs[int(field.typ)] }
field_names << field.name
}
}
Expand Down
2 changes: 1 addition & 1 deletion vlib/v/markused/markused.v
Expand Up @@ -367,7 +367,7 @@ pub fn mark_used(mut table ast.Table, pref_ &pref.Preferences, ast_files []&ast.
walker.mark_root_fns(all_fn_root_names)

if walker.n_asserts > 0 {
walker.fn_decl(mut all_fns['__print_assert_failure'])
unsafe { walker.fn_decl(mut all_fns['__print_assert_failure']) }
}
if table.used_maps > 0 {
for k, mut mfn in all_fns {
Expand Down
2 changes: 1 addition & 1 deletion vlib/v/markused/walker.v
Expand Up @@ -60,7 +60,7 @@ pub fn (mut w Walker) mark_root_fns(all_fn_root_names []string) {
$if trace_skip_unused_roots ? {
println('>>>> ${fn_name} uses: ')
}
w.fn_decl(mut w.all_fns[fn_name])
unsafe { w.fn_decl(mut w.all_fns[fn_name]) }
}
}
}
Expand Down

0 comments on commit 053a7a9

Please sign in to comment.