diff --git a/doc/docs.md b/doc/docs.md index 25f28b4fc6..7b07e910e1 100644 --- a/doc/docs.md +++ b/doc/docs.md @@ -1383,6 +1383,48 @@ fn read_log() { } ``` +If the function returns a value the `defer` block is executed *after* the return +expression is evaluated: + +```v +import os + +enum State { + normal + write_log + return_error +} + +// write log file and return number of bytes written +fn write_log(s State) ?int { + mut f := os.create('log.txt') ? + defer { + f.close() + } + if s == .write_log { + // `f.close()` will be called after `f.write()` has been + // executed, but before `write_log()` finally returns the + // number of bytes written to `main()` + return f.writeln('This is a log file') + } else if s == .return_error { + // the file will be closed after the `error()` function + // has returned - so the error message will still report + // it as open + return error('nothing written; file open: $f.is_opened') + } + // the file will be closed here, too + return 0 +} + +fn main() { + n := write_log(.return_error) or { + println('Error: $err') + 0 + } + println('$n bytes written') +} +``` + ## Structs ```v diff --git a/vlib/os/file.c.v b/vlib/os/file.c.v index 39cec934ff..1d64b6167b 100644 --- a/vlib/os/file.c.v +++ b/vlib/os/file.c.v @@ -155,7 +155,7 @@ pub fn (mut f File) write(buf []byte) ?int { } } */ - written := int(C.fwrite(buf.data, buf.len, 1, f.cfile)) + written := int(C.fwrite(buf.data, 1, buf.len, f.cfile)) if written == 0 && buf.len != 0 { return error('0 bytes written') } @@ -178,7 +178,7 @@ pub fn (mut f File) writeln(s string) ?int { } */ // TODO perf - written := int(C.fwrite(s.str, s.len, 1, f.cfile)) + written := int(C.fwrite(s.str, 1, s.len, f.cfile)) if written == 0 && s.len != 0 { return error('0 bytes written') } @@ -196,7 +196,7 @@ pub fn (mut f File) write_string(s string) ?int { return error('file is not opened') } // TODO perf - written := int(C.fwrite(s.str, s.len, 1, f.cfile)) + written := int(C.fwrite(s.str, 1, s.len, f.cfile)) if written == 0 && s.len != 0 { return error('0 bytes written') } diff --git a/vlib/v/gen/c/cgen.v b/vlib/v/gen/c/cgen.v index a6c347bfa6..5a394a6f40 100644 --- a/vlib/v/gen/c/cgen.v +++ b/vlib/v/gen/c/cgen.v @@ -1228,16 +1228,6 @@ fn (mut g Gen) stmt(node ast.Stmt) { } ast.NodeError {} ast.Return { - g.write_defer_stmts_when_needed() - // af := g.autofree && node.exprs.len > 0 && node.exprs[0] is ast.CallExpr && !g.is_builtin_mod - /* - af := g.autofree && !g.is_builtin_mod - if false && af { - g.writeln('// ast.Return free') - g.autofree_scope_vars(node.pos.pos - 1, node.pos.line_nr, true) - g.writeln('// ast.Return free_end2') - } - */ g.return_stmt(node) } ast.SqlStmt { @@ -4720,7 +4710,7 @@ fn (mut g Gen) gen_optional_error(target_type ast.Type, expr ast.Expr) { fn (mut g Gen) return_stmt(node ast.Return) { g.write_v_source_line_info(node.pos) if node.exprs.len > 0 { - // skip `retun $vweb.html()` + // skip `return $vweb.html()` if node.exprs[0] is ast.ComptimeCall { g.expr(node.exprs[0]) g.writeln(';') @@ -4737,6 +4727,7 @@ fn (mut g Gen) return_stmt(node ast.Return) { fn_return_is_optional := g.fn_decl.return_type.has_flag(.optional) mut has_semicolon := false if node.exprs.len == 0 { + g.write_defer_stmts_when_needed() if fn_return_is_optional { styp := g.typ(g.fn_decl.return_type) g.writeln('return ($styp){0};') @@ -4749,45 +4740,53 @@ fn (mut g Gen) return_stmt(node ast.Return) { } return } + tmpvar := g.new_tmp_var() + ret_typ := g.typ(g.fn_decl.return_type) + mut use_tmp_var := g.defer_stmts.len > 0 || g.defer_profile_code.len > 0 // handle promoting none/error/function returning 'Option' if fn_return_is_optional { optional_none := node.exprs[0] is ast.None ftyp := g.typ(node.types[0]) mut is_regular_option := ftyp in ['Option2', 'Option'] if optional_none || is_regular_option || node.types[0] == ast.error_type_idx { - g.write('return ') + if use_tmp_var { + g.write('$ret_typ $tmpvar = ') + } else { + g.write('return ') + } g.gen_optional_error(g.fn_decl.return_type, node.exprs[0]) - // g.writeln('; /*ret1*/') g.writeln(';') + if use_tmp_var { + g.write_defer_stmts_when_needed() + g.writeln('return $tmpvar;') + } return } } // regular cases - if fn_return_is_multi && node.exprs.len > 0 && !g.expr_is_multi_return_call(node.exprs[0]) { // not_optional_none { //&& !fn_return_is_optional { + if fn_return_is_multi && node.exprs.len > 0 && !g.expr_is_multi_return_call(node.exprs[0]) { if node.exprs.len == 1 && node.exprs[0] is ast.IfExpr { // use a temporary for `return if cond { x,y } else { a,b }` - tmpvar := g.new_tmp_var() - tmptyp := g.typ(g.fn_decl.return_type) - g.write('$tmptyp $tmpvar = ') + g.write('$ret_typ $tmpvar = ') g.expr(node.exprs[0]) g.writeln(';') + g.write_defer_stmts_when_needed() g.writeln('return $tmpvar;') return } // typ_sym := g.table.get_type_symbol(g.fn_decl.return_type) // mr_info := typ_sym.info as ast.MultiReturn mut styp := '' - mut opt_tmp := '' - mut opt_type := '' if fn_return_is_optional { - opt_type = g.typ(g.fn_decl.return_type) - // Create a tmp for this option - opt_tmp = g.new_tmp_var() - g.writeln('$opt_type $opt_tmp;') + g.writeln('$ret_typ $tmpvar;') styp = g.base_type(g.fn_decl.return_type) g.write('opt_ok(&($styp/*X*/[]) { ') } else { - g.write('return ') + if use_tmp_var { + g.write('$ret_typ $tmpvar = ') + } else { + g.write('return ') + } styp = g.typ(g.fn_decl.return_type) } // Use this to keep the tmp assignments in order @@ -4843,13 +4842,22 @@ fn (mut g Gen) return_stmt(node ast.Return) { } g.write('}') if fn_return_is_optional { - g.writeln(' }, (Option*)(&$opt_tmp), sizeof($styp));') - g.write('return $opt_tmp') + g.writeln(' }, (Option*)(&$tmpvar), sizeof($styp));') + g.write_defer_stmts_when_needed() + g.write('return $tmpvar') } // Make sure to add our unpacks if multi_unpack.len > 0 { g.insert_before_stmt(multi_unpack) } + if use_tmp_var && !fn_return_is_optional { + if !has_semicolon { + g.writeln(';') + } + g.write_defer_stmts_when_needed() + g.writeln('return $tmpvar;') + has_semicolon = true + } } else if node.exprs.len >= 1 { // normal return return_sym := g.table.get_type_symbol(node.types[0]) @@ -4865,10 +4873,7 @@ fn (mut g Gen) return_stmt(node ast.Return) { } if fn_return_is_optional && !expr_type_is_opt && return_sym.name !in ['Option2', 'Option'] { styp := g.base_type(g.fn_decl.return_type) - opt_type := g.typ(g.fn_decl.return_type) - // Create a tmp for this option - opt_tmp := g.new_tmp_var() - g.writeln('$opt_type $opt_tmp;') + g.writeln('$ret_typ $tmpvar;') g.write('opt_ok(&($styp[]) { ') if !g.fn_decl.return_type.is_ptr() && node.types[0].is_ptr() { if !(node.exprs[0] is ast.Ident && !g.is_amp) { @@ -4881,9 +4886,10 @@ fn (mut g Gen) return_stmt(node ast.Return) { g.write(', ') } } - g.writeln(' }, (Option*)(&$opt_tmp), sizeof($styp));') + g.writeln(' }, (Option*)(&$tmpvar), sizeof($styp));') + g.write_defer_stmts_when_needed() g.autofree_scope_vars(node.pos.pos - 1, node.pos.line_nr, true) - g.writeln('return $opt_tmp;') + g.writeln('return $tmpvar;') return } // autofree before `return` @@ -4897,24 +4903,20 @@ fn (mut g Gen) return_stmt(node ast.Return) { } // free := g.is_autofree && !g.is_builtin_mod // node.exprs[0] is ast.CallExpr // Create a temporary variable for the return expression - mut gen_tmp_var := !g.is_builtin_mod // node.exprs[0] is ast.CallExpr - mut tmp := '' - if gen_tmp_var { + use_tmp_var = use_tmp_var || !g.is_builtin_mod // node.exprs[0] is ast.CallExpr + if use_tmp_var { // `return foo(a, b, c)` // `tmp := foo(a, b, c); free(a); free(b); free(c); return tmp;` // Save return value in a temp var so that all args (a,b,c) can be freed // Don't use a tmp var if a variable is simply returned: `return x` if node.exprs[0] !is ast.Ident { - tmp = g.new_tmp_var() - // g.write('/*tmp return var*/ ') - g.write(' ') - g.write(g.typ(g.fn_decl.return_type)) - g.write(' ') - g.write(tmp) - g.write(' = ') + g.write('$ret_typ $tmpvar = ') } else { - gen_tmp_var = false - g.autofree_scope_vars(node.pos.pos - 1, node.pos.line_nr, true) + use_tmp_var = false + g.write_defer_stmts_when_needed() + if !g.is_builtin_mod { + g.autofree_scope_vars(node.pos.pos - 1, node.pos.line_nr, true) + } g.write('return ') } } else { @@ -4932,14 +4934,15 @@ fn (mut g Gen) return_stmt(node ast.Return) { } else { g.expr_with_cast(node.exprs[0], node.types[0], g.fn_decl.return_type) } - if gen_tmp_var { + if use_tmp_var { g.writeln(';') has_semicolon = true - if tmp != '' { + g.write_defer_stmts_when_needed() + if !g.is_builtin_mod { g.autofree_scope_vars(node.pos.pos - 1, node.pos.line_nr, true) - g.write('return $tmp') - has_semicolon = false } + g.write('return $tmpvar') + has_semicolon = false } } else { // if node.exprs.len == 0 { println('this should never happen') diff --git a/vlib/v/parser/parse_type.v b/vlib/v/parser/parse_type.v index 9cecde5c7f..4b49f3a232 100644 --- a/vlib/v/parser/parse_type.v +++ b/vlib/v/parser/parse_type.v @@ -359,7 +359,7 @@ pub fn (mut p Parser) parse_any_type(language ast.Language, is_ptr bool, check_d return p.parse_multi_return_type() } else { - // no defer + // no p.next() if name == 'map' { return p.parse_map_type() } @@ -369,79 +369,80 @@ pub fn (mut p Parser) parse_any_type(language ast.Language, is_ptr bool, check_d if name == 'thread' { return p.parse_thread_type() } - defer { - p.next() - } + mut ret := ast.Type(0) if name == '' { // This means the developer is using some wrong syntax like `x: int` instead of `x int` p.error('expecting type declaration') - return 0 - } - match name { - 'voidptr' { - return ast.voidptr_type - } - 'byteptr' { - return ast.byteptr_type - } - 'charptr' { - return ast.charptr_type - } - 'i8' { - return ast.i8_type - } - 'i16' { - return ast.i16_type - } - 'int' { - return ast.int_type - } - 'i64' { - return ast.i64_type - } - 'byte' { - return ast.byte_type - } - 'u16' { - return ast.u16_type - } - 'u32' { - return ast.u32_type - } - 'u64' { - return ast.u64_type - } - 'f32' { - return ast.f32_type - } - 'f64' { - return ast.f64_type - } - 'string' { - return ast.string_type - } - 'char' { - return ast.char_type - } - 'bool' { - return ast.bool_type - } - 'float_literal' { - return ast.float_literal_type - } - 'int_literal' { - return ast.int_literal_type - } - else { - if name.len == 1 && name[0].is_capital() { - return p.parse_generic_template_type(name) + } else { + match name { + 'voidptr' { + ret = ast.voidptr_type } - if p.peek_tok.kind == .lt { - return p.parse_generic_struct_inst_type(name) + 'byteptr' { + ret = ast.byteptr_type + } + 'charptr' { + ret = ast.charptr_type + } + 'i8' { + ret = ast.i8_type + } + 'i16' { + ret = ast.i16_type + } + 'int' { + ret = ast.int_type + } + 'i64' { + ret = ast.i64_type + } + 'byte' { + ret = ast.byte_type + } + 'u16' { + ret = ast.u16_type + } + 'u32' { + ret = ast.u32_type + } + 'u64' { + ret = ast.u64_type + } + 'f32' { + ret = ast.f32_type + } + 'f64' { + ret = ast.f64_type + } + 'string' { + ret = ast.string_type + } + 'char' { + ret = ast.char_type + } + 'bool' { + ret = ast.bool_type + } + 'float_literal' { + ret = ast.float_literal_type + } + 'int_literal' { + ret = ast.int_literal_type + } + else { + p.next() + if name.len == 1 && name[0].is_capital() { + return p.parse_generic_template_type(name) + } + if p.tok.kind == .lt { + return p.parse_generic_struct_inst_type(name) + } + return p.parse_enum_or_struct_type(name, language) } - return p.parse_enum_or_struct_type(name, language) } } + p.next() + return ret } } } diff --git a/vlib/v/tests/defer_return_test.v b/vlib/v/tests/defer_return_test.v new file mode 100644 index 0000000000..da5052f496 --- /dev/null +++ b/vlib/v/tests/defer_return_test.v @@ -0,0 +1,109 @@ +struct Qwe { +mut: + n int +} + +fn mut_x(mut x Qwe) &Qwe { + n := x.n + // defer statement should not have run, yet + assert n == 10 + x.n += 5 + return unsafe { &x } +} + +fn deferer() &Qwe { + mut s := &Qwe{ + n: 10 + } + defer { + // this should be done after the call to `mut_x()` + s.n += 2 + } + return mut_x(mut s) +} + +fn test_defer_in_return() { + q := deferer() + // both `mut_x()` and `defer` have been run + assert q.n == 17 +} + +fn defer_multi_ret(mut a Qwe) (int, f64) { + defer { + a.n *= 2 + } + // the return values should be calculated before `a.n` is doubled + return 3 * a.n, 2.5 * f64(a.n) +} + +fn test_defer_in_multi_return() { + mut x := Qwe{ + n: 3 + } + y, z := defer_multi_ret(mut x) + assert x.n == 6 + assert y == 9 + assert z == 7.5 +} + +fn option_return_good(mut a Qwe) ?Qwe { + defer { + a.n += 7 + } + a.n += 3 + return a +} + +fn test_defer_opt_return() { + mut x := Qwe{ + n: -2 + } + y := option_return_good(mut x) or { + Qwe{ + n: -100 + } + } + assert x.n == 8 + assert y.n == 1 +} + +fn option_return_err(mut a Qwe) ?Qwe { + defer { + a.n += 5 + } + a.n += 2 + return error('Error: $a.n') +} + +fn test_defer_err_return() { + mut x := Qwe{ + n: 17 + } + mut e_msg := '' + y := option_return_err(mut x) or { + e_msg = '$err' + Qwe{ + n: -119 + } + } + assert x.n == 24 + assert y.n == -119 + assert e_msg == 'Error: 19' +} + +fn return_opt_call(mut a Qwe) ?Qwe { + defer { + a.n += 5 + } + a.n += 2 + return option_return_good(mut a) +} + +fn test_defer_option_call() { + mut x := Qwe{ + n: -1 + } + b := return_opt_call(mut x) or { Qwe{} } + assert x.n == 16 + assert b.n == 4 +}