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

cmd/compile: switching over channel value incorrectly does not match when the direction is narrower in the case #67190

Open
Jorropo opened this issue May 5, 2024 · 10 comments
Open

cmd/compile: switching over channel value incorrectly does not match when the direction is narrower in the case #67190

Jorropo opened this issue May 5, 2024 · 10 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Backlog

Comments

@Jorropo
Copy link
Member

Jorropo commented May 5, 2024 •
edited

Go version

go version devel go1.23-619b419a4b Sun May 5 00:26:04 2024 +0000 linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/tmp/go-build'
GOENV='/home/hugo/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/hugo/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/hugo/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/hugo/k/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/home/hugo/k/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='devel go1.23-619b419a4b Sun May 5 00:26:04 2024 +0000'
GODEBUG=''
GCCGO='/usr/bin/gccgo'
GOAMD64='v3'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build988502288=/tmp/go-build -gno-record-gcc-switches'

What did you do?

@LeaBit on discord gophers report this behavior:

package main

import "fmt"

func main() {
	ch1 := make(chan struct{})

	var ch2 <-chan struct{} = ch1

	switch ch1 {
	case ch2:
		fmt.Println("Narrow case: Same!")
	default:
		fmt.Println("Narrow case: Not same!")
	}

	switch ch2 {
	case ch1:
		fmt.Println("Narrow switch: Same!")
	default:
		fmt.Println("Narrow switch: Not same!")
	}

	fmt.Println("Equal:", ch1 == ch2)
}

What did you see happen?

Narrow case: Not same!
Narrow switch: Same!
Equal: true

What did you expect to see?

Narrow case: Same!
Narrow switch: Same!
Equal: true
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label May 5, 2024
@Jorropo Jorropo added help wanted NeedsFix The path to resolution is known, but the work has not been done. and removed compiler/runtime Issues related to the Go compiler and/or runtime. labels May 5, 2024
@Jorropo Jorropo added this to the Backlog milestone May 5, 2024
@Jorropo Jorropo added the compiler/runtime Issues related to the Go compiler and/or runtime. label May 5, 2024
@Jorropo
Copy link
Member Author

Jorropo commented May 5, 2024

cc @golang/compiler

@randall77
Copy link
Contributor

randall77 commented May 5, 2024 •
edited

The behavior changed from 1.19 to 1.20.
A bisect would be helpful to track down the cause.

@Jorropo Jorropo changed the title cmd/compile: switching over channel value incorrectly does not match when the direction is narower in the case cmd/compile: switching over channel value incorrectly does not match when the direction is narrower in the case May 5, 2024
@Jorropo
Copy link
Member Author

Jorropo commented May 5, 2024

I'll send a fix

@Jorropo
Copy link
Member Author

Jorropo commented May 6, 2024

Bisect results:

@Jorropo
Copy link
Member Author

Jorropo commented May 6, 2024 •
edited

Ok so the bug, this code assumes that the type of the case can't be assigned to the type of the tag it needs to compared with any and comparing with any first compare using the exact type tag (which is wrong because you can't assign <-chan T to chan T but you can == them).

ch1 := make(chan struct{})
var ch2 <-chan struct{} = ch1
fmt.Println("Equal:", ch1 == ch2)
fmt.Println("any Equal:", any(ch1) == any(ch2))

prints true and false, I don't know if this is a bug or intended.

@Jorropo
Copy link
Member Author

Jorropo commented May 6, 2024 •
edited

It looks like the any compare behavior is correct:

Two interface values are equal if they have identical dynamic types and equal dynamic values or if both have value nil.

so TIL a == b has different semantics as any(a) == any(b)

https://gophers.slack.com/archives/C1C1YSQBT/p1714960380488909

@cuonglm
Copy link
Member

cuonglm commented May 6, 2024

Possibly duplicate of #44051

@Jorropo
Copy link
Member Author

Jorropo commented May 6, 2024

I don't think so, here I'm assuming the behavior seen in #44051 is correct and not a bug yet this is still broken.

@gopherbot
Copy link

Change https://go.dev/cl/583395 mentions this issue: Revert "[dev.unified] cmd/compile/internal/noder: better switch statements"

@Jorropo
Copy link
Member Author

Jorropo commented May 6, 2024

Needs more changes in cmd/walk/switch.go to achieve what I want, it's tricky, someone else should make sure to do it right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
Go Compiler / Runtime
Status: Todo
Milestone
Backlog
Development

No branches or pull requests
4 participants
@cuonglm @randall77 @gopherbot @Jorropo