From ae7689f73901b94dcc9c41cbd932627543c67d3f Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Wed, 30 Sep 2020 07:27:24 +0200 Subject: [PATCH] autofree: simplify, clean up, and fix complex arg exprs --- vlib/v/gen/cgen.v | 35 ++++++++- vlib/v/gen/fn.v | 81 +++++++++++--------- vlib/v/pref/pref.v | 10 ++- vlib/v/tests/valgrind/1.strings_and_arrays.v | 4 +- 4 files changed, 86 insertions(+), 44 deletions(-) diff --git a/vlib/v/gen/cgen.v b/vlib/v/gen/cgen.v index 55bb6ae9a4..9dd78d74da 100644 --- a/vlib/v/gen/cgen.v +++ b/vlib/v/gen/cgen.v @@ -110,6 +110,8 @@ mut: match_sumtype_exprs []ast.Expr match_sumtype_syms []table.TypeSymbol // tmp_arg_vars_to_free []string + // autofree_pregen map[string]string + autofree_tmp_vars []string // to avoid redefining the same tmp vars in a single function called_fn_name string cur_mod string is_js_call bool // for handling a special type arg #1 `json.decode(User, ...)` @@ -738,6 +740,7 @@ 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.inside_or_block { // TODO remove the inside_or_block hack. strings are not freed in or{} atm if g.strs_to_free.len != 0 { @@ -750,6 +753,7 @@ fn (mut g Gen) stmt(node ast.Stmt) { // g.strs_to_free.free() } } + */ } // println('cgen.stmt()') // g.writeln('//// stmt start') @@ -816,7 +820,20 @@ 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) + } g.expr(node.expr) + if af { + if g.strs_to_free.len > 0 { + g.writeln(';\n// strs_to_free2:') + for s in g.strs_to_free { + g.writeln('string_free(&$s);') + } + g.strs_to_free = [] + } + } if g.inside_ternary == 0 && !node.is_expr && !(node.expr is ast.IfExpr) { g.writeln(';') } @@ -1931,14 +1948,24 @@ fn (mut g Gen) expr(node ast.Expr) { // if g.fileis('1.strings') { // println('before:' + node.autofree_pregen) // } - if g.pref.autofree && node.autofree_pregen != '' { // g.strs_to_free0.len != 0 { - // g.insert_before_stmt('/*START2*/' + g.strs_to_free0.join('\n') + '/*END*/') - g.insert_before_stmt('/*START3*/' + node.autofree_pregen + '/*END*/') - // for s in g.strs_to_free0 { + if g.pref.autofree { + // println('pos=$node.pos.pos') + } + // if g.pref.autofree && node.autofree_pregen != '' { // g.strs_to_free0.len != 0 { + /* + if g.pref.autofree { + s := g.autofree_pregen[node.pos.pos.str()] + if s != '' { + // g.insert_before_stmt('/*START2*/' + g.strs_to_free0.join('\n') + '/*END*/') + // g.insert_before_stmt('/*START3*/' + node.autofree_pregen + '/*END*/') + g.insert_before_stmt('/*START3*/' + s + '/*END*/') + // for s in g.strs_to_free0 { + } // //g.writeln(s) // } g.strs_to_free0 = [] } + */ } ast.CastExpr { // g.write('/*cast*/') diff --git a/vlib/v/gen/fn.v b/vlib/v/gen/fn.v index 8c11e67d65..bb5b5a17ef 100644 --- a/vlib/v/gen/fn.v +++ b/vlib/v/gen/fn.v @@ -15,6 +15,11 @@ fn (mut g Gen) gen_fn_decl(it ast.FnDecl, skip bool) { // || it.no_body { return } + if g.pref.autofree { + defer { + g.autofree_tmp_vars = [] + } + } // if g.fileis('vweb.v') { // println('\ngen_fn_decl() $it.name $it.is_generic $g.cur_generic_type') // } @@ -422,9 +427,7 @@ fn (mut g Gen) method_call(node ast.CallExpr) { } } // TODO2 - unsafe { - g.generate_tmp_autofree_arg_vars(mut node, name) - } + // g.generate_tmp_autofree_arg_vars(node, name) // // if node.receiver_type != 0 { // g.write('/*${g.typ(node.receiver_type)}*/') @@ -541,9 +544,8 @@ fn (mut g Gen) fn_call(node ast.CallExpr) { name += '_' + g.typ(node.generic_type) } // TODO2 - unsafe { - g.generate_tmp_autofree_arg_vars(mut node, name) - } + // cgen shouldn't modify ast nodes, this should be moved + // g.generate_tmp_autofree_arg_vars(node, name) // Handle `print(x)` if is_print && node.args[0].typ != table.string_type { // && !free_tmp_arg_vars { typ := node.args[0].typ @@ -623,39 +625,44 @@ fn (mut g Gen) fn_call(node ast.CallExpr) { g.is_json_fn = false } -fn (mut g Gen) generate_tmp_autofree_arg_vars(mut node ast.CallExpr, name string) { - // if g.fileis('1.strings') { - // println('gen tmp autofree()') - // } +fn (mut g Gen) autofree_call_pregen(node ast.CallExpr) { + g.writeln('// autofree_call()') // 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 && node.args.len > 0 && !node.args[0].typ.has_flag(.optional) // TODO copy pasta checker.v - // mut cur_line := '' - if free_tmp_arg_vars { - free_tmp_arg_vars = false // set the flag to true only if we have at least one arg to free - g.tmp_count2++ - for i, arg in node.args { - if !arg.is_tmp_autofree { - continue - } - free_tmp_arg_vars = true - // t := g.new_tmp_var() + '_arg_expr_${name}_$i' - fn_name := node.name.replace('.', '_') // can't use name... - t := '_tt${g.tmp_count2}_arg_expr_${fn_name}_$i' - g.called_fn_name = name - str_expr := g.write_expr_to_string(arg.expr) - // g.insert_before_stmt('string $t = $str_expr; // new4. to free arg #$i name=$name') - // g.strs_to_free0 << 'string $t = $str_expr; // new5. to free arg #$i name=$name' - node.autofree_pregen += 'string $t = $str_expr; // new6. to free arg #$i name=$name\n' - // println('setting pregen to ' + node.autofree_pregen) - // cur_line = g.go_before_stmt(0) - // println('cur line ="$cur_line"') - // g.writeln('string $t = $str_expr; // new3. to free arg #$i name=$name') - // Now free the tmp arg vars right after the function call - g.strs_to_free << 'string_free(&$t);' + if !free_tmp_arg_vars { + return + } + free_tmp_arg_vars = false // set the flag to true only if we have at least one arg to free + g.tmp_count2++ + for i, arg in node.args { + if !arg.is_tmp_autofree { + continue } - // g.strs_to_free << (';') + if arg.expr is ast.CallExpr { + // Any argument can be an expression that has to be freed. Generate a tmp expression + // for each of those recursively. + g.autofree_call_pregen(arg.expr as ast.CallExpr) + } + free_tmp_arg_vars = true + // t := g.new_tmp_var() + '_arg_expr_${name}_$i' + fn_name := node.name.replace('.', '_') // can't use name... + // t := '_tt${g.tmp_count2}_arg_expr_${fn_name}_$i' + t := '_arg_expr_${fn_name}_$i' + // g.called_fn_name = name + used := t in g.autofree_tmp_vars + if used { + g.write('$t = ') + } else { + g.write('string $t = ') + g.autofree_tmp_vars << t + } + g.expr(arg.expr) + g.writeln(';// new af pre') + // Now free the tmp arg vars right after the function call + g.strs_to_free << t + // g.strs_to_free << 'string_free(&$t);' } } @@ -698,7 +705,8 @@ fn (mut g Gen) call_args(node ast.CallExpr) { // g.write('_arg_expr_${g.called_fn_name}_$i') // Use these variables here. fn_name := node.name.replace('.', '_') - name := '_tt${g.tmp_count2}_arg_expr_${fn_name}_$i' + // name := '_tt${g.tmp_count2}_arg_expr_${fn_name}_$i' + name := '_arg_expr_${fn_name}_$i' g.write('/*af arg*/' + name) } } else { @@ -708,7 +716,8 @@ fn (mut g Gen) call_args(node ast.CallExpr) { if use_tmp_var_autofree { // TODO copypasta, move to an inline fn fn_name := node.name.replace('.', '_') - name := '_tt${g.tmp_count2}_arg_expr_${fn_name}_$i' + // name := '_tt${g.tmp_count2}_arg_expr_${fn_name}_$i' + name := '_arg_expr_${fn_name}_$i' g.write('/*af arg2*/' + name) } else { g.expr(arg.expr) diff --git a/vlib/v/pref/pref.v b/vlib/v/pref/pref.v index e683edafc5..d219c03496 100644 --- a/vlib/v/pref/pref.v +++ b/vlib/v/pref/pref.v @@ -41,8 +41,8 @@ pub enum CompilerType { } const ( - list_of_flags_with_param = ['o', 'output', 'd', 'define', 'b', 'backend', 'cc', 'os', 'target-os', - 'cf', 'cflags', 'path'] + list_of_flags_with_param = ['o', 'd', 'define', 'b', 'backend', 'cc', 'os', 'target-os', 'cf', + 'cflags', 'path'] ) pub struct Preferences { @@ -342,7 +342,11 @@ pub fn parse_args(args []string) (&Preferences, string) { continue } eprint('Unknown argument `$arg`') - eprintln(if command.len == 0 {''} else {' for command `$command`'}) + eprintln(if command.len == 0 { + '' + } else { + ' for command `$command`' + }) exit(1) } } diff --git a/vlib/v/tests/valgrind/1.strings_and_arrays.v b/vlib/v/tests/valgrind/1.strings_and_arrays.v index 7d6e0998d2..245526fd9e 100644 --- a/vlib/v/tests/valgrind/1.strings_and_arrays.v +++ b/vlib/v/tests/valgrind/1.strings_and_arrays.v @@ -31,6 +31,7 @@ fn str_tmp_expr() { println('a' + 'b') // tmp expression result must be freed handle_strings('c' + 'd', 'e' + 'f') // multiple tmp expressions must be freed handle_int(handle_strings('x' + 'y', 'f')) // exprs 2 levels deep must bee freed + handle_strings('1', add_strings('2', '3')) // test a fn that returns a string } fn str_tmp_expr_advanced() { @@ -41,7 +42,8 @@ fn str_tmp_expr_advanced() { // t1.free() // t2.free() // t3.free() - // handle_strings('c' + 'd', add_strings('e' + 'f', 'g')) // both lvl 1 and lvl2 exprs must be freed + // + handle_strings('c' + 'd', add_strings('e' + 'f', 'g')) // both lvl 1 and lvl2 exprs must be freed } struct Foo {