From 15bdb8f7cd2868a8396644b5c58331a497790274 Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Thu, 27 Aug 2020 11:30:28 +0200 Subject: [PATCH] autofree: tmp arg var frees fixes --- 0.3_roadmap.txt | 1 + vlib/os/os.v | 3 +- vlib/v/gen/cgen.v | 4 +- vlib/v/gen/fn.v | 37 ++++++++++++------- vlib/v/gen/str.v | 3 +- ...gs_and_arrays.v => 1.strings_and_arrays.v} | 9 +++++ 6 files changed, 41 insertions(+), 16 deletions(-) rename vlib/v/tests/valgrind/{strings_and_arrays.v => 1.strings_and_arrays.v} (81%) diff --git a/0.3_roadmap.txt b/0.3_roadmap.txt index aaf72e5b68..5cf552dc84 100644 --- a/0.3_roadmap.txt +++ b/0.3_roadmap.txt @@ -18,4 +18,5 @@ - vfmt: fix common errors automatically to save time (make vars mutable and vice versa, add missing imports etc) - method expressions with an explicit receiver as the first argument - fix all remaining generics issues (`foo(5)` instead of `foo(5)` etc) +- merge v.c and v_win.c diff --git a/vlib/os/os.v b/vlib/os/os.v index 4dcb94cfc9..c135bd76ba 100644 --- a/vlib/os/os.v +++ b/vlib/os/os.v @@ -128,7 +128,8 @@ pub fn cp_all(osource_path, odest_path string, overwrite bool) ? { } // single file copy if !os.is_dir(source_path) { - adjusted_path := if os.is_dir(dest_path) {os.join_path(dest_path,os.file_name(source_path)) } else { dest_path } + adjusted_path := if os.is_dir(dest_path) { +os.join_path(dest_path,os.file_name(source_path)) } else { dest_path } if os.exists(adjusted_path) { if overwrite { os.rm(adjusted_path) diff --git a/vlib/v/gen/cgen.v b/vlib/v/gen/cgen.v index c95f303296..db6e53c836 100644 --- a/vlib/v/gen/cgen.v +++ b/vlib/v/gen/cgen.v @@ -106,8 +106,9 @@ mut: comptime_var_type_map map[string]table.Type match_sumtype_exprs []ast.Expr match_sumtype_syms []table.TypeSymbol - tmp_idxs []int + tmp_arg_vars_to_free []string called_fn_name string + cur_mod string } const ( @@ -932,6 +933,7 @@ fn (mut g Gen) stmt(node ast.Stmt) { } ast.Module { g.is_builtin_mod = node.name == 'builtin' + g.cur_mod = node.name } ast.Return { g.write_defer_stmts_when_needed() diff --git a/vlib/v/gen/fn.v b/vlib/v/gen/fn.v index 55aacbaca5..45bdac0620 100644 --- a/vlib/v/gen/fn.v +++ b/vlib/v/gen/fn.v @@ -462,7 +462,6 @@ fn (mut g Gen) fn_call(node ast.CallExpr) { if is_json_encode { g.gen_json_for_type(node.args[0].typ) json_type_str = g.table.get_type_symbol(node.args[0].typ).name - // `json__encode` => `json__encode_User` encode_name := c_name(name) + '_' + util.no_dots(json_type_str) g.writeln('// json.encode') @@ -517,9 +516,10 @@ fn (mut g Gen) fn_call(node ast.CallExpr) { */ // Create a temporary var for each argument in order to free it (only if it's a complex expression, // like `foo(get_string())` or `foo(a + b)` - free_tmp_arg_vars := g.autofree && g.pref.experimental && node.args.len > 0 && !node.args[0].typ.has_flag(.optional) + free_tmp_arg_vars := g.autofree && g.pref.experimental && !g.is_builtin_mod && g.cur_mod != + 'os' && node.args.len > 0 && !node.args[0].typ.has_flag(.optional) if free_tmp_arg_vars { - g.tmp_idxs = []int{cap: 10} // TODO perf + g.tmp_arg_vars_to_free = []string{cap: 10} // TODO perf for i, arg in node.args { if arg.typ != table.string_type { continue @@ -527,7 +527,7 @@ fn (mut g Gen) fn_call(node ast.CallExpr) { if arg.expr is ast.Ident || arg.expr is ast.StringLiteral { continue } - t := '_arg_expr_${name}_$i' // g.new_tmp_var() + t := g.new_tmp_var() + '_arg_expr_${name}_$i' // g.new_tmp_var() g.called_fn_name = name /* g.write('string $t = ') @@ -535,12 +535,12 @@ fn (mut g Gen) fn_call(node ast.CallExpr) { g.writeln('; // to free $i ') */ str_expr := g.write_expr_to_string(arg.expr) - g.insert_before_stmt('string $t = $str_expr; // to free $i ') - g.tmp_idxs << i + g.insert_before_stmt('string $t = $str_expr; // new. to free $i ') + g.tmp_arg_vars_to_free << t // i } } // Handle `print(x)` - if is_print && node.args[0].typ != table.string_type { + if is_print && node.args[0].typ != table.string_type && !free_tmp_arg_vars { typ := node.args[0].typ mut styp := g.typ(typ) sym := g.table.get_type_symbol(typ) @@ -609,8 +609,14 @@ fn (mut g Gen) fn_call(node ast.CallExpr) { } g.is_c_call = false g.is_json_fn = false - if free_tmp_arg_vars { - g.tmp_idxs.clear() + if free_tmp_arg_vars && g.tmp_arg_vars_to_free.len > 0 { + // Now free the tmp arg vars right after the function call + g.writeln(';') + for tmp in g.tmp_arg_vars_to_free { + g.writeln('string_free(&$tmp);') + } + g.writeln('') + g.tmp_arg_vars_to_free.clear() } } @@ -640,10 +646,15 @@ fn (mut g Gen) call_args(args []ast.CallArg, expected_types []table.Type) { if is_interface { g.expr(arg.expr) } else if g.autofree && g.pref.experimental && arg.typ == table.string_type && - g.tmp_idxs.len > 0 && i in g.tmp_idxs { - // Save expressions in temp variables so that they can be freed later. - // `foo(str + str2) => x := str + str2; foo(x); x.free()` - g.write('_arg_expr_${g.called_fn_name}_$i') + g.tmp_arg_vars_to_free.len > 0 { // && g.tmp_arg_vars_to_free.contains('_$i') { // && i in g.tmp_idxs { + for name in g.tmp_arg_vars_to_free { + // Save expressions in temp variables so that they can be freed later. + // `foo(str + str2) => x := str + str2; foo(x); x.free()` + // g.write('_arg_expr_${g.called_fn_name}_$i') + if name.contains('_$i') { + g.write(name) + } + } } else { g.ref_or_deref_arg(arg, expected_types[i]) } diff --git a/vlib/v/gen/str.v b/vlib/v/gen/str.v index f17a4fa9c3..deb5251648 100644 --- a/vlib/v/gen/str.v +++ b/vlib/v/gen/str.v @@ -194,7 +194,8 @@ fn (mut g Gen) string_inter_literal_sb_optimized(call_expr ast.CallExpr) { fn (mut g Gen) string_inter_literal(node ast.StringInterLiteral) { mut cur_line := '' mut tmp := '' - free := g.pref.autofree && g.inside_call && !g.inside_return && g.inside_ternary == 0 && !g.inside_const + free := !g.pref.experimental && g.pref.autofree && g.inside_call && !g.inside_return && + g.inside_ternary == 0 && !g.inside_const // && g.cur_fn != 0 && // g.cur_fn.name != '' if free { diff --git a/vlib/v/tests/valgrind/strings_and_arrays.v b/vlib/v/tests/valgrind/1.strings_and_arrays.v similarity index 81% rename from vlib/v/tests/valgrind/strings_and_arrays.v rename to vlib/v/tests/valgrind/1.strings_and_arrays.v index 71bf62ebd2..5a0a8adf73 100644 --- a/vlib/v/tests/valgrind/strings_and_arrays.v +++ b/vlib/v/tests/valgrind/1.strings_and_arrays.v @@ -15,6 +15,14 @@ fn foo() { // nums.free() // this should result in a double free and a CI error } +fn handle_strings(s, p string) { +} + +fn str_expr() { + println('a' + 'b') // tmp expression result must be freed + handle_strings('c' + 'd', 'e' + 'f') +} + fn str_replace() { s := 'hello world' r := s.replace('hello', 'hi') @@ -27,6 +35,7 @@ fn str_replace() { fn main() { println('start') foo() + // str_expr() // str_replace() println('end') }