cgen: fix array auto cloning #20078
cgen: fix array auto cloning #20078
Conversation
vlib/v/gen/c/fn.v
Outdated
| @@ -1525,7 +1525,21 @@ fn (mut g Gen) method_call(node ast.CallExpr) { | |||
| g.write(')') | |||
| } | |||
| } else { | |||
| g.expr(node.left) | |||
| if is_range_slice && node.left is ast.IndexExpr && node.left.index is ast.RangeExpr { | |||
There was a problem hiding this comment.
there is no need to nest this if further; it can be:
} else if ... { ... } else { g.expr(node.left) }
vlib/v/gen/c/fn.v
Outdated
|
|
||
| if arr_sym.kind == .array_fixed { | ||
| arr_info := arr_sym.array_fixed_info() | ||
| g.write('new_array_from_c_array_no_alloc(${arr_info.size}, ${arr_info.size}, sizeof(${g.typ(arr_info.elem_type)}), ') |
There was a problem hiding this comment.
new_array_from_c_array_no_alloc will create a dynamic array, that will have its .data pointer the same as the last argument. This is not safe in the general case, and can lead to hard to diagnose bugs (the fixed arrays are usually on the stack, dynamic arrays and slices to them usually have their data on the heap; breaking that assumption can lead to leaking/returning dynamic arrays, with their .data pointer set to an address on the stack, and thus after the return, a potential dangling pointer).
Imho use new_array_from_c_array instead, which will copy the data.
There was a problem hiding this comment.
Note also that:
fixed := [8]int{}
dynamic := fixed[..]
dump(dynamic)
dump(dynamic.data)
dump(fixed)
dump(unsafe { voidptr(&fixed[0]) })also creates a copy, it does not reuse the data, for the same reason, that it is not safe to do so in the general case.
There was a problem hiding this comment.
I've fixed the slicing on array/array_fixed.
There was a problem hiding this comment.
In the end, it was just one line fix.
felipensp
changed the title
Fix #20071
馃[deprecated] Generated by Copilot at 2142430
Implement range slices for fixed arrays in the C backend. Use
new_array_from_c_array_no_allocto create V arrays from C arrays without copying.馃[deprecated] Generated by Copilot at 2142430