From 053a7a986300a9d0a040f59dca2522df13e7f11e Mon Sep 17 00:00:00 2001 From: shove Date: Mon, 25 Dec 2023 19:57:52 +0800 Subject: [PATCH] ast, checker: add a notice, for accessing by key, map values, that contain pointers (to use unsafe or an `or {}` block) (#20266) --- vlib/v/ast/table.v | 2 +- vlib/v/checker/check_types.v | 37 ++++++++++++++++ vlib/v/checker/checker.v | 13 +++++- vlib/v/checker/fn.v | 24 +++++----- .../tests/map_index_reference_value.out | 44 ++++++++++++++----- .../tests/map_index_reference_value.vv | 10 +++++ vlib/v/gen/c/cmain.v | 2 +- vlib/v/gen/c/orm.v | 4 +- vlib/v/markused/markused.v | 2 +- vlib/v/markused/walker.v | 2 +- 10 files changed, 110 insertions(+), 30 deletions(-) diff --git a/vlib/v/ast/table.v b/vlib/v/ast/table.v index 684b9271f71630..d74cb06cdd76c2 100644 --- a/vlib/v/ast/table.v +++ b/vlib/v/ast/table.v @@ -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 diff --git a/vlib/v/checker/check_types.v b/vlib/v/checker/check_types.v index 27c412112159c7..f59fa83205eaee 100644 --- a/vlib/v/checker/check_types.v +++ b/vlib/v/checker/check_types.v @@ -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 +} diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index b41fc4419d94f4..caf10ea8022cdc 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -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 @@ -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() diff --git a/vlib/v/checker/fn.v b/vlib/v/checker/fn.v index e4156ec6c236cc..fc29be22bc2874 100644 --- a/vlib/v/checker/fn.v +++ b/vlib/v/checker/fn.v @@ -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 @@ -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 { @@ -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) @@ -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 } } @@ -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 } } @@ -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 { @@ -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 diff --git a/vlib/v/checker/tests/map_index_reference_value.out b/vlib/v/checker/tests/map_index_reference_value.out index 227fa1c6781687..b2aaf787b330cf 100644 --- a/vlib/v/checker/tests/map_index_reference_value.out +++ b/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 | diff --git a/vlib/v/checker/tests/map_index_reference_value.vv b/vlib/v/checker/tests/map_index_reference_value.vv index 949b045661b55b..633e669db651c6 100644 --- a/vlib/v/checker/tests/map_index_reference_value.vv +++ b/vlib/v/checker/tests/map_index_reference_value.vv @@ -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'] } diff --git a/vlib/v/gen/c/cmain.v b/vlib/v/gen/c/cmain.v index 7918ee8f24aa52..7cc5572a6df1ca 100644 --- a/vlib/v/gen/c/cmain.v +++ b/vlib/v/gen/c/cmain.v @@ -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}");') diff --git a/vlib/v/gen/c/orm.v b/vlib/v/gen/c/orm.v index 4134b413719c49..270671e7b86336 100644 --- a/vlib/v/gen/c/orm.v +++ b/vlib/v/gen/c/orm.v @@ -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 { @@ -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 } } diff --git a/vlib/v/markused/markused.v b/vlib/v/markused/markused.v index be1f809249d2e3..cba8ed9c65926d 100644 --- a/vlib/v/markused/markused.v +++ b/vlib/v/markused/markused.v @@ -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 { diff --git a/vlib/v/markused/walker.v b/vlib/v/markused/walker.v index e5b4501e9e6713..1f0c79aa0e782e 100644 --- a/vlib/v/markused/walker.v +++ b/vlib/v/markused/walker.v @@ -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]) } } } }