From 7a25c03aa79a875c7c00cc3eecb8d9db432b176f Mon Sep 17 00:00:00 2001
From: Felipe Pena <felipensp@gmail.com>
Date: Fri, 9 Jun 2023 08:34:32 -0300
Subject: [PATCH] cgen: simplify fixed arr return (#18380)

---
 vlib/v/checker/checker.v          |  8 +++--
 vlib/v/gen/c/assign.v             | 10 ------
 vlib/v/gen/c/auto_str_methods.v   | 26 ++++++++--------
 vlib/v/gen/c/cgen.v               | 12 +++----
 vlib/v/gen/c/dumpexpr.v           | 13 ++++++--
 vlib/v/gen/c/fn.v                 | 14 +++++++++
 vlib/v/gen/c/index.v              |  4 ---
 vlib/v/gen/c/infix.v              |  4 +++
 vlib/v/gen/c/str.v                |  2 ++
 vlib/v/tests/fixed_array_2_test.v | 52 +++++++++++++++++++++++++++++++
 10 files changed, 107 insertions(+), 38 deletions(-)
 create mode 100644 vlib/v/tests/fixed_array_2_test.v

diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v
index 20d5020342..029da8f25f 100644
--- a/vlib/v/checker/checker.v
+++ b/vlib/v/checker/checker.v
@@ -2538,9 +2538,11 @@ pub fn (mut c Checker) expr(node_ ast.Expr) ast.Type {
 			tsym := c.table.sym(unwrapped_expr_type)
 			if tsym.kind == .array_fixed {
 				info := tsym.info as ast.ArrayFixed
-				// for dumping fixed array we must registed the fixed array struct to return from function
-				c.table.find_or_register_array_fixed(info.elem_type, info.size, info.size_expr,
-					true)
+				if !info.is_fn_ret {
+					// for dumping fixed array we must register the fixed array struct to return from function
+					c.table.find_or_register_array_fixed(info.elem_type, info.size, info.size_expr,
+						true)
+				}
 			}
 			type_cname := if node.expr_type.has_flag(.option) {
 				'_option_${tsym.cname}'
diff --git a/vlib/v/gen/c/assign.v b/vlib/v/gen/c/assign.v
index 06a2e8cb3b..1249bc2d1d 100644
--- a/vlib/v/gen/c/assign.v
+++ b/vlib/v/gen/c/assign.v
@@ -636,13 +636,6 @@ fn (mut g Gen) assign_stmt(node_ ast.AssignStmt) {
 				} else if is_fixed_array_var {
 					// TODO Instead of the translated check, check if it's a pointer already
 					// and don't generate memcpy &
-					right_is_fixed_ret := !(val is ast.CallExpr
-						&& (val as ast.CallExpr).or_block.kind == .propagate_option)
-						&& ((right_sym.info is ast.ArrayFixed
-						&& (right_sym.info as ast.ArrayFixed).is_fn_ret)
-						|| (val is ast.DumpExpr && right_sym.info is ast.ArrayFixed)
-						|| (val is ast.CallExpr
-						&& g.table.sym(g.unwrap_generic((val as ast.CallExpr).return_type)).kind == .array_fixed))
 					typ_str := g.typ(val_type).trim('*')
 					final_typ_str := if is_fixed_array_var { '' } else { '(${typ_str}*)' }
 					final_ref_str := if is_fixed_array_var {
@@ -661,9 +654,6 @@ fn (mut g Gen) assign_stmt(node_ ast.AssignStmt) {
 						g.expr(left)
 						g.write(', ${final_ref_str}')
 						g.expr(val)
-						if right_is_fixed_ret {
-							g.write('.ret_arr')
-						}
 						g.write(', sizeof(${typ_str})) /*assign*/')
 					}
 				} else if is_decl {
diff --git a/vlib/v/gen/c/auto_str_methods.v b/vlib/v/gen/c/auto_str_methods.v
index fabf8f04eb..d102a91427 100644
--- a/vlib/v/gen/c/auto_str_methods.v
+++ b/vlib/v/gen/c/auto_str_methods.v
@@ -649,15 +649,15 @@ fn (mut g Gen) gen_str_for_array_fixed(info ast.ArrayFixed, styp string, str_fn_
 	is_elem_ptr := typ.is_ptr()
 	sym_has_str_method, str_method_expects_ptr, _ := sym.str_method_info()
 	elem_str_fn_name := g.get_str_fn(typ)
+	def_arg := if info.is_fn_ret { '${g.typ(typ)} a[${info.size}]' } else { '${styp} a' }
 
-	g.definitions.writeln('static string ${str_fn_name}(${styp} a); // auto')
-	g.auto_str_funcs.writeln('static string ${str_fn_name}(${styp} a) { return indent_${str_fn_name}(a, 0);}')
-	g.definitions.writeln('static string indent_${str_fn_name}(${styp} a, int indent_count); // auto')
-	g.auto_str_funcs.writeln('static string indent_${str_fn_name}(${styp} a, int indent_count) {')
+	g.definitions.writeln('static string ${str_fn_name}(); // auto')
+	g.auto_str_funcs.writeln('static string ${str_fn_name}(${def_arg}) { return indent_${str_fn_name}(a, 0);}')
+	g.definitions.writeln('static string indent_${str_fn_name}(${def_arg}, int indent_count); // auto')
+	g.auto_str_funcs.writeln('static string indent_${str_fn_name}(${def_arg}, int indent_count) {')
 	g.auto_str_funcs.writeln('\tstrings__Builder sb = strings__new_builder(${info.size} * 10);')
 	g.auto_str_funcs.writeln('\tstrings__Builder_write_string(&sb, _SLIT("["));')
 	g.auto_str_funcs.writeln('\tfor (int i = 0; i < ${info.size}; ++i) {')
-	suffix := if info.is_fn_ret { '.ret_arr' } else { '' }
 	if sym.kind == .function {
 		g.auto_str_funcs.writeln('\t\tstring x = ${elem_str_fn_name}();')
 		g.auto_str_funcs.writeln('\t\tstrings__Builder_write_string(&sb, x);')
@@ -666,30 +666,30 @@ fn (mut g Gen) gen_str_for_array_fixed(info ast.ArrayFixed, styp string, str_fn_
 		if should_use_indent_func(sym.kind) && !sym_has_str_method {
 			if is_elem_ptr {
 				g.auto_str_funcs.writeln('\t\tstrings__Builder_write_string(&sb, _SLIT("${deref_label}"));')
-				g.auto_str_funcs.writeln('\t\tif ( 0 == a${suffix}[i] ) {')
+				g.auto_str_funcs.writeln('\t\tif ( 0 == a[i] ) {')
 				g.auto_str_funcs.writeln('\t\t\tstrings__Builder_write_string(&sb, _SLIT("0"));')
 				g.auto_str_funcs.writeln('\t\t}else{')
-				g.auto_str_funcs.writeln('\t\t\tstrings__Builder_write_string(&sb, ${elem_str_fn_name}( ${deref} a${suffix}[i]) );')
+				g.auto_str_funcs.writeln('\t\t\tstrings__Builder_write_string(&sb, ${elem_str_fn_name}( ${deref} a[i]) );')
 				g.auto_str_funcs.writeln('\t\t}')
 			} else {
-				g.auto_str_funcs.writeln('\t\tstrings__Builder_write_string(&sb, ${elem_str_fn_name}( ${deref} a${suffix}[i]) );')
+				g.auto_str_funcs.writeln('\t\tstrings__Builder_write_string(&sb, ${elem_str_fn_name}( ${deref} a[i]) );')
 			}
 		} else if sym.kind in [.f32, .f64] {
 			if sym.kind == .f32 {
-				field_str := str_intp_g32('a${suffix}[i]')
+				field_str := str_intp_g32('a[i]')
 				g.auto_str_funcs.writeln('\t\tstrings__Builder_write_string(&sb, ${field_str} );')
 			} else {
-				field_str := str_intp_g64('a${suffix}[i]')
+				field_str := str_intp_g64('a[i]')
 				g.auto_str_funcs.writeln('\t\tstrings__Builder_write_string(&sb, ${field_str} );')
 			}
 		} else if sym.kind == .string {
-			field_str := str_intp_sq('a${suffix}[i]')
+			field_str := str_intp_sq('a[i]')
 			g.auto_str_funcs.writeln('\t\tstrings__Builder_write_string(&sb, ${field_str});')
 		} else if sym.kind == .rune {
-			tmp_str := str_intp_rune('${elem_str_fn_name}(  ${deref} a${suffix}[i])')
+			tmp_str := str_intp_rune('${elem_str_fn_name}(  ${deref} a[i])')
 			g.auto_str_funcs.writeln('\t\tstrings__Builder_write_string(&sb, ${tmp_str});')
 		} else {
-			g.auto_str_funcs.writeln('\t\tstrings__Builder_write_string(&sb, ${elem_str_fn_name}( ${deref} a${suffix}[i]));')
+			g.auto_str_funcs.writeln('\t\tstrings__Builder_write_string(&sb, ${elem_str_fn_name}( ${deref} a[i]));')
 		}
 	}
 	g.auto_str_funcs.writeln('\t\tif (i < ${info.size - 1}) {')
diff --git a/vlib/v/gen/c/cgen.v b/vlib/v/gen/c/cgen.v
index ec28db81a3..80ac39d6f8 100644
--- a/vlib/v/gen/c/cgen.v
+++ b/vlib/v/gen/c/cgen.v
@@ -138,6 +138,7 @@ mut:
 	inside_or_block           bool
 	inside_call               bool
 	inside_curry_call         bool // inside foo()()!, foo()()?, foo()()
+	expected_fixed_arr        bool
 	inside_for_c_stmt         bool
 	inside_comptime_for_field bool
 	inside_cast_in_heap       int // inside cast to interface type in heap (resolve recursive calls)
@@ -2452,11 +2453,6 @@ fn (mut g Gen) expr_with_cast(expr ast.Expr, got_type_raw ast.Type, expected_typ
 	}
 	// no cast
 	g.expr(expr)
-	if expr is ast.CallExpr && !(expr as ast.CallExpr).is_fn_var && !expected_type.has_flag(.option)
-		&& exp_sym.kind == .array_fixed {
-		// it's non-option fixed array, requires accessing .ret_arr member to get the array
-		g.write('.ret_arr')
-	}
 }
 
 fn write_octal_escape(mut b strings.Builder, c u8) {
@@ -4989,12 +4985,16 @@ fn (mut g Gen) return_stmt(node ast.Return) {
 					g.writeln('{0};')
 					if node.exprs[0] is ast.Ident {
 						g.write('memcpy(${tmpvar}.ret_arr, ${g.expr_string(node.exprs[0])}, sizeof(${g.typ(node.types[0])})) /*ret*/')
-					} else {
+					} else if node.exprs[0] is ast.ArrayInit {
 						tmpvar2 := g.new_tmp_var()
 						g.write('${g.typ(node.types[0])} ${tmpvar2} = ')
 						g.expr_with_cast(node.exprs[0], node.types[0], g.fn_decl.return_type)
 						g.writeln(';')
 						g.write('memcpy(${tmpvar}.ret_arr, ${tmpvar2}, sizeof(${g.typ(node.types[0])})) /*ret*/')
+					} else {
+						g.write('memcpy(${tmpvar}.ret_arr, ')
+						g.expr_with_cast(node.exprs[0], node.types[0], g.fn_decl.return_type)
+						g.write(', sizeof(${g.typ(node.types[0])})) /*ret*/')
 					}
 				} else {
 					g.expr_with_cast(node.exprs[0], node.types[0], g.fn_decl.return_type)
diff --git a/vlib/v/gen/c/dumpexpr.v b/vlib/v/gen/c/dumpexpr.v
index b44817a927..61a8071143 100644
--- a/vlib/v/gen/c/dumpexpr.v
+++ b/vlib/v/gen/c/dumpexpr.v
@@ -67,6 +67,10 @@ fn (mut g Gen) dump_expr(node ast.DumpExpr) {
 		g.inside_opt_or_res = old_inside_opt_or_res
 	}
 	g.write(')')
+	if (g.inside_assign || g.expected_fixed_arr) && !expr_type.has_flag(.option)
+		&& g.table.type_kind(expr_type) == .array_fixed {
+		g.write('.ret_arr')
+	}
 }
 
 fn (mut g Gen) dump_expr_definitions() {
@@ -106,8 +110,13 @@ fn (mut g Gen) dump_expr_definitions() {
 			dump_typedefs['typedef ${str_tdef};'] = true
 			str_dumparg_ret_type = str_dumparg_type
 		} else if !typ.has_flag(.option) && dump_sym.kind == .array_fixed {
-			// fixed array returned from function
-			str_dumparg_ret_type = '_v_' + str_dumparg_type
+			if (dump_sym.info as ast.ArrayFixed).is_fn_ret {
+				str_dumparg_ret_type = str_dumparg_type
+				str_dumparg_type = str_dumparg_type.trim_string_left('_v_')
+			} else {
+				// fixed array returned from function
+				str_dumparg_ret_type = '_v_' + str_dumparg_type
+			}
 		} else {
 			str_dumparg_ret_type = str_dumparg_type
 		}
diff --git a/vlib/v/gen/c/fn.v b/vlib/v/gen/c/fn.v
index 1fca275b26..0a74d55d9c 100644
--- a/vlib/v/gen/c/fn.v
+++ b/vlib/v/gen/c/fn.v
@@ -1474,6 +1474,11 @@ fn (mut g Gen) method_call(node ast.CallExpr) {
 		g.write(', ${array_depth}')
 	}
 	g.write(')')
+	if node.return_type != 0 && !node.return_type.has_flag(.option)
+		&& g.table.sym(node.return_type).kind == .array_fixed {
+		// it's non-option fixed array, requires accessing .ret_arr member to get the array
+		g.write('.ret_arr')
+	}
 }
 
 fn (mut g Gen) fn_call(node ast.CallExpr) {
@@ -1776,6 +1781,11 @@ fn (mut g Gen) fn_call(node ast.CallExpr) {
 			if name != '&' {
 				g.write(')')
 			}
+			if node.return_type != 0 && !node.return_type.has_flag(.option)
+				&& g.table.sym(node.return_type).kind == .array_fixed {
+				// it's non-option fixed array, requires accessing .ret_arr member to get the array
+				g.write('.ret_arr')
+			}
 			if tmp_cnt_save >= 0 {
 				g.writeln(';')
 				g.keep_alive_call_postgen(node, tmp_cnt_save)
@@ -1916,6 +1926,10 @@ fn (mut g Gen) autofree_call_postgen(node_pos int) {
 }
 
 fn (mut g Gen) call_args(node ast.CallExpr) {
+	g.expected_fixed_arr = true
+	defer {
+		g.expected_fixed_arr = false
+	}
 	args := if g.is_js_call {
 		if node.args.len < 1 {
 			g.error('node should have at least 1 arg', node.pos)
diff --git a/vlib/v/gen/c/index.v b/vlib/v/gen/c/index.v
index cfee3dc7e2..63741bcf29 100644
--- a/vlib/v/gen/c/index.v
+++ b/vlib/v/gen/c/index.v
@@ -368,10 +368,6 @@ fn (mut g Gen) index_of_fixed_array(node ast.IndexExpr, sym ast.TypeSymbol) {
 		} else {
 			g.expr(node.left)
 		}
-		if node.left is ast.CallExpr && sym.kind == .array_fixed
-			&& (sym.info as ast.ArrayFixed).is_fn_ret {
-			g.write('.ret_arr')
-		}
 	}
 	g.write('[')
 	if g.is_direct_array_access || g.pref.translated || node.index is ast.IntegerLiteral {
diff --git a/vlib/v/gen/c/infix.v b/vlib/v/gen/c/infix.v
index 8d8a156c0e..7cc8a50f76 100644
--- a/vlib/v/gen/c/infix.v
+++ b/vlib/v/gen/c/infix.v
@@ -8,6 +8,10 @@ import v.token
 import v.util
 
 fn (mut g Gen) infix_expr(node ast.InfixExpr) {
+	g.expected_fixed_arr = true
+	defer {
+		g.expected_fixed_arr = false
+	}
 	if node.auto_locked != '' {
 		g.writeln('sync__RwMutex_lock(&${node.auto_locked}->mtx);')
 	}
diff --git a/vlib/v/gen/c/str.v b/vlib/v/gen/c/str.v
index e95ff32ec7..47193fed5f 100644
--- a/vlib/v/gen/c/str.v
+++ b/vlib/v/gen/c/str.v
@@ -54,8 +54,10 @@ fn (mut g Gen) string_inter_literal_sb_optimized(call_expr ast.CallExpr) {
 fn (mut g Gen) gen_expr_to_string(expr ast.Expr, etype ast.Type) {
 	old_inside_opt_or_res := g.inside_opt_or_res
 	g.inside_opt_or_res = true
+	g.expected_fixed_arr = true
 	defer {
 		g.inside_opt_or_res = old_inside_opt_or_res
+		g.expected_fixed_arr = false
 	}
 	is_shared := etype.has_flag(.shared_f)
 	mut typ := etype
diff --git a/vlib/v/tests/fixed_array_2_test.v b/vlib/v/tests/fixed_array_2_test.v
new file mode 100644
index 0000000000..3e4c2d5f14
--- /dev/null
+++ b/vlib/v/tests/fixed_array_2_test.v
@@ -0,0 +1,52 @@
+struct Test {}
+
+fn (t Test) conversion_error(value u16) [4]u8 {
+	return conversion_error(value)
+}
+
+fn arr_opt() ?[2]int {
+	return [1, 2]!
+}
+
+fn conversion_error(value u16) [4]u8 {
+	mut return_value := [4]u8{}
+	for i := 0; i < 4; i++ {
+		return_value[i] = u8(value)
+	}
+	return return_value
+}
+
+fn test_assign() {
+	// -- Block below works fine
+	value := conversion_error(42)
+	println(value)
+}
+
+fn test_assign_method() {
+	value2 := Test{}.conversion_error(42)
+	println(value2)
+}
+
+fn test_ret() {
+	// -- Block above works fine
+	println(conversion_error(42))
+	println(Test{}.conversion_error(42))
+}
+
+fn test_assign_dump() {
+	y := dump(conversion_error(42))
+	dump(y)
+	y2 := dump(Test{}.conversion_error(42))
+	dump(y2)
+}
+
+fn test_assert() {
+	a := [1, 2]!
+	assert dump(a) == [1, 2]!
+}
+
+fn test_call() {
+	conversion_error(42)
+	Test{}.conversion_error(42)
+	dump(arr_opt())
+}