Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cgen: fix several issues with autofree(fix #20635) #20659

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 15 additions & 10 deletions vlib/v/gen/c/cgen.v
Original file line number Diff line number Diff line change
Expand Up @@ -2873,7 +2873,8 @@ fn (mut g Gen) get_ternary_name(name string) string {
}

fn (mut g Gen) gen_clone_assignment(val ast.Expr, typ ast.Type, add_eq bool) bool {
if val !in [ast.Ident, ast.SelectorExpr] {
if val !in [ast.Ident, ast.SelectorExpr, ast.CallExpr, ast.MatchExpr, ast.IfExpr, ast.IndexExpr,
ast.ParExpr, ast.UnsafeExpr] {
return false
}
right_sym := g.table.sym(typ)
Expand Down Expand Up @@ -2978,6 +2979,11 @@ fn (mut g Gen) autofree_scope_vars2(scope &ast.Scope, start_pos int, end_pos int
// TODO: free options
continue
}
is_result := obj.typ.has_flag(.result)
if is_result {
// TODO: free result
continue
}
g.autofree_variable(obj)
}
else {}
Expand Down Expand Up @@ -5348,6 +5354,14 @@ fn (mut g Gen) return_stmt(node ast.Return) {
node.types[0].has_flag(.option)
}
}
// autofree before `return`
// set free_parent_scopes to true, since all variables defined in parent
// scopes need to be freed before the return
if g.is_autofree {
if expr0 is ast.Ident {
g.returned_var_name = expr0.name
}
}
if fn_return_is_option && !expr_type_is_opt && return_sym.name != c.option_name {
styp := g.base_type(fn_ret_type)
g.writeln('${ret_typ} ${tmpvar};')
Expand Down Expand Up @@ -5408,15 +5422,6 @@ fn (mut g Gen) return_stmt(node ast.Return) {
g.writeln('return ${tmpvar};')
return
}
// autofree before `return`
// set free_parent_scopes to true, since all variables defined in parent
// scopes need to be freed before the return
if g.is_autofree {
expr := node.exprs[0]
if expr is ast.Ident {
g.returned_var_name = expr.name
}
}
// free := g.is_autofree && !g.is_builtin_mod // node.exprs[0] is ast.CallExpr
// Create a temporary variable for the return expression
if use_tmp_var || !g.is_builtin_mod {
Expand Down
58 changes: 58 additions & 0 deletions vlib/v/tests/autofree_test.v
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// vtest vflags: -autofree
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This // vtest vflags: -autofree is currently recognized only for .vv programs under vlib/v/gen/c/testdata/. The logic for it, is implemented in vlib/v/gen/c/coutput_test.v .

Autofree programs however, are usually tested by vlib/v/slow_tests/valgrind/valgrind_test.v , and are located under vlib/v/slow_tests/valgrind .

I think you should move that program there too, just rename fn test_main() { to fn main() {.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for telling me. ❤️
this is a complicated issue, I still need to fix it, finally I will move it to vlib/v/slow_tests/valgrind

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on it 🙇🏻‍♂️ .


fn return_array_with_result() ![]int {
mut arr := []int{}
arr << 123
return arr
}

fn return_array_with_option() ?[]int {
mut arr := []int{}
arr << 123
return arr
}

fn return_strng(s string) string {
return s
}

fn test_main() {
arr1 := return_array_with_result()!
assert arr1 == [123]
arr2 := return_array_with_option()?
assert arr2 == [123]

// test ident
str1 := '${'123'}abc'
str2 := str1
assert str2 == '123abc'

// test CallExpr
str3 := return_strng(str1)
assert str3 == '123abc'

// test MatchExpr
str4 := match true {
true {
str1
}
else {
str1
}
}
assert str4 == '123abc'

// test IfExpr
str5 := if true { str1 } else { str1 }
assert str5 == '123abc'

// test ParExpr
// vfmt off
str6 := (str1)
// vfmt on
assert str6 == '123abc'

// test UnsafeExpr
str7 := unsafe { str1 }
assert str7 == '123abc'
}