From 815439a381b9f6ae1e39d373793c2407e7c4f224 Mon Sep 17 00:00:00 2001 From: yuyi Date: Wed, 16 Aug 2023 01:37:51 +0800 Subject: [PATCH] checker: fix assigning an array slice (fix #19120) (#19137) --- vlib/builtin/string.v | 4 ++-- vlib/encoding/base32/base32.v | 4 ++-- vlib/flag/flag.v | 2 +- vlib/net/urllib/urllib.v | 2 +- vlib/regex/regex_util.v | 4 ++-- vlib/v/checker/assign.v | 22 +++++++++++++++++++++ vlib/v/checker/tests/slice_clone_notice.out | 15 ++++++++++++++ vlib/v/checker/tests/slice_clone_notice.vv | 12 +++++++++++ vlib/v/gen/js/tests/array.v | 8 ++++---- vlib/v/tests/array_slice_assign_test.v | 12 +++++++++++ vlib/v/tests/array_slice_test.v | 6 +++--- 11 files changed, 76 insertions(+), 15 deletions(-) create mode 100644 vlib/v/checker/tests/slice_clone_notice.out create mode 100644 vlib/v/checker/tests/slice_clone_notice.vv create mode 100644 vlib/v/tests/array_slice_assign_test.v diff --git a/vlib/builtin/string.v b/vlib/builtin/string.v index 02c67c478..cfc1534ef 100644 --- a/vlib/builtin/string.v +++ b/vlib/builtin/string.v @@ -2190,12 +2190,12 @@ pub fn (s string) trim_indent() string { // trim first line if it's blank if lines.len > 0 && lines.first().is_blank() { - lines = lines[1..] + lines = unsafe { lines[1..] } } // trim last line if it's blank if lines.len > 0 && lines.last().is_blank() { - lines = lines[..lines.len - 1] + lines = unsafe { lines[..lines.len - 1] } } mut trimmed_lines := []string{cap: lines.len} diff --git a/vlib/encoding/base32/base32.v b/vlib/encoding/base32/base32.v index 3d7e2c2ac..a33edb34f 100644 --- a/vlib/encoding/base32/base32.v +++ b/vlib/encoding/base32/base32.v @@ -212,8 +212,8 @@ fn (enc &Encoding) encode_(src_ []u8, mut dst []u8) { break } - src = src[5..] - dst = dst[8..] + src = unsafe { src[5..] } + dst = unsafe { dst[8..] } } } diff --git a/vlib/flag/flag.v b/vlib/flag/flag.v index dfcfd463f..2876b2e24 100644 --- a/vlib/flag/flag.v +++ b/vlib/flag/flag.v @@ -138,7 +138,7 @@ pub fn new_flag_parser(args []string) &FlagParser { if idx_dashdash >= 0 { all_before_dashdash.trim(idx_dashdash) if idx_dashdash < original_args.len { - all_after_dashdash = original_args[idx_dashdash + 1..] + all_after_dashdash = unsafe { original_args[idx_dashdash + 1..] } } } return &FlagParser{ diff --git a/vlib/net/urllib/urllib.v b/vlib/net/urllib/urllib.v index 536516fab..de2b990e4 100644 --- a/vlib/net/urllib/urllib.v +++ b/vlib/net/urllib/urllib.v @@ -889,7 +889,7 @@ fn resolve_path(base string, ref string) string { } '..' { if dst.len > 0 { - dst = dst[..dst.len - 1] + dst = unsafe { dst[..dst.len - 1] } } } else { diff --git a/vlib/regex/regex_util.v b/vlib/regex/regex_util.v index 68d7fd5fa..0ae444c48 100644 --- a/vlib/regex/regex_util.v +++ b/vlib/regex/regex_util.v @@ -526,9 +526,9 @@ pub fn (mut re RE) replace_n(in_txt string, repl_str string, count int) string { mut lst := re.find_all(in_txt) if count < 0 { // start from the right of the string - lst = lst#[count * 2..] // limitate the number of substitions + lst = unsafe { lst#[count * 2..] } // limitate the number of substitions } else if count > 0 { // start from the left of the string - lst = lst#[..count * 2] // limitate the number of substitions + lst = unsafe { lst#[..count * 2] } // limitate the number of substitions } else if count == 0 { // no replace return in_txt } diff --git a/vlib/v/checker/assign.v b/vlib/v/checker/assign.v index e3f251b57..c97020cc5 100644 --- a/vlib/v/checker/assign.v +++ b/vlib/v/checker/assign.v @@ -483,6 +483,28 @@ fn (mut c Checker) assign_stmt(mut node ast.AssignStmt) { } } } + } else if mut left is ast.Ident && right is ast.IndexExpr { + if (right as ast.IndexExpr).left is ast.Ident + && (right as ast.IndexExpr).index is ast.RangeExpr + && ((right as ast.IndexExpr).left.is_mut() || left.is_mut()) + && !c.inside_unsafe { + // `mut a := arr[..]` auto add clone() -> `mut a := arr[..].clone()` + c.add_error_detail_with_pos('To silence this notice, use either an explicit `a[..].clone()`, +or use an explicit `unsafe{ a[..] }`, if you do not want a copy of the slice.', + right.pos()) + c.note('an implicit clone of the slice was done here', right.pos()) + right = ast.CallExpr{ + name: 'clone' + left: right + left_type: left_type + is_method: true + receiver_type: left_type + return_type: left_type + scope: c.fn_scope + } + right_type = c.expr(mut right) + node.right[i] = right + } } } if node.op == .assign { diff --git a/vlib/v/checker/tests/slice_clone_notice.out b/vlib/v/checker/tests/slice_clone_notice.out new file mode 100644 index 000000000..fa122db85 --- /dev/null +++ b/vlib/v/checker/tests/slice_clone_notice.out @@ -0,0 +1,15 @@ +vlib/v/checker/tests/slice_clone_notice.vv:4:13: notice: an implicit clone of the slice was done here + 2 | xs := [1, 2, 3, 4, 5, 6, 7, 8] + 3 | + 4 | mut s := xs[1..] + | ~~~~~ + 5 | + 6 | s.sort(a > b) +Details: vlib/v/checker/tests/slice_clone_notice.vv:4:13: details: To silence this notice, use either an explicit `a[..].clone()`, +or use an explicit `unsafe{ a[..] }`, if you do not want a copy of the slice. + 2 | xs := [1, 2, 3, 4, 5, 6, 7, 8] + 3 | + 4 | mut s := xs[1..] + | ~~~~~ + 5 | + 6 | s.sort(a > b) diff --git a/vlib/v/checker/tests/slice_clone_notice.vv b/vlib/v/checker/tests/slice_clone_notice.vv new file mode 100644 index 000000000..8e4fbaa60 --- /dev/null +++ b/vlib/v/checker/tests/slice_clone_notice.vv @@ -0,0 +1,12 @@ +fn main() { + xs := [1, 2, 3, 4, 5, 6, 7, 8] + + mut s := xs[1..] + + s.sort(a > b) + + println(s) + assert s == [8, 7, 6, 5, 4, 3, 2] + println(xs) + assert xs == [1, 2, 3, 4, 5, 6, 7, 8] +} diff --git a/vlib/v/gen/js/tests/array.v b/vlib/v/gen/js/tests/array.v index ef63016ef..47cd41456 100644 --- a/vlib/v/gen/js/tests/array.v +++ b/vlib/v/gen/js/tests/array.v @@ -24,9 +24,9 @@ fn main() { mut arr2 := [1, 2, 3, 4, 5] // Array slices - slice1 := arr1[1..3] - slice2 := arr2[..3] - slice3 := arr2[3..] + slice1 := unsafe { arr1[1..3] } + slice2 := unsafe { arr2[..3] } + slice3 := unsafe { arr2[3..] } // Array indexes idx1 := slice1[1] @@ -45,7 +45,7 @@ fn main() { println('\n\n') // String slices - mut slice4 := idx1[..4] + mut slice4 := unsafe { idx1[..4] } print('Back\t=> ') println(slice4) // 'Back' diff --git a/vlib/v/tests/array_slice_assign_test.v b/vlib/v/tests/array_slice_assign_test.v new file mode 100644 index 000000000..512403c3e --- /dev/null +++ b/vlib/v/tests/array_slice_assign_test.v @@ -0,0 +1,12 @@ +fn test_array_slice_assign() { + xs := [1, 2, 3, 4, 5, 6, 7, 8] + + mut s := xs[1..].clone() + + s.sort(a > b) + + println(s) + assert s == [8, 7, 6, 5, 4, 3, 2] + println(xs) + assert xs == [1, 2, 3, 4, 5, 6, 7, 8] +} diff --git a/vlib/v/tests/array_slice_test.v b/vlib/v/tests/array_slice_test.v index 050d26933..a89a3087b 100644 --- a/vlib/v/tests/array_slice_test.v +++ b/vlib/v/tests/array_slice_test.v @@ -82,8 +82,8 @@ fn test_self_slice_push() { fn test_slice_push_child() { mut a := [1.0, 2.0625, 3.5, -7.75, 7.125, 8.4375, 0.5] - mut b := a[2..6] // `b` is initially created as reference - mut c := b[1..3] // `c` is initiall reference to `a` and `b` + mut b := unsafe { a[2..6] } // `b` is initially created as reference + mut c := unsafe { b[1..3] } // `c` is initiall reference to `a` and `b` b << -2.25 // `b` should be reallocated, so `a` doesn't change c[1] = -13.5 // this should change `c` and `a` but not `b` assert c == [-7.75, -13.5] @@ -93,7 +93,7 @@ fn test_slice_push_child() { fn test_predictable_reallocation_parent() { mut a := []i64{len: 4, cap: 6, init: -25} - mut b := a[1..3] + mut b := unsafe { a[1..3] } b[1] = -5238543910438573201 assert a == [i64(-25), -25, -5238543910438573201, -25] a << 5 -- 2.39.5