From 55536bb364a96921f82bb2b7e6c48fe4f9ebd3a0 Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Sun, 18 Oct 2020 00:48:06 +0200 Subject: [PATCH] autofree: handle more statements --- vlib/builtin/string_test.v | 4 +- vlib/v/gen/cgen.v | 53 ++++++++++++-------- vlib/v/gen/fn.v | 15 +++--- vlib/v/tests/valgrind/1.strings_and_arrays.v | 25 ++++++++- 4 files changed, 68 insertions(+), 29 deletions(-) diff --git a/vlib/builtin/string_test.v b/vlib/builtin/string_test.v index 6a1b6ac652..651fea607d 100644 --- a/vlib/builtin/string_test.v +++ b/vlib/builtin/string_test.v @@ -241,6 +241,8 @@ two ', four!'] s = strings.join(' ') assert s.contains('one') && s.contains('two ') && s.contains('four') + empty := []string{len:0} + assert empty.join('A') == '' } fn test_clone() { @@ -906,7 +908,7 @@ fn test_sorter() { i: 102 } ] - cmp := fn (a, b &Ka) int { + cmp := fn (a &Ka, b &Ka) int { return compare_strings(a.s, b.s) } arr.sort_with_compare(cmp) diff --git a/vlib/v/gen/cgen.v b/vlib/v/gen/cgen.v index 5e2a7ab3e8..edbb14851b 100644 --- a/vlib/v/gen/cgen.v +++ b/vlib/v/gen/cgen.v @@ -100,8 +100,8 @@ mut: inside_vweb_tmpl bool inside_return bool inside_or_block bool + strs_to_free0 []string // strings.Builder strs_to_free []string // strings.Builder - // strs_to_free0 []string // strings.Builder inside_call bool has_main bool inside_const bool @@ -111,6 +111,7 @@ mut: match_sumtype_syms []table.TypeSymbol // tmp_arg_vars_to_free []string // autofree_pregen map[string]string + // autofree_pregen_buf strings.Builder // autofree_tmp_vars []string // to avoid redefining the same tmp vars in a single function called_fn_name string cur_mod string @@ -755,6 +756,9 @@ fn (mut g Gen) stmt(node ast.Stmt) { defer { // If we have temporary string exprs to free after this statement, do it. e.g.: // `foo('a' + 'b')` => `tmp := 'a' + 'b'; foo(tmp); string_free(&tmp);` + if g.pref.autofree { + g.autofree_call_postgen() + } /* if g.pref.autofree { // && !g.inside_or_block { // TODO remove the inside_or_block hack. strings are not freed in or{} atm @@ -835,14 +839,14 @@ fn (mut g Gen) stmt(node ast.Stmt) { } ast.ExprStmt { g.write_v_source_line_info(node.pos) - af := g.pref.autofree && node.expr is ast.CallExpr && !g.is_builtin_mod - if af { - g.autofree_call_pregen(node.expr as ast.CallExpr) - } + // af := g.pref.autofree && node.expr is ast.CallExpr && !g.is_builtin_mod + // if af { + // g.autofree_call_pregen(node.expr as ast.CallExpr) + // } g.expr(node.expr) - if af { - g.autofree_call_postgen() - } + // if af { + // g.autofree_call_postgen() + // } if g.inside_ternary == 0 && !node.is_expr && !(node.expr is ast.IfExpr) { g.writeln(';') } @@ -1011,9 +1015,9 @@ fn (mut g Gen) stmt(node ast.Stmt) { af := g.pref.autofree && node.exprs.len > 0 && node.exprs[0] is ast.CallExpr && !g.is_builtin_mod if g.pref.autofree { g.writeln('// ast.Return free') - if af { - g.autofree_call_pregen(node.exprs[0] as ast.CallExpr) - } + // if af { + // g.autofree_call_pregen(node.exprs[0] as ast.CallExpr) + // } // g.autofree_scope_vars(node.pos.pos) g.write_autofree_stmts_when_needed(node) } @@ -1390,11 +1394,12 @@ fn (mut g Gen) gen_assign_stmt(assign_stmt ast.AssignStmt) { } } // Autofree tmp arg vars - first_right := assign_stmt.right[0] - af := g.pref.autofree && first_right is ast.CallExpr && !g.is_builtin_mod - if af { - g.autofree_call_pregen(first_right as ast.CallExpr) - } + // first_right := assign_stmt.right[0] + // af := g.pref.autofree && first_right is ast.CallExpr && !g.is_builtin_mod + // if af { + // g.autofree_call_pregen(first_right as ast.CallExpr) + // } + // // // Handle optionals. We need to declare a temp variable for them, that's why they are handled // here, not in call_expr(). @@ -1422,9 +1427,9 @@ fn (mut g Gen) gen_assign_stmt(assign_stmt ast.AssignStmt) { gen_or = true g.or_block(tmp_opt, call_expr.or_block, call_expr.return_type) g.writeln('/*=============ret*/') - if af && is_optional { - g.autofree_call_postgen() - } + // if af && is_optional { + // g.autofree_call_postgen() + // } // return } } @@ -1987,7 +1992,15 @@ fn (mut g Gen) expr(node ast.Expr) { // if g.fileis('1.strings') { // println('before:' + node.autofree_pregen) // } - if g.pref.autofree { + if g.pref.autofree && !g.is_builtin_mod && g.strs_to_free0.len == 0 { + // if len != 0, that means we are handling call expr inside call expr (arg) + // and it'll get messed up here, since it's handled recursively in autofree_call_pregen() + // so just skip it + g.autofree_call_pregen(node) + if g.strs_to_free0.len > 0 { + g.insert_before_stmt(g.strs_to_free0.join('\n')) + } + g.strs_to_free0 = [] // println('pos=$node.pos.pos') } // if g.pref.autofree && node.autofree_pregen != '' { // g.strs_to_free0.len != 0 { diff --git a/vlib/v/gen/fn.v b/vlib/v/gen/fn.v index 0bcb23a183..5c7215974c 100644 --- a/vlib/v/gen/fn.v +++ b/vlib/v/gen/fn.v @@ -620,7 +620,7 @@ fn (mut g Gen) fn_call(node ast.CallExpr) { } fn (mut g Gen) autofree_call_pregen(node ast.CallExpr) { - g.writeln('// autofree_call()') + // g.writeln('// autofree_call_pregen()') // Create a temporary var before fn call for each argument in order to free it (only if it's a complex expression, // like `foo(get_string())` or `foo(a + b)` mut free_tmp_arg_vars := g.autofree && g.pref.experimental && !g.is_builtin_mod && @@ -648,19 +648,22 @@ fn (mut g Gen) autofree_call_pregen(node ast.CallExpr) { // g.called_fn_name = name // used := t in g.autofree_tmp_vars used := scope.known_var(t) - if used { - g.write('$t = ') + mut s := if used { + '$t = ' } else { - g.write('string $t = ') scope.register(t, ast.Var{ name: t typ: table.string_type is_arg: true // TODO hack so that it's not freed twice when out of scope. it's probably better to use one model }) + 'string $t = ' // g.autofree_tmp_vars << t } - g.expr(arg.expr) - g.writeln(';// new af pre') + // g.expr(arg.expr) + s += g.write_expr_to_string(arg.expr) + // g.writeln(';// new af pre') + s += ';// new af2 pre' + g.strs_to_free0 << s // Now free the tmp arg vars right after the function call g.strs_to_free << t // g.strs_to_free << 'string_free(&$t);' diff --git a/vlib/v/tests/valgrind/1.strings_and_arrays.v b/vlib/v/tests/valgrind/1.strings_and_arrays.v index 496241eb68..77e427afed 100644 --- a/vlib/v/tests/valgrind/1.strings_and_arrays.v +++ b/vlib/v/tests/valgrind/1.strings_and_arrays.v @@ -13,14 +13,14 @@ fn foo() { // nums.free() // this should result in a double free and a CI error } -fn handle_strings(s, p string) int { +fn handle_strings(s string, p string) int { return 0 } fn handle_int(n int) { } -fn add_strings(a, b string) string { +fn add_strings(a string, b string) string { return a + b } @@ -120,6 +120,25 @@ fn optional_return() { } } +fn handle_string(s string) bool { + return true +} + +fn if_cond() { + // handle_string('1' + '2') + if handle_string('1' + '2') { + // if '1' + '2' == '12' { + println('yes') + } else { + println('no') + } +} + +fn addition_with_tmp_expr() { + x := 1 + handle_strings('a' + 'b', 'c') + println(x) +} + fn tt() { // time.parse_rfc2822('1234') } @@ -136,6 +155,8 @@ fn main() { optional_str() optional_return() // str_replace() + if_cond() + addition_with_tmp_expr() println('end') }