From 163c7ba2bbf2583667c121beb07104277e780e73 Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Tue, 5 Jul 2022 23:51:29 +0300 Subject: [PATCH] checker: stricter `[]&Type{len:x}` check --- vlib/os/os.c.v | 79 +++++++++++++------------ vlib/sync/pool/pool.v | 34 ++++++----- vlib/v/builder/builder.v | 18 +++--- vlib/v/checker/checker.v | 4 +- vlib/v/checker/containers.v | 7 +++ vlib/v/checker/tests/ptr_array_init.out | 7 +++ vlib/v/checker/tests/ptr_array_init.vv | 3 + vlib/v/gen/c/cgen.v | 24 ++++---- vlib/v/parser/parser.v | 24 ++++---- vlib/v/parser/struct.v | 2 +- 10 files changed, 119 insertions(+), 83 deletions(-) create mode 100644 vlib/v/checker/tests/ptr_array_init.out create mode 100644 vlib/v/checker/tests/ptr_array_init.vv diff --git a/vlib/os/os.c.v b/vlib/os/os.c.v index 5197065768..d626176696 100644 --- a/vlib/os/os.c.v +++ b/vlib/os/os.c.v @@ -949,23 +949,26 @@ pub fn open_append(path string) ?File { // Note: this function will NOT return when successfull, since // the child process will take control over execution. pub fn execvp(cmdpath string, cmdargs []string) ? { - mut cargs := []&char{} - cargs << &char(cmdpath.str) - for i in 0 .. cmdargs.len { - cargs << &char(cmdargs[i].str) + unsafe { + mut cargs := []&char{} + cargs << &char(cmdpath.str) + for i in 0 .. cmdargs.len { + cargs << &char(cmdargs[i].str) + } + cargs << &char(0) + mut res := int(0) + $if windows { + res = C._execvp(&char(cmdpath.str), cargs.data) + } $else { + res = C.execvp(&char(cmdpath.str), cargs.data) + } + if res == -1 { + return error_with_code(posix_get_error_msg(C.errno), C.errno) + } + + // just in case C._execvp returned ... that happens on windows ... + exit(res) } - cargs << &char(0) - mut res := int(0) - $if windows { - res = C._execvp(&char(cmdpath.str), cargs.data) - } $else { - res = C.execvp(&char(cmdpath.str), cargs.data) - } - if res == -1 { - return error_with_code(posix_get_error_msg(C.errno), C.errno) - } - // just in case C._execvp returned ... that happens on windows ... - exit(res) } // execve - loads and executes a new child process, *in place* of the current process. @@ -975,27 +978,29 @@ pub fn execvp(cmdpath string, cmdargs []string) ? { // Note: this function will NOT return when successfull, since // the child process will take control over execution. pub fn execve(cmdpath string, cmdargs []string, envs []string) ? { - mut cargv := []&char{} - mut cenvs := []&char{} - cargv << &char(cmdpath.str) - for i in 0 .. cmdargs.len { - cargv << &char(cmdargs[i].str) - } - for i in 0 .. envs.len { - cenvs << &char(envs[i].str) - } - cargv << &char(0) - cenvs << &char(0) - mut res := int(0) - $if windows { - res = C._execve(&char(cmdpath.str), cargv.data, cenvs.data) - } $else { - res = C.execve(&char(cmdpath.str), cargv.data, cenvs.data) - } - // Note: normally execve does not return at all. - // If it returns, then something went wrong... - if res == -1 { - return error_with_code(posix_get_error_msg(C.errno), C.errno) + unsafe { + mut cargv := []&char{} + mut cenvs := []&char{} + cargv << &char(cmdpath.str) + for i in 0 .. cmdargs.len { + cargv << &char(cmdargs[i].str) + } + for i in 0 .. envs.len { + cenvs << &char(envs[i].str) + } + cargv << &char(0) + cenvs << &char(0) + mut res := int(0) + $if windows { + res = C._execve(&char(cmdpath.str), cargv.data, cenvs.data) + } $else { + res = C.execve(&char(cmdpath.str), cargv.data, cenvs.data) + } + // Note: normally execve does not return at all. + // If it returns, then something went wrong... + if res == -1 { + return error_with_code(posix_get_error_msg(C.errno), C.errno) + } } } diff --git a/vlib/sync/pool/pool.v b/vlib/sync/pool/pool.v index 3bf9ad0ab8..071d9410d0 100644 --- a/vlib/sync/pool/pool.v +++ b/vlib/sync/pool/pool.v @@ -81,17 +81,19 @@ pub fn (mut pool PoolProcessor) work_on_pointers(items []voidptr) { if pool.njobs > 0 { njobs = pool.njobs } - pool.thread_contexts = []voidptr{len: items.len} - pool.results = []voidptr{len: items.len} - pool.items = []voidptr{cap: items.len} - pool.items << items - pool.waitgroup.add(njobs) - for i := 0; i < njobs; i++ { - if njobs > 1 { - go process_in_thread(mut pool, i) - } else { - // do not run concurrently, just use the same thread: - process_in_thread(mut pool, i) + unsafe { + pool.thread_contexts = []voidptr{len: items.len} + pool.results = []voidptr{len: items.len} + pool.items = []voidptr{cap: items.len} + pool.items << items + pool.waitgroup.add(njobs) + for i := 0; i < njobs; i++ { + if njobs > 1 { + go process_in_thread(mut pool, i) + } else { + // do not run concurrently, just use the same thread: + process_in_thread(mut pool, i) + } } } pool.waitgroup.wait() @@ -136,11 +138,13 @@ pub fn (pool &PoolProcessor) get_results() []T { // get_results_ref - get a list of type safe results in the main thread. pub fn (pool &PoolProcessor) get_results_ref() []&T { - mut res := []&T{cap: pool.results.len} - for i in 0 .. pool.results.len { - res << &T(pool.results[i]) + unsafe { + mut res := []&T{cap: pool.results.len} + for i in 0 .. pool.results.len { + res << &T(pool.results[i]) + } + return res } - return res } // set_shared_context - can be called during the setup so that you can diff --git a/vlib/v/builder/builder.v b/vlib/v/builder/builder.v index d5710973be..f3b0c86223 100644 --- a/vlib/v/builder/builder.v +++ b/vlib/v/builder/builder.v @@ -251,17 +251,19 @@ pub fn (mut b Builder) resolve_deps() { eprintln(mods.str()) eprintln('-------------------------------') } - mut reordered_parsed_files := []&ast.File{} - for m in mods { - for pf in b.parsed_files { - if m == pf.mod.name { - reordered_parsed_files << pf - // eprintln('pf.mod.name: $pf.mod.name | pf.path: $pf.path') + unsafe { + mut reordered_parsed_files := []&ast.File{} + for m in mods { + for pf in b.parsed_files { + if m == pf.mod.name { + reordered_parsed_files << pf + // eprintln('pf.mod.name: $pf.mod.name | pf.path: $pf.path') + } } } + b.table.modules = mods + b.parsed_files = reordered_parsed_files } - b.table.modules = mods - b.parsed_files = reordered_parsed_files } // graph of all imported modules diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index 9083b8219b..2a9c2d83ee 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -271,9 +271,10 @@ pub fn (mut c Checker) check_files(ast_files []&ast.File) { // c.files = ast_files mut has_main_mod_file := false mut has_main_fn := false + unsafe { mut files_from_main_module := []&ast.File{} for i in 0 .. ast_files.len { - mut file := unsafe { ast_files[i] } + mut file := ast_files[i] c.timers.start('checker_check $file.path') c.check(file) if file.mod.name == 'main' { @@ -302,6 +303,7 @@ pub fn (mut c Checker) check_files(ast_files []&ast.File) { has_main_fn = true } } + } c.timers.start('checker_post_process_generic_fns') last_file := c.file // post process generic functions. must be done after all files have been diff --git a/vlib/v/checker/containers.v b/vlib/v/checker/containers.v index 647914c58f..ce88c2c3b0 100644 --- a/vlib/v/checker/containers.v +++ b/vlib/v/checker/containers.v @@ -56,6 +56,13 @@ pub fn (mut c Checker) array_init(mut node ast.ArrayInit) ast.Type { && c.table.cur_fn.generic_names.len == 0 { c.error('generic struct cannot use in non-generic function', node.pos) } + + // &int{} check + if node.elem_type.is_any_kind_of_pointer() && !c.inside_unsafe + && (node.has_len || node.has_cap) { + c.warn('arrays of references need to be initialized right away (unless used inside `unsafe`)', + node.pos) + } return node.typ } if node.is_fixed { diff --git a/vlib/v/checker/tests/ptr_array_init.out b/vlib/v/checker/tests/ptr_array_init.out new file mode 100644 index 0000000000..dd72b47e9f --- /dev/null +++ b/vlib/v/checker/tests/ptr_array_init.out @@ -0,0 +1,7 @@ +vlib/v/checker/tests/sum.vv:2:14: warning: arrays of references need to be initialized right away (unless used inside `unsafe`) + 1 | fn main() { + 2 | println(*[]&int{len: 1}[0]) + | ~~~~~~~ + 3 | } + 4 | + diff --git a/vlib/v/checker/tests/ptr_array_init.vv b/vlib/v/checker/tests/ptr_array_init.vv new file mode 100644 index 0000000000..e0fb2d1f48 --- /dev/null +++ b/vlib/v/checker/tests/ptr_array_init.vv @@ -0,0 +1,3 @@ +fn main() { + println(*[]&int{len: 1}[0]) +} diff --git a/vlib/v/gen/c/cgen.v b/vlib/v/gen/c/cgen.v index a3e93c86fa..38ef07c695 100644 --- a/vlib/v/gen/c/cgen.v +++ b/vlib/v/gen/c/cgen.v @@ -4896,14 +4896,16 @@ fn (mut g Gen) write_sorted_types() { defer { g.type_definitions.writeln('// #end sorted_symbols') } - mut symbols := []&ast.TypeSymbol{cap: g.table.type_symbols.len} // structs that need to be sorted - for sym in g.table.type_symbols { - if sym.name !in c.builtins { - symbols << sym + unsafe { + mut symbols := []&ast.TypeSymbol{cap: g.table.type_symbols.len} // structs that need to be sorted + for sym in g.table.type_symbols { + if sym.name !in c.builtins { + symbols << sym + } } + sorted_symbols := g.sort_structs(symbols) + g.write_types(sorted_symbols) } - sorted_symbols := g.sort_structs(symbols) - g.write_types(sorted_symbols) } fn (mut g Gen) write_types(symbols []&ast.TypeSymbol) { @@ -5186,11 +5188,13 @@ fn (mut g Gen) sort_structs(typesa []&ast.TypeSymbol) []&ast.TypeSymbol { '\nif you feel this is an error, please create a new issue here: https://github.com/vlang/v/issues and tag @joe-conigliaro') } // sort types - mut sorted_symbols := []&ast.TypeSymbol{cap: dep_graph_sorted.nodes.len} - for node in dep_graph_sorted.nodes { - sorted_symbols << g.table.sym_by_idx(g.table.type_idxs[node.name]) + unsafe { + mut sorted_symbols := []&ast.TypeSymbol{cap: dep_graph_sorted.nodes.len} + for node in dep_graph_sorted.nodes { + sorted_symbols << g.table.sym_by_idx(g.table.type_idxs[node.name]) + } + return sorted_symbols } - return sorted_symbols } [inline] diff --git a/vlib/v/parser/parser.v b/vlib/v/parser/parser.v index ee4aa7d362..e0b4e6da1b 100644 --- a/vlib/v/parser/parser.v +++ b/vlib/v/parser/parser.v @@ -99,7 +99,7 @@ pub mut: vet_errors []vet.Error } -__global codegen_files = []&ast.File{} +__global codegen_files = unsafe { []&ast.File{} } // for tests pub fn parse_stmt(text string, table &ast.Table, scope &ast.Scope) ast.Stmt { @@ -425,17 +425,19 @@ pub fn parse_files(paths []string, table &ast.Table, pref &pref.Preferences) []& } */ } - mut files := []&ast.File{cap: paths.len} - for path in paths { - timers.start('parse_file $path') - files << parse_file(path, table, .skip_comments, pref) - timers.show('parse_file $path') + unsafe { + mut files := []&ast.File{cap: paths.len} + for path in paths { + timers.start('parse_file $path') + files << parse_file(path, table, .skip_comments, pref) + timers.show('parse_file $path') + } + if codegen_files.len > 0 { + files << codegen_files + codegen_files.clear() + } + return files } - if codegen_files.len > 0 { - files << codegen_files - codegen_files.clear() - } - return files } // codegen allows you to generate V code, so that it can be parsed, diff --git a/vlib/v/parser/struct.v b/vlib/v/parser/struct.v index 83ceafb3ff..36c5fa9e96 100644 --- a/vlib/v/parser/struct.v +++ b/vlib/v/parser/struct.v @@ -39,7 +39,7 @@ fn (mut p Parser) struct_decl(is_anon bool) ast.StructDecl { if p.disallow_declarations_in_script_mode() { return ast.StructDecl{} } - mut name := if is_anon { '' } else { p.check_name()} + mut name := if is_anon { '' } else { p.check_name() } if name.len == 1 && name[0].is_capital() { p.error_with_pos('single letter capital names are reserved for generic template types.', name_pos)