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.

Sign up for GitHub

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

Jump to bottom

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
Closed

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

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 •
edited

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 •
edited

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.

@callthingsoff callthingsoff mentioned this issue Nov 6, 2023
Closed
@gopherbot
Copy link

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

This was referenced Nov 15, 2023
Open
Closed
@gopherbot
Copy link

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
@TapirLiu @gopherbot
slices: improve Insert panic message for index out of range
2551fff 
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
Assignees
No one assigned
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
No milestone
5 participants
@gazerro @eliben @bcmills @randall77 @gopherbot