diff --git a/vlib/v/ast/ast.v b/vlib/v/ast/ast.v index 1cc88502e5..1476e71919 100644 --- a/vlib/v/ast/ast.v +++ b/vlib/v/ast/ast.v @@ -336,16 +336,17 @@ pub struct Stmt { */ pub struct Var { pub: - name string - expr Expr - share table.ShareType - is_mut bool - is_arg bool // fn args should not be autofreed + name string + expr Expr + share table.ShareType + is_mut bool + is_autofree_tmp bool + is_arg bool // fn args should not be autofreed pub mut: - typ table.Type - pos token.Position - is_used bool - is_changed bool // to detect mutable vars that are never changed + typ table.Type + pos token.Position + is_used bool + is_changed bool // to detect mutable vars that are never changed } pub struct GlobalField { diff --git a/vlib/v/gen/cgen.v b/vlib/v/gen/cgen.v index 04b25fa35a..89385cd166 100644 --- a/vlib/v/gen/cgen.v +++ b/vlib/v/gen/cgen.v @@ -103,7 +103,7 @@ mut: inside_return bool inside_or_block bool strs_to_free0 []string // strings.Builder - strs_to_free []string // strings.Builder + // strs_to_free []string // strings.Builder inside_call bool has_main bool inside_const bool @@ -118,6 +118,7 @@ mut: called_fn_name string cur_mod string is_js_call bool // for handling a special type arg #1 `json.decode(User, ...)` + nr_vars_to_free int } const ( @@ -718,6 +719,7 @@ fn (mut g Gen) stmts(stmts []ast.Stmt) { g.stmts_with_tmp_var(stmts, '') } +// tmp_var is used in `if` expressions only fn (mut g Gen) stmts_with_tmp_var(stmts []ast.Stmt, tmp_var string) { g.indent++ if g.inside_ternary > 0 { @@ -769,25 +771,6 @@ fn (mut g Gen) stmt(node ast.Stmt) { g.stmt_path_pos << g.out.len } 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 - if g.strs_to_free.len != 0 { - g.writeln('// strs_to_free:') - for s in g.strs_to_free { - g.writeln(s) - } - g.strs_to_free = [] - // s := g.strs_to_free.str() - // g.strs_to_free.free() - } - } - */ } // println('cgen.stmt()') // g.writeln('//// stmt start') @@ -1056,6 +1039,12 @@ fn (mut g Gen) stmt(node ast.Stmt) { if !g.skip_stmt_pos { // && g.stmt_path_pos.len > 0 { g.stmt_path_pos.delete_last() } + // 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 { + p := node.position() + g.autofree_call_postgen(p.pos) + } } fn (mut g Gen) write_defer_stmts() { @@ -1177,6 +1166,9 @@ fn (mut g Gen) for_in(it ast.ForInStmt) { } } g.stmts(it.stmts) + if it.key_type == table.string_type && !g.is_builtin_mod { + // g.writeln('string_free(&$key);') + } g.writeln('}') g.writeln('/*for in map cleanup*/') g.writeln('for (int $idx = 0; $idx < ${keys_tmp}.len; ++$idx) { string_free(&(($key_styp*)${keys_tmp}.data)[$idx]); }') @@ -1973,6 +1965,10 @@ fn (mut g Gen) autofree_var_call(free_fn_name string, v ast.Var) { // fn args should not be autofreed return } + if v.is_used && v.is_autofree_tmp { + // tmp expr vars do not need to be freed again here + return + } if v.typ.is_ptr() { g.writeln('\t${free_fn_name}(${c_name(v.name)}); // autofreed ptr var') } else { @@ -3582,8 +3578,8 @@ fn (mut g Gen) return_statement(node ast.Return, af bool) { g.writeln(';') if af { // free the tmp arg expr if we have one before the return - g.autofree_call_postgen() - g.strs_to_free = [] + g.autofree_call_postgen(node.pos.pos) + // g.strs_to_free = [] } styp := g.typ(g.fn_decl.return_type) g.writeln('return *($styp*)&$tmp;') diff --git a/vlib/v/gen/fn.v b/vlib/v/gen/fn.v index 5e7fec1444..6e49c70b15 100644 --- a/vlib/v/gen/fn.v +++ b/vlib/v/gen/fn.v @@ -663,15 +663,27 @@ 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) - mut s := if used { - '$t = ' + mut s := '' + if used { + // This means this tmp var name was already used (the same function was called and + // `_arg_fnname_1` was already generated. + // We do not need to declare this variable again, so just generate `t = ...` + // instead of `string t = ...`, and we need to mark this variable as unused, + // so that it's freed after the call. (Used tmp arg vars are not freed to avoid double frees). + if x := scope.find(t) { + match mut x { + ast.Var { x.is_used = false } + else {} + } + } + s = '$t = ' } else { 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 + 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 + is_autofree_tmp: true }) - 'string $t = ' + s = 'string $t = ' // g.autofree_tmp_vars << t } // g.expr(arg.expr) @@ -680,16 +692,23 @@ fn (mut g Gen) autofree_call_pregen(node ast.CallExpr) { 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 << t + g.nr_vars_to_free++ // g.strs_to_free << 'string_free(&$t);' } } -fn (mut g Gen) autofree_call_postgen() { +fn (mut g Gen) autofree_call_postgen(node_pos int) { + /* if g.strs_to_free.len == 0 { return } - g.writeln(';\n// strs_to_free2:') + */ + if g.nr_vars_to_free <= 0 { + return + } + g.writeln('\n// strs_to_free3:') + /* for s in g.strs_to_free { g.writeln('string_free(&$s);') } @@ -698,10 +717,37 @@ fn (mut g Gen) autofree_call_postgen() { // if we reset the array here, then the vars will not be freed after the block. g.strs_to_free = [] } + */ + scope := g.file.scope.innermost(node_pos) + for _, obj in scope.objects { + match mut obj { + ast.Var { + // if var.typ == 0 { + // // TODO why 0? + // continue + // } + v := *obj + is_optional := v.typ.has_flag(.optional) + if is_optional { + // TODO: free optionals + continue + } + if !v.is_autofree_tmp { + continue + } + if v.is_used { + // this means this tmp expr var has already been freed + continue + } + obj.is_used = true + g.autofree_variable(v) + g.nr_vars_to_free-- + } + else {} + } + } } -// fn (mut g Gen) call_args(args []ast.CallArg, expected_types []table.Type, tmp_arg_vars_to_free []string) { -// fn (mut g Gen) call_args(args []ast.CallArg, expected_types []table.Type) { fn (mut g Gen) call_args(node ast.CallExpr) { args := if g.is_js_call { node.args[1..] } else { node.args } expected_types := node.expected_arg_types diff --git a/vlib/v/tests/valgrind/1.strings_and_arrays.v b/vlib/v/tests/valgrind/1.strings_and_arrays.v index 6764047b30..570d05552d 100644 --- a/vlib/v/tests/valgrind/1.strings_and_arrays.v +++ b/vlib/v/tests/valgrind/1.strings_and_arrays.v @@ -175,6 +175,24 @@ fn return_if_expr() string { } } +fn loop_map() { + m := { + 'UK': 'London' + 'France': 'Paris' + } + // keys must be freed + for country, capital in m { + println(country + capital) + } +} + +fn free_inside_opt_block() { + x := opt('a' + 'b') or { + get_string('c' + 'd') // c+d must be freed before a+b + return + } +} + fn main() { println('start') simple() @@ -192,6 +210,8 @@ fn main() { addition_with_tmp_expr() if_expr() return_if_expr() + free_inside_opt_block() + // loop_map() println('end') }