Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

checker: fix assigning array slice (fix #19120) #19137

Merged
merged 6 commits into from
Aug 15, 2023

Conversation

yuyi98
Copy link
Member

@yuyi98 yuyi98 commented Aug 14, 2023

This PR fix assigning array slice (fix #19120).

  • Fix assigning array slice.
  • mut a := arr[..i] -> mut a := arr[..i].clone() if left var or right var is mut and not inside unsafe.
  • Add test.
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]
}

PS D:\Test\v\tt1> v run .
tt1.v: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: tt1.v: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)
[8, 7, 6, 5, 4, 3, 2]
[1, 2, 3, 4, 5, 6, 7, 8]

Comment on lines +28 to +29
slice2 := unsafe { arr2[..3] }
slice3 := unsafe { arr2[3..] }
Copy link
Member

@spytheman spytheman Aug 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should not need unsafe, arr2 is immutable, but slice2 and slice3 are not mutable too -> they can not be used to make modifications to arr2.

@yuyi98
Copy link
Member Author

yuyi98 commented Aug 15, 2023

"unsafe" here is not just about not modifying the immutable array.
Also to ensure that the left var and right arrays are not affected by their respective changes.

@spytheman
Copy link
Member

Also to ensure that the left var and right arrays are not affected by their respective changes.

That is a valid viewpoint, now that I think about it more. It will make some programs a little more cumbersome, but clearer/less surprising indeed.

@spytheman spytheman merged commit 815439a into vlang:master Aug 15, 2023
48 checks passed
@yuyi98 yuyi98 deleted the fix_mut_array_slice branch August 16, 2023 01:34
Wertzui123 pushed a commit to Wertzui123/v that referenced this pull request Oct 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Modifications to mutable slices affect non-mut arrays
2 participants