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

x/exp/slices: Replace doesn't panic with invalid indices #56700

Closed
blackgreen100 opened this issue Nov 11, 2022 · 2 comments
Closed

x/exp/slices: Replace doesn't panic with invalid indices #56700

blackgreen100 opened this issue Nov 11, 2022 · 2 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@blackgreen100
Copy link

blackgreen100 commented Nov 11, 2022

What version of Go are you using (go version)?

Go 1.19

Does this issue reproduce with the latest release?

Yes, at tip

What did you do?

The documentation for slices.Replace states that:

[...] Replace panics if s[i:j] is not a valid slice of s.

The following program doesn't panic, instead it returns a rather unexpected output:

func main() {
	x := []int{10, 20, 30, 40}
	y := []int{3, 4}

	lo, hi := 2, 1
	// fmt.Println(x[lo:hi]) this would panic with `panic: runtime error: slice bounds out of range [2:1]`
	z := slices.Replace(x, lo, hi, y...)
	fmt.Println(z)
}

https://go.dev/play/p/hCkCLy0YZnT?v=gotip

What did you expect to see?

A panic, because the slice expression x[2:1] is out of bounds as per the specs (and as per the Replace documentation):

For arrays or strings, the indices are in range if 0 <= low <= high <= len(a), otherwise they are out of range. For slices, the upper index bound is the slice capacity cap(a) rather than the length

What did you see instead?

The output of printing z is:

[10 20 3 4 20 30 40]

(unexpected outputs occur also in the general case when j - i != len(y))

Should probably just add bounds check like slices.Delete at the beginning of the function:

_ = s[i:j] // bounds check
@gopherbot gopherbot added this to the Unreleased milestone Nov 11, 2022
@blackgreen100 blackgreen100 changed the title x/exp/slices: behavior of slices.Replace is inconsistent with subcli x/exp/slices: behavior of slices.Replace is inconsistent with documentation Nov 11, 2022
@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 11, 2022
@mknyszek
Copy link
Contributor

CC @ianlancetaylor

Despite the "Documentation" tag that gopherbot added, this just seems like a bug in that package.

@blackgreen100 blackgreen100 changed the title x/exp/slices: behavior of slices.Replace is inconsistent with documentation x/exp/slices: Replace doesn't panic with invalid indices Nov 11, 2022
@gopherbot
Copy link

Change https://go.dev/cl/449915 mentions this issue: slices: don't accept out of order indexes in Replace

@seankhliao seankhliao added NeedsFix The path to resolution is known, but the work has not been done. and removed Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Nov 11, 2022
@golang golang locked and limited conversation to collaborators Nov 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants