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

slices: Insert function does not panic if i is out of range and there are no values to insert #63913

Closed
gazerro opened this issue Nov 2, 2023 · 5 comments
Labels
NeedsFix The path to resolution is known, but the work has not been done.

Comments

@gazerro
Copy link
Contributor

gazerro commented Nov 2, 2023

The slices.Insert function should panic if i is out of range, but it does not panic if there are no values to insert:

s := []string{"a", "b", "c"}
s = slices.Insert(s, -1) // does not panic
s = slices.Insert(s, 5) // does not panic
fmt.Println(s)
@gazerro gazerro changed the title slices: Insert function does not panic if i is out of range and there are not values to insert slices: Insert function does not panic if i is out of range and there are no values to insert Nov 2, 2023
@bcmills
Copy link
Contributor

bcmills commented Nov 2, 2023

The documentation does indeed claim that it will panic:
“Insert panics if i is out of range.”

(attn @ianlancetaylor @eliben)

@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Nov 2, 2023
@eliben
Copy link
Member

eliben commented Nov 2, 2023

Interesting edge case. I looked back at the original implementation of slices.Insert when it was just moved into the stdlib, and that one panics for the test cases specified here. However, following the optimizations in https://go-review.googlesource.com/c/go/+/494817 it no longer panics for these cases. @randall77 @cuonglm WDYT?

@randall77
Copy link
Contributor

Making it panic in these cases is fine.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/540155 mentions this issue: slices: make Insert panic if index is out of range and there are no values

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/542455 mentions this issue: slices: imporve Insert panic message for index out of range

gopherbot pushed a commit that referenced this issue Nov 19, 2023
The panic message of the current implementation for index out of range is not ideal.
This PR tries to improve it.

Fixes #63913 and #64152

Change-Id: Ibcf6c9c0f555c8b8bf46b7d6f20f0ccc5698acd4
GitHub-Last-Rev: 1bbec23
GitHub-Pull-Request: #64163
Reviewed-on: https://go-review.googlesource.com/c/go/+/542455
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@golang.org>
Auto-Submit: Keith Randall <khr@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
5 participants