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

breaking changes to 'slices.SortFunc' #208

Closed
KEINOS opened this issue Jul 27, 2023 · 2 comments
Closed

breaking changes to 'slices.SortFunc' #208

KEINOS opened this issue Jul 27, 2023 · 2 comments

Comments

@KEINOS
Copy link

KEINOS commented Jul 27, 2023

A breaking change have been made to slices package in golang.org/x/exp module (slices.SortFunc in particular).

slices: update to current standard library version
Update x/exp/slices to the current standard library slices package,
while retaining the ability to use it with Go 1.18 through Go 1.20.

Note that this changes some of the sorting functions to use a
comparison function rather than a less function. We don't promise
backward compatibility in x/exp packages. Being compatible with the
Go 1.21 package seems more useful for people not yet using 1.21,
as it will make the transition to 1.21 easier.

It is the comparison function, an argument of slices.SortFunc, that affects our package.
func(a, b E) bool vs func(a, b E) int.

- func SortFunc[E any](x []E, less func(a, b E) bool) {
+ func SortFunc[S ~[]E, E any](x S, cmp func(a, b E) int) {

Currently there are no problems, but when updating dependent modules the following error occurs:

How to reproduce error

$ cat version.json
{
  "version": "v0.10.1"
}

$ go test ./...
?       github.com/multiformats/go-multiaddr/multiaddr  [no test files]
ok      github.com/multiformats/go-multiaddr    0.262s
ok      github.com/multiformats/go-multiaddr/net        0.533s

$ go get -u golang.org/x/exp
go: upgraded golang.org/x/exp v0.0.0-20230626212559-97b1e661b5df => v0.0.0-20230725093048-515e97ebf090

$ go test ./...
# github.com/multiformats/go-multiaddr
./multiaddr.go:223:25: type func(a Multiaddr, b Multiaddr) bool of func(a, b Multiaddr) bool {…} does not match inferred type func(a Multiaddr, b Multiaddr) int for func(a E, b E) int
FAIL    github.com/multiformats/go-multiaddr [build failed]
FAIL    github.com/multiformats/go-multiaddr/net [build failed]
FAIL

Proposed fix

slices.SortFunc(addrs, func(a, b Multiaddr) bool { return bytes.Compare(a.Bytes(), b.Bytes()) < 0 })

- slices.SortFunc(addrs, func(a, b Multiaddr) bool { return bytes.Compare(a.Bytes(), b.Bytes()) < 0 })
+ slices.SortFunc(addrs, func(a, b Multiaddr) int { return bytes.Compare(a.Bytes(), b.Bytes()) })

Note

Even if all other modules are updated, no further changes seem to be necessary at this stage.

$ go get -u ./...
go: upgraded github.com/ipfs/go-cid v0.0.7 => v0.4.1
go: added github.com/klauspost/cpuid/v2 v2.2.5
go: upgraded github.com/minio/sha256-simd v0.1.1-0.20190913151208-6de447530771 => v1.0.1
go: upgraded github.com/mr-tron/base58 v1.1.3 => v1.2.0
go: upgraded github.com/multiformats/go-base32 v0.0.3 => v0.1.0
go: upgraded github.com/multiformats/go-base36 v0.1.0 => v0.2.0
go: upgraded github.com/multiformats/go-multibase v0.0.3 => v0.2.0
go: upgraded github.com/multiformats/go-multihash v0.0.14 => v0.2.3
go: upgraded github.com/multiformats/go-varint v0.0.6 => v0.0.7
go: upgraded golang.org/x/crypto v0.1.0 => v0.11.0
go: upgraded golang.org/x/exp v0.0.0-20230626212559-97b1e661b5df => v0.0.0-20230725093048-515e97ebf090
go: upgraded golang.org/x/sys v0.1.0 => v0.10.0
go: added lukechampine.com/blake3 v1.2.1

$ go test ./...
?       github.com/multiformats/go-multiaddr/multiaddr  [no test files]
ok      github.com/multiformats/go-multiaddr    0.727s
ok      github.com/multiformats/go-multiaddr/net        0.567s

Env info

$ go version
go version go1.20.6 darwin/amd64

$ # go-multiaddr ver
$ cat version.json
{
  "version": "v0.10.1"
}
@KEINOS
Copy link
Author

KEINOS commented Jul 27, 2023

@marten-seemann
Copy link
Contributor

This has been resolved.

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

No branches or pull requests

2 participants