For loop with map produces incorrect values when assigned in-loop to new struct

[BrentFarris]

Go version

1.22

Output of go env in your module/workspace:

set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\brent\AppData\Local\go-build
set GOENV=C:\Users\brent\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\brent\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\brent\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Program Files\Go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLCHAIN=auto
set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.22.0
set GCCGO=gccgo
set GOAMD64=v1
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=NUL
set GOWORK=
set CGO_CFLAGS=-O2 -g
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-O2 -g
set CGO_FFLAGS=-O2 -g
set CGO_LDFLAGS=-O2 -g
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=C:\Users\brent\AppData\Local\Temp\go-build1737355225=/tmp/go-build -gno-record-gcc-switches

What did you do?

I was looping through a map 2x to do a simple start/end assignment (the lazy way). I noticed the numbers were all over the place and incorrect for the end value (the one being assigned). I thought it was something strange with using a range loop on the map directly. So I pulled out the keys and I assigned using the keys directly. Both of them produced the same error.

I thought I was going crazy and was just not seeing what was going on. So I decided to try a 3rd method. Use the copy variable for the assignment, that actually worked correctly. I've made a sample on the Go Playground here: https://go.dev/play/p/I6eZssj97fn

Code from playground:

package main

type startEnd struct {
	start int
	end   int
}

func makePositions() map[string]startEnd {
	positions := make(map[string]startEnd)
	positions["ONE"] = startEnd{64, 844}
	positions["TWO"] = startEnd{32, 844}
	positions["THREE"] = startEnd{148, 844}
	positions["FOUR"] = startEnd{500, 844}
	return positions
}

func incorrect() {
	println("Incorrect...")
	positions := makePositions()
	for k, v := range positions {
		for _, v2 := range positions {
			if v2.start <= v.start {
				continue
			}
			positions[k] = startEnd{v.start, min(v2.start, v.end)}
		}
	}
	for k, v := range positions {
		println(k, v.start, v.end)
	}
}

func correct() {
	println("Correct...")
	positions := makePositions()
	for k, v := range positions {
		for _, v2 := range positions {
			if v2.start <= v.start {
				continue
			}
			v.end = min(v2.start, v.end)
			positions[k] = v
		}
	}
	for k, v := range positions {
		println(k, v.start, v.end)
	}
}

func main() {
	incorrect()
	println("")
	correct()
}

Output:

Incorrect...
ONE 64 148
TWO 32 500
THREE 148 500
FOUR 500 844

Correct...
ONE 64 148
TWO 32 64
THREE 148 500
FOUR 500 844

What did you see happen?

ONE 64 148
TWO 32 500
THREE 148 500
FOUR 500 844

What did you expect to see?

ONE 64 148
TWO 32 64
THREE 148 500
FOUR 500 844

[seankhliao]

you're updating position[k], but doing the comparison against v, which is only updated once at the start.

Unlike many projects, the Go project does not use GitHub Issues for general discussion or asking questions. GitHub Issues are used for tracking bugs and proposals only.

For questions please refer to https://github.com/golang/go/wiki/Questions

[BrentFarris]

🤦‍♂️ Yup, sorry for wasting your time. I usually never use maps this way and for obvious reasons