autofree: simplify, use more scoping, fix expressions inside or blocks

vlib/v/ast/ast.v

@@ -340,6 +340,7 @@ pub:
 	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

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.autofree_call_postgen(node.pos.pos)
-				g.strs_to_free = []
+				// g.strs_to_free = []
 			}
 			styp := g.typ(g.fn_decl.return_type)
 			g.writeln('return *($styp*)&$tmp;')

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 {
+		mut s := ''
-			'$t = '
+		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
+				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_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

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')
 }