Skip to content

Commit

Permalink
checker: fix assigning an array slice (fix #19120) (#19137)
Browse files Browse the repository at this point in the history
  • Loading branch information
yuyi98 authored Aug 15, 2023
1 parent 717076b commit 815439a
Show file tree
Hide file tree
Showing 11 changed files with 76 additions and 15 deletions.
4 changes: 2 additions & 2 deletions vlib/builtin/string.v
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
4 changes: 2 additions & 2 deletions vlib/encoding/base32/base32.v
Original file line number Diff line number Diff line change
Expand Up @@ -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..] }
}
}

Expand Down
2 changes: 1 addition & 1 deletion vlib/flag/flag.v
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
2 changes: 1 addition & 1 deletion vlib/net/urllib/urllib.v
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions vlib/regex/regex_util.v
Original file line number Diff line number Diff line change
Expand Up @@ -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

This comment has been minimized.

Copy link
@penguindark

penguindark Aug 15, 2023

Member

Why unsafe here?
# is safe for definiton :/

This comment has been minimized.

Copy link
@medvednikov

medvednikov Aug 15, 2023

Member

it's not related to #
it's about slice views
unsafe memory
no cloning

This comment has been minimized.

Copy link
@penguindark

penguindark Aug 15, 2023

Member

Ok, thanks for the explanation 👍

} 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
}
Expand Down
22 changes: 22 additions & 0 deletions vlib/v/checker/assign.v
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
15 changes: 15 additions & 0 deletions vlib/v/checker/tests/slice_clone_notice.out
Original file line number Diff line number Diff line change
@@ -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)
12 changes: 12 additions & 0 deletions vlib/v/checker/tests/slice_clone_notice.vv
Original file line number Diff line number Diff line change
@@ -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]
}
8 changes: 4 additions & 4 deletions vlib/v/gen/js/tests/array.v
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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'

Expand Down
12 changes: 12 additions & 0 deletions vlib/v/tests/array_slice_assign_test.v
Original file line number Diff line number Diff line change
@@ -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]
}
6 changes: 3 additions & 3 deletions vlib/v/tests/array_slice_test.v
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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
Expand Down

0 comments on commit 815439a

Please sign in to comment.