Skip to content

cgen,autofree: don't clone array fn args in struct field init, add test#26709

Open
tankf33der wants to merge 1 commit intovlang:masterfrom
tankf33der:cloned_arg
Open

cgen,autofree: don't clone array fn args in struct field init, add test#26709
tankf33der wants to merge 1 commit intovlang:masterfrom
tankf33der:cloned_arg

Conversation

@tankf33der
Copy link
Contributor

Slightly changing the strategy — it will be more beneficial for everyone if vlib/ works in -autofree mode, rather than examples/; examples/ will then work on its own if everything is done correctly and in full.

Right now all arrays in -autofree mode are cloned when placed into a struct; this is my attempt to implement an exception so that arrays.reverse_iterator() does not clone and it is possible to modify data in-place inside the loop.

v -autofree test vlib/v/tests/structs/ tests continue to pass successfully, just as before the patch.

From now v -autofree test vlib/arrays/reverse_iterator_test.v will work in -autofree mode too.

@tankf33der tankf33der requested a review from felipensp March 9, 2026 16:00
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d50f80510d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +788 to +790
is_fn_arg := field_type_sym.kind == .array && sfield.expr is ast.Ident
&& sfield.expr.obj is ast.Var && (sfield.expr.obj as ast.Var).is_arg
if !is_fn_arg {

Choose a reason for hiding this comment

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

P1 Badge Keep cloning for owning struct initializations

This new is_fn_arg fast path skips cloning for all array function arguments in struct literals, which changes ownership semantics beyond arrays.reverse_iterator. In -autofree, caller locals are still freed at scope end while generated struct free methods also free array fields, so code like fn wrap(a []int) S { return S{a:a} } with arr := [1]; s := wrap(arr) can now free the same backing buffer twice when both arr and s are cleaned up. The bypass should be limited to explicitly non-owning structs/fields instead of globally disabling gen_clone_assignment for arg-backed arrays.

Useful? React with 👍 / 👎.

@JalonSolov
Copy link
Collaborator

Close/re-open to run CI with latest V.

@JalonSolov JalonSolov closed this Mar 10, 2026
@JalonSolov JalonSolov reopened this Mar 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants