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

cmd/compile: compilation results for generic types are affected by the filename #51048

Closed
sxyazi opened this issue Feb 7, 2022 · 10 comments
Closed
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Milestone

Comments

@sxyazi
Copy link

sxyazi commented Feb 7, 2022

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

$ go version
go version go1.18beta2 linux/arm64

Does this issue reproduce with the latest release?

Yes, it can be reproduced in the latest beta version. The minimal reproducible code I put at https://github.com/sxyazi/awesomeProject

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

go env Output
$ go env
GO111MODULE=""
GOARCH="arm64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOENV="/root/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/root/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/root/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/root/sdk/go1.18beta2"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/root/sdk/go1.18beta2/pkg/tool/linux_arm64"
GOVCS=""
GOVERSION="go1.18beta2"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/www/awesomeProject/go.mod"
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 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3302494008=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I executed go test, but it got an error. When I changed the file name from foo.go to hello.go, it worked correctly again. I suspect it has something to do with the compilation order of the filenames.

reproducing

What did you expect to see?

Pass the test correctly

What did you see instead?

./foo.go:14:13: T does not implement []E ([]E is not an interface)
@mvdan
Copy link
Member

mvdan commented Feb 7, 2022

Can you please try with the latest Go master version? This bug might have been fixed in the week since beta2 came out. Note that you'll need to change the import of the constraints package as per #50792.

@sxyazi
Copy link
Author

sxyazi commented Feb 7, 2022

@mvdan Thanks for your reply quickly. I used gotip to get the master branch version:

go version devel go1.18-867a3d5 Mon Feb 7 12:32:51 2022 +0000 darwin/arm64

and replaced constraints with golang.org/x/exp/constraints to import it, as the method in that issue. But unfortunately, the problem still exists

@mvdan
Copy link
Member

mvdan commented Feb 7, 2022

Ah, could you please push to your repo so we can also reproduce with master?

@mvdan mvdan added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 7, 2022
@sxyazi
Copy link
Author

sxyazi commented Feb 7, 2022

Okay, it's there now

@danscales
Copy link
Contributor

The problem is fixed if you change

func Sum[T []E, E constraints.Integer | constraints.Float](items T) (total E) {

to

func Sum[T interface{ []E }, E constraints.Integer | constraints.Float](items T) (total E) {

or

func Sum[T ~[]E, E constraints.Integer | constraints.Float](items T) (total E) {

As the error message indicates, []E is not an interface constraint. You can wrap it in interface{} to make it an interface constraint. But we also consider a type U to be equivalent to interface { U } (a real type constraint) if U contains either a tilde or a bar.

@griesemer @findleyr

Robert/Rob: Things do in fact work if foo.go is renamed to hello.go. Also, we are not complaining about the use of T []E in MakeCollection. So, the error message about []E not be an interface (hence not a proper constraint) seems to not be provided consistently in all cases. Would be good to investigate why it is not happen when foo.go is renamed to hello.go, why it's not happening for MakeCollection, etc.

@griesemer griesemer self-assigned this Feb 7, 2022
@griesemer griesemer added this to the Go1.18 milestone Feb 7, 2022
@griesemer
Copy link
Contributor

griesemer commented Feb 7, 2022

It looks like an ordering problem (hence the dependence of the filename, the sources are processed by the type checker in filename-sorted order for deterministic behavior).

Simpler reproducer:

package p

// func f[T int]() {}  // using this f doesn't produce the error

func _[T int]() {
	_ = f[T] // ERROR T does not implement int (int is not an interface)
}

func f[T int]() {}  // using this f produces the error

@gopherbot
Copy link

Change https://go.dev/cl/383834 mentions this issue: go/types, types2: ensure we have an interface before checking constraints

@sxyazi
Copy link
Author

sxyazi commented Feb 7, 2022

@griesemer Thanks for your work. Is complaining about mistakes to be expected? Because I find it works well everywhere else except complaining

@griesemer
Copy link
Contributor

@sxyazi I'm not sure I understand: what do you mean with "except complaining"? Do you mean that compiler is complaining with errors?

With respect to this issue: this was a real bug in the compiler; thanks for bringing it up to our attention. I believe it should be fixed with the above CL. (And yes, if you find more cases that you think should work but don't, please file an issue.)

@sxyazi
Copy link
Author

sxyazi commented Feb 7, 2022

@griesemer OK, I see. Thanks for your work again.

@golang golang locked and limited conversation to collaborators Jun 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Projects
None yet
Development

No branches or pull requests

5 participants