From 0e8c7ca2e3a47bc478c31c5866e4d34ed74c5960 Mon Sep 17 00:00:00 2001
From: Alexander Medvednikov <alexander@vlang.io>
Date: Sat, 7 Nov 2020 04:00:45 +0100
Subject: [PATCH] autofree: fix string reassignment

---
 vlib/v/gen/cgen.v                            | 21 ++++++++++++++++++--
 vlib/v/gen/fn.v                              |  4 ++--
 vlib/v/tests/valgrind/1.strings_and_arrays.v | 13 ++++++++++--
 3 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/vlib/v/gen/cgen.v b/vlib/v/gen/cgen.v
index 40737b78b4..7f17f75386 100644
--- a/vlib/v/gen/cgen.v
+++ b/vlib/v/gen/cgen.v
@@ -1412,13 +1412,30 @@ fn (mut g Gen) gen_assign_stmt(assign_stmt ast.AssignStmt) {
 		else {}
 	}
 	// Free the old value assigned to this string var (only if it's `str = [new value]`)
-	if g.pref.autofree && assign_stmt.op == .assign && assign_stmt.left_types.len == 1 &&
-		assign_stmt.left_types[0] == table.string_type && assign_stmt.left[0] is ast.Ident {
+	mut af := g.pref.autofree && assign_stmt.op == .assign && assign_stmt.left_types.len == 1 &&
+		assign_stmt.left_types[0] == table.string_type && assign_stmt.left[0] is ast.Ident
+	mut sref_name := ''
+	if af {
 		ident := assign_stmt.left[0] as ast.Ident
 		if ident.name != '_' {
+			/*
 			g.write('string_free(&')
 			g.expr(assign_stmt.left[0])
 			g.writeln('); // free str on re-assignment')
+			*/
+			sref_name = '_sref$assign_stmt.pos.pos'
+			g.write('string $sref_name = (') // TODO we are copying the entire string here, optimize
+			// we can't just do `.str` since we need the extra data from the string struct
+			// doing `&string` is also not an option since the stack memory with the data will be overwritten
+			g.expr(assign_stmt.left[0])
+			g.writeln('); // free str on re-assignment2')
+			defer {
+				if af {
+					g.writeln('string_free(&$sref_name);')
+				}
+			}
+		} else {
+			af = false
 		}
 	}
 	// Autofree tmp arg vars
diff --git a/vlib/v/gen/fn.v b/vlib/v/gen/fn.v
index 86e39f8b1a..2f773835b4 100644
--- a/vlib/v/gen/fn.v
+++ b/vlib/v/gen/fn.v
@@ -741,7 +741,7 @@ fn (mut g Gen) autofree_call_postgen(node_pos int) {
 		return
 	}
 	g.doing_autofree_tmp = true
-	g.write('/* postgen */')
+	// g.write('/* postgen */')
 	scope := g.file.scope.innermost(node_pos)
 	for _, obj in scope.objects {
 		match mut obj {
@@ -770,7 +770,7 @@ fn (mut g Gen) autofree_call_postgen(node_pos int) {
 			else {}
 		}
 	}
-	g.write('/* postgen end */')
+	// g.write('/* postgen end */')
 	g.doing_autofree_tmp = false
 }
 
diff --git a/vlib/v/tests/valgrind/1.strings_and_arrays.v b/vlib/v/tests/valgrind/1.strings_and_arrays.v
index 8f37576631..884aa3e51f 100644
--- a/vlib/v/tests/valgrind/1.strings_and_arrays.v
+++ b/vlib/v/tests/valgrind/1.strings_and_arrays.v
@@ -1,3 +1,4 @@
+// This program is built and run via Valgrind to ensure there are no leaks with -autofree
 fn simple() {
 	nums := [1, 2, 3] // local array must be freed
 	println(nums)
@@ -64,8 +65,12 @@ fn str_inter() {
 
 fn str_replace() {
 	mut s := 'hello world'
-	s = s.replace('hello', 'hi')
+	s = s.replace('hello', 'hi') // s can't be freed as usual before the assignment, since it's used in the right expr
 	println(s)
+	//
+	mut s2 := 'aa' + 'bb'
+	s2 = s2.replace('a', 'c')
+	println(s2)
 	/*
 	r := s.replace('hello', 'hi')
 	cloned := s.replace('hello', 'hi').clone()
@@ -85,8 +90,12 @@ fn str_replace2() {
 }
 
 fn reassign_str() {
+	mut x := 'a'
+	x = 'b' // nothing has to be freed here
+	//
 	mut s := 'a' + 'b'
 	s = 'x' + 'y' // 'a' + 'b' must be freed before the re-assignment
+	s = s + '!' // old s ref must be copied and freed after the assignment, since s is still used in the right expr
 }
 
 fn match_expr() string {
@@ -212,12 +221,12 @@ fn free_inside_opt_block() {
 fn main() {
 	println('start')
 	simple()
+	reassign_str()
 	str_tmp_expr()
 	str_tmp_expr_advanced()
 	str_tmp_expr_advanced_var_decl()
 	str_inter()
 	match_expr()
-	reassign_str()
 	optional_str()
 	// optional_return()
 	str_replace()