From c59d6403eec25d7e7f43292b73492ac957eb69f2 Mon Sep 17 00:00:00 2001 From: Felipe Pena Date: Wed, 1 Jan 2025 20:46:50 -0300 Subject: [PATCH 1/4] cgen: remove double string cloning (#23331) --- vlib/v/gen/c/array.v | 8 +++++--- vlib/v/gen/c/assert.v | 3 ++- vlib/v/gen/c/infix.v | 1 + 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/vlib/v/gen/c/array.v b/vlib/v/gen/c/array.v index 1c92d673b226e6..208a96af14ffb3 100644 --- a/vlib/v/gen/c/array.v +++ b/vlib/v/gen/c/array.v @@ -46,7 +46,7 @@ fn (mut g Gen) array_init(node ast.ArrayInit, var_name string) { } for i, expr in node.exprs { if node.expr_types[i] == ast.string_type - && expr !in [ast.StringLiteral, ast.StringInterLiteral] { + && expr !in [ast.IndexExpr, ast.CallExpr, ast.StringLiteral, ast.StringInterLiteral, ast.InfixExpr] { g.write('string_clone(') g.expr(expr) g.write(')') @@ -1035,12 +1035,14 @@ fn (mut g Gen) gen_array_insert(node ast.CallExpr) { g.expr(node.args[1].expr) g.write('.len)') } else { + needs_clone := left_info.elem_type == ast.string_type + && node.args[1].expr !in [ast.IndexExpr, ast.CallExpr, ast.StringLiteral, ast.StringInterLiteral, ast.InfixExpr] g.write(', &(${elem_type_str}[]){') - if left_info.elem_type == ast.string_type { + if needs_clone { g.write('string_clone(') } g.expr_with_cast(node.args[1].expr, node.args[1].typ, left_info.elem_type) - if left_info.elem_type == ast.string_type { + if needs_clone { g.write(')') } g.write('})') diff --git a/vlib/v/gen/c/assert.v b/vlib/v/gen/c/assert.v index 63f48feca2c24d..87a970e2b5d50e 100644 --- a/vlib/v/gen/c/assert.v +++ b/vlib/v/gen/c/assert.v @@ -229,7 +229,8 @@ fn (mut g Gen) gen_assert_single_expr(expr ast.Expr, typ ast.Type) { } else { mut should_clone := true - if typ == ast.string_type && expr is ast.StringLiteral { + if typ == ast.string_type + && expr in [ast.IndexExpr, ast.CallExpr, ast.StringLiteral, ast.StringInterLiteral] { should_clone = false } if expr is ast.CTempVar { diff --git a/vlib/v/gen/c/infix.v b/vlib/v/gen/c/infix.v index 784f9140581417..bb446e4ddea887 100644 --- a/vlib/v/gen/c/infix.v +++ b/vlib/v/gen/c/infix.v @@ -1054,6 +1054,7 @@ fn (mut g Gen) infix_expr_left_shift_op(node ast.InfixExpr) { needs_clone := !g.is_builtin_mod && array_info.elem_type.idx() == ast.string_type_idx && array_info.elem_type.nr_muls() == 0 + && node.right !in [ast.StringLiteral, ast.StringInterLiteral, ast.CallExpr, ast.IndexExpr, ast.InfixExpr] if needs_clone { g.write('string_clone(') } From 87c0a9ce2812fafd603b92dd96406d77dc1ac189 Mon Sep 17 00:00:00 2001 From: Felipe Pena Date: Wed, 1 Jan 2025 20:48:50 -0300 Subject: [PATCH 2/4] checker: fix comptime indexexpr resolving (#23333) --- vlib/v/checker/fn.v | 8 ++++ vlib/v/gen/c/fn.v | 8 ++++ vlib/x/json2/decoder.v | 4 +- vlib/x/json2/json2.v | 18 +++++++++ vlib/x/json2/tests/decode_nested_array_test.v | 39 +++++++++++++++++++ 5 files changed, 75 insertions(+), 2 deletions(-) create mode 100644 vlib/x/json2/tests/decode_nested_array_test.v diff --git a/vlib/v/checker/fn.v b/vlib/v/checker/fn.v index bf2287c0354a1b..d2a2d47762c857 100644 --- a/vlib/v/checker/fn.v +++ b/vlib/v/checker/fn.v @@ -2043,6 +2043,14 @@ fn (mut c Checker) resolve_comptime_args(func &ast.Fn, node_ ast.CallExpr, concr } } else if call_arg.expr is ast.ComptimeCall { comptime_args[k] = c.type_resolver.get_type(call_arg.expr) + } else if call_arg.expr is ast.IndexExpr && c.comptime.is_comptime(call_arg.expr) { + mut ctyp := c.type_resolver.get_type(call_arg.expr) + param_typ_sym := c.table.sym(param_typ) + cparam_type_sym := c.table.sym(c.unwrap_generic(ctyp)) + if param_typ_sym.kind == .array && cparam_type_sym.info is ast.Array { + ctyp = cparam_type_sym.info.elem_type + } + comptime_args[k] = ctyp } } } diff --git a/vlib/v/gen/c/fn.v b/vlib/v/gen/c/fn.v index 9342347b2c43e8..19716b4d46813c 100644 --- a/vlib/v/gen/c/fn.v +++ b/vlib/v/gen/c/fn.v @@ -1659,6 +1659,14 @@ fn (mut g Gen) resolve_comptime_args(func &ast.Fn, mut node_ ast.CallExpr, concr comptime_args[k] = cparam_type_sym.info.key_type comptime_args[k + 1] = cparam_type_sym.info.value_type } + } else if mut call_arg.expr is ast.IndexExpr && g.comptime.is_comptime(call_arg.expr) { + mut ctyp := g.type_resolver.get_type(call_arg.expr) + param_typ_sym := g.table.sym(param_typ) + cparam_type_sym := g.table.sym(g.unwrap_generic(ctyp)) + if param_typ_sym.kind == .array && cparam_type_sym.info is ast.Array { + ctyp = cparam_type_sym.info.elem_type + } + comptime_args[k] = ctyp } } } diff --git a/vlib/x/json2/decoder.v b/vlib/x/json2/decoder.v index 5ffa6db122783a..431b366d3524a1 100644 --- a/vlib/x/json2/decoder.v +++ b/vlib/x/json2/decoder.v @@ -291,8 +291,8 @@ fn decode_array_item[T](mut field T, arr []Any) { $else $if T is [][]?u64 { field << arr.map(it.as_map().values().map(?u64(it.u64()))) } $else $if T is [][]bool { field << arr.map(it.as_map().values().map(it.bool())) } $else $if T is [][]?bool { field << arr.map(it.as_map().values().map(?bool(it.bool()))) } - $else $if T is [][]string { field << arr.map(it.as_map().values().map(it.string())) } - $else $if T is [][]?string { field << arr.map(it.as_map().values().map(?string(it.string()))) } + $else $if T is [][]string { field << arr.map(it.as_map().values().map(it.str())) } + $else $if T is [][]?string { field << arr.map(it.as_map().values().map(?string(it.str()))) } } } // vfmt on diff --git a/vlib/x/json2/json2.v b/vlib/x/json2/json2.v index b135639b38ab22..9cf49d935b4195 100644 --- a/vlib/x/json2/json2.v +++ b/vlib/x/json2/json2.v @@ -95,6 +95,24 @@ pub fn (f Any) i64() i64 { } } +// u8 uses `Any` as a 8-bit unsigned integer. +pub fn (f Any) u8() u8 { + match f { + u8 { + return f + } + u16, u32, i8, i16, i32, int, i64, f32, f64, bool { + return u8(u16(f)) + } + string { + return f.u8() + } + else { + return 0 + } + } +} + // u64 uses `Any` as a 64-bit unsigned integer. pub fn (f Any) u64() u64 { match f { diff --git a/vlib/x/json2/tests/decode_nested_array_test.v b/vlib/x/json2/tests/decode_nested_array_test.v new file mode 100644 index 00000000000000..68809ccc0fadcc --- /dev/null +++ b/vlib/x/json2/tests/decode_nested_array_test.v @@ -0,0 +1,39 @@ +import x.json2 + +struct Dataset { + values [][]string +} + +struct Dataset2 { + values [][]u8 +} + +fn test_string() { + d := Dataset{ + values: [ + ['a', 'b', 'c'], + ['d', 'e', 'f'], + ] + } + s := json2.encode(d) + println(s) + d2 := json2.decode[Dataset](s)! + assert d2.str() == "Dataset{ + values: [['a', 'b', 'c'], ['d', 'e', 'f']] +}" +} + +fn test_u8() { + d := Dataset2{ + values: [ + [u8(1), 2, 3, 4, 5], + [u8(2), 3, 4, 5, 6], + ] + } + s := json2.encode(d) + println(s) + d2 := json2.decode[Dataset2](s)! + assert d2.str() == 'Dataset2{ + values: [[1, 2, 3, 4, 5], [2, 3, 4, 5, 6]] +}' +} From a3665820109cbaa92111a6c35704397e4bce3004 Mon Sep 17 00:00:00 2001 From: Felipe Pena Date: Wed, 1 Jan 2025 20:50:38 -0300 Subject: [PATCH 3/4] cgen: fix type_default for array init >= 8 items (spotted while building the vhamll project) (#23334) --- vlib/v/gen/c/cgen.v | 2 +- .../tests/structs/default_expr_array_init_test.v | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 vlib/v/tests/structs/default_expr_array_init_test.v diff --git a/vlib/v/gen/c/cgen.v b/vlib/v/gen/c/cgen.v index e23aedfb112f7c..61da187473bd05 100644 --- a/vlib/v/gen/c/cgen.v +++ b/vlib/v/gen/c/cgen.v @@ -7043,7 +7043,7 @@ fn (mut g Gen) type_default_impl(typ_ ast.Type, decode_sumtype bool) string { } } else { default_str := g.expr_string_opt(field.typ, field.default_expr) - if default_str.count('\n') > 1 { + if default_str.count(';\n') > 1 { g.type_default_vars.writeln(default_str.all_before_last('\n')) expr_str = default_str.all_after_last('\n') } else { diff --git a/vlib/v/tests/structs/default_expr_array_init_test.v b/vlib/v/tests/structs/default_expr_array_init_test.v new file mode 100644 index 00000000000000..8b9a738c7dc229 --- /dev/null +++ b/vlib/v/tests/structs/default_expr_array_init_test.v @@ -0,0 +1,15 @@ +struct Foo { +pub mut: + integer_range_for_discrete []int = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9] +} + +fn foo(a string) Foo { + return match a { + 'a' { Foo{} } + else { panic('foo') } + } +} + +fn test_main() { + assert foo('a') == Foo{} +} From 7cf77fbda0e771dcb9dbfc3e013ec796c121faf9 Mon Sep 17 00:00:00 2001 From: Bruno-Vdr Date: Thu, 2 Jan 2025 00:53:34 +0100 Subject: [PATCH 4/4] tools: allow for passing arbitrary compiler flags to `v bug`, for example: `v bug -cg -autofree file.v` (#23335) --- cmd/tools/vbug.v | 132 ++++++++++++++++++++++++++++------------------- 1 file changed, 80 insertions(+), 52 deletions(-) diff --git a/cmd/tools/vbug.v b/cmd/tools/vbug.v index b9a234219bfa71..0b4bbe50be62ee 100644 --- a/cmd/tools/vbug.v +++ b/cmd/tools/vbug.v @@ -1,8 +1,15 @@ -import net.urllib import os +import term import readline +import net.urllib -const vroot = @VMODROOT +fn elog(msg string) { + eprintln(term.ecolorize(term.gray, msg)) +} + +fn olog(msg string) { + println(term.colorize(term.green, msg)) +} // get output from `v doctor` fn get_vdoctor_output(is_verbose bool) string { @@ -10,55 +17,64 @@ fn get_vdoctor_output(is_verbose bool) string { verbose_flag := if is_verbose { '-v' } else { '' } result := os.execute('${os.quoted_path(vexe)} ${verbose_flag} doctor') if result.exit_code != 0 { - eprintln('unable to get `v doctor` output: ${result.output}') + elog('> unable to get `v doctor` output: ${result.output}') return '' } return result.output } -// get output from `./v -g -o vdbg cmd/v && ./vdbg file.v` -fn get_v_build_output(is_verbose bool, is_yes bool, file_path string) string { +fn runv(label string, user_cmd string) os.Result { + mut result := os.Result{} + elog('> ${label} using: ${term.ecolorize(term.magenta, user_cmd)}') + result = os.execute(user_cmd) + print(result.output) + return result +} + +// get output from `./v -g -o vdbg cmd/v && ./vdbg -user_args run file.v` +fn get_v_build_output(is_verbose bool, is_yes bool, file_path string, user_args string, generated_file string) string { + mut result := os.Result{} mut vexe := os.getenv('VEXE') + // prepare a V compiler with -g to have better backtraces if possible wd := os.getwd() + vroot := @VMODROOT os.chdir(vroot) or {} verbose_flag := if is_verbose { '-v' } else { '' } vdbg_path := $if windows { '${vroot}/vdbg.exe' } $else { '${vroot}/vdbg' } vdbg_compilation_cmd := '${os.quoted_path(vexe)} ${verbose_flag} -g -o ${os.quoted_path(vdbg_path)} cmd/v' - vdbg_result := os.execute(vdbg_compilation_cmd) + result = runv('Prepare vdbg', vdbg_compilation_cmd) os.chdir(wd) or {} - if vdbg_result.exit_code == 0 { + + if result.exit_code == 0 { vexe = vdbg_path } else { - eprintln('unable to compile V in debug mode: ${vdbg_result.output}\ncommand: ${vdbg_compilation_cmd}\n') + elog('> unable to compile V in debug mode: ${result.output}\ncommand: ${vdbg_compilation_cmd}\n') } - mut result := os.execute('${os.quoted_path(vexe)} ${verbose_flag} ${os.quoted_path(file_path)}') + result = runv('Compile', '${os.quoted_path(vexe)} ${verbose_flag} ${user_args} ${os.quoted_path(file_path)}') defer { os.rm(vdbg_path) or { if is_verbose { - eprintln('unable to delete `vdbg`: ${err}') + elog('> unable to delete `vdbg`: ${err}') } } } if result.exit_code == 0 { defer { - mut generated_file := file_path.all_before_last('.') - $if windows { - generated_file += '.exe' - } os.rm(generated_file) or { if is_verbose { - eprintln('unable to delete generated file: ${err}') + elog('> unable to delete generated file: ${err}') } } } run := is_yes || ask('It looks like the compilation went well, do you want to run the file?') if run { - result = os.execute('${os.quoted_path(vexe)} ${verbose_flag} run ${os.quoted_path(file_path)}') + result = runv('Run', generated_file) if result.exit_code == 0 && !is_yes { - confirm_or_exit('It looks like the file ran correctly as well, are you sure you want to continue?') + elog('> The file ran correctly as well.') + confirm_or_exit('Are you sure you want to continue?') } } } @@ -66,7 +82,7 @@ fn get_v_build_output(is_verbose bool, is_yes bool, file_path string) string { } fn ask(msg string) bool { - prompt := os.input_opt('${msg} [Y/n] ') or { 'y' } + prompt := os.input_opt(term.colorize(term.bright_white, '${msg} [Y/n] ')) or { 'y' } return prompt == '' || prompt[0].ascii_str().to_lower() != 'n' } @@ -77,59 +93,68 @@ fn confirm_or_exit(msg string) { } fn main() { + unbuffer_stdout() + mut compiler_args := []string{} mut file_path := '' - mut is_verbose := false - mut is_yes := false - for arg in os.args[2..] { - match arg { - '-v' { - is_verbose = true - } - '-y' { - is_yes = true + is_verbose := '-v' in os.args + is_yes := '-y' in os.args + + for arg in os.args[1..] { + if arg == 'bug' { + continue + } + if arg.ends_with('.v') || arg.ends_with('.vsh') || arg.ends_with('.vv') { + if file_path != '' { + elog('> v bug: only one V file can be submitted') + exit(1) } - else { - if !arg.ends_with('.v') && !arg.ends_with('.vsh') && !arg.ends_with('.vv') { - eprintln('unknown argument: `${arg}`') - exit(1) - } - if file_path != '' { - eprintln('only one V file can be submitted') - exit(1) - } - file_path = arg + file_path = arg + } else { + if arg !in ['-y', '-v'] { + compiler_args << arg } } } + if file_path == '' { - eprintln('v bug: no v file listed to report') + elog('> v bug: no v file listed to report') exit(1) } + os.unsetenv('VCOLORS') // collect error information // output from `v doctor` vdoctor_output := get_vdoctor_output(is_verbose) // file content file_content := os.read_file(file_path) or { - eprintln('unable to get file "${file_path}" content: ${err}') + elog('> unable to get file "${file_path}" content: ${err}') '' } - // output from `./v -g -o vdbg cmd/v && ./vdbg file.v` - build_output := get_v_build_output(is_verbose, is_yes, file_path) + + user_args := compiler_args.join(' ') + mut generated_file := file_path.all_before_last('.') + if os.user_os() == 'windows' { + generated_file += '.exe' + } + build_output := get_v_build_output(is_verbose, is_yes, file_path, user_args, generated_file) + // ask the user if he wants to submit even after an error if !is_yes && (vdoctor_output == '' || file_content == '' || build_output == '') { - confirm_or_exit('An error occurred retrieving the information, do you want to continue?') + elog('> Error while retrieving the information.') + confirm_or_exit('Do you want to continue?') } expected_result := readline.read_line('What did you expect to see? ') or { // Ctrl-C was pressed - eprintln('\nCanceled') + elog('\nCanceled') exit(1) } // open prefilled issue creation page, or print link as a fallback if !is_yes && vdoctor_output.contains('behind V master') { - confirm_or_exit('It looks like your installation of V is outdated, we advise you to run `v up` before submitting an issue. Are you sure you want to continue?') + olog('> It looks like your installation of V is outdated.') + olog('> We advise you to run `v up` before submitting an issue.') + confirm_or_exit('Are you sure you want to continue?') } // When updating this template, make sure to update `.github/ISSUE_TEMPLATE/bug_report.md` too @@ -140,30 +165,33 @@ ${vdoctor_output} ``` **What did you do?** -`./v -g -o vdbg cmd/v && ./vdbg ${file_path}` +`./v -g -o vdbg cmd/v && ./vdbg ${user_args} ${file_path} && ${generated_file}` {file_content} +**What did you see instead?** +``` +${build_output}``` + **What did you expect to see?** ${expected_result} -**What did you see instead?** -``` -${build_output}```' +' mut encoded_body := urllib.query_escape(raw_body.replace_once('{file_content}', '```v\n${file_content}\n```')) mut generated_uri := 'https://github.com/vlang/v/issues/new?labels=Bug&body=${encoded_body}' if generated_uri.len > 8192 { // GitHub doesn't support URLs longer than 8192 characters encoded_body = urllib.query_escape(raw_body.replace_once('{file_content}', 'See attached file `${file_path}`')) generated_uri = 'https://github.com/vlang/v/issues/new?labels=Bug&body=${encoded_body}' - println('Your file is too big to be submitted. Head over to the following URL and attach your file.') - println(generated_uri) + elog('> Your file is too big to be submitted.') + elog('> Go to the following URL, and attach your file:') + olog(generated_uri) } else { os.open_uri(generated_uri) or { if is_verbose { - eprintln(err) + elog(err.str()) } - println(generated_uri) + olog(generated_uri) } } }