slices: better panic message for out of range Insert

[sedyh]

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

$ go version 
go version go1.21 darwin/arm64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
GO111MODULE=""
GOARCH="arm64"
GOBIN=""
GOCACHE="/Users/sedyh/Library/Caches/go-build"
GOENV="/Users/sedyh/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/sedyh/Desktop/share/go/path/go1.21.1/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/sedyh/Desktop/share/go/path/go1.21.1"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/Users/sedyh/Desktop/share/go/root/go1.21.1"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/sedyh/Desktop/share/go/root/go1.21.1/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="go1.21.1"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/x5/kntr8jy137vg9cjlm83chwmh0000gn/T/go-build3952104386=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

For some reason slices.Insert causes out of bounds panic if there no values at if !overlaps(v, s[i+m:]) {}.

As far as I can see in https://go.dev/cl/540155, this is intended, but it seems wrong to me, because:

• I don't see any point in this. I think this is the only case why I wanted to use it.
• This case is not processed separately, it is a standard out of bounds panic.
• This is not stated in the function comments.

What did you expect to see?

s := []float64{}
s = slices.Grow(s, 5)
s = slices.Insert(s, 4, 1.0)

The function works the same regardless of how full the slice is.

What did you see instead?

panic: runtime error: slice bounds out of range [5:1]

This is not a specially caused panic, it looks like a missed case.

[randall77]

What would end up in s[0] through s[3]?
In other words, what is the slice that you expect to result from the example you gave?

[sedyh]

What would end up in s[0] through s[3]? In other words, what is the slice that you expect to result from the example you gave?

I hoped that slices.Insert would fill them with default values for me (.0) if index is bigger than cap or replace the existing values if index is lower than cap.

[gophun]

• This is not stated in the function comments.

It is stated in the doc comment: "Insert panics if i is out of range."

This case is not processed separately, it is a standard out of bounds panic.

Because it is a standard out of bounds panic. slices.Grow does not grow the length of a slice, it only grows the capacity. You would get the same panic here:

s := []float64{}
s = slices.Grow(s, 5)
s[4] = 1.0

[sedyh]

I would really like some of the functions in this package to be able to do make/append and fill the passed slice up to requested index as needed.

Is there any workaround or slice trick that combines insert and growing the length?

[sedyh]

Is there any workaround or slice trick that combines insert and growing the length?

Slice tricks' Expand look like what I need, I wish that func was in the slices package.

a = append(a[:i], append(make([]T, j), a[i:]...)...)

[zigo101]

The one-line trick is quite inefficient for some cases.

For OP's specified case:

s := []float64{}
s = slices.Grow(s, 5)
s = slices.Insert(s, 4, 1.0)

just do:

s := append(make([]float64, 4, 5), 1.0)

[zigo101]

If looks it is essential to put the following line at the start of the slices.Insert funciton:

_ = s[i:]

Otherwise, the panic message is misleading.

The line should be helpful for BCE.

This fix for #63913 is not a perfect one.

[sedyh]

Just do:
s := append(make([]float64, 4, 5), 1.0)

Thanks for suggestion, but I also need to be able to insert elements at any index location other than extending the slice, so the previous one is more suitable.

It is stated in the doc comment: "Insert panics if i is out of range."

I feel like it would be better to state "or slice is empty" separately, even if the first option implies this.

Otherwise, the panic message is misleading.

+1

[gopherbot]

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

[gazerro]

I think it could help if the term "out of range" is not used in the documentation:

... Insert panics if i is out of range.. ...

because it might not be clear that "out of range" refers to the slicing s[i:]. It could be interpreted as referring to the slicing s[:i]. With the same value of i, s[i:] can be out of range, and s[:i] may not be.

To remove this ambiguity, I would write:

... Insert panics if i > len(s). ...

[gopherbot]

Change https://go.dev/cl/542535 mentions this issue: slices: add explanation of 'out of range' for Insert

[callthingsoff]

... Insert panics if i > len(s). ...

@gazerro Also panics when i < 0.

[gazerro]

@callthingsoff You're right. We can use a similar sentence used for Delete and Replace (see recent commit 7a1fce8), so it would be

... Insert panics if i > len(s) or s[:i] is not a valid slice of s. ...

[callthingsoff]

We can use a similar sentence used for Delete and Replace (see recent commit 7a1fce8), so it would be

... Insert panics if i > len(s) or s[:i] is not a valid slice of s. ...

cc @ianlancetaylor @randall77