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: comparable does not match each comparable type #51392

Closed
blaubaer opened this issue Feb 28, 2022 · 7 comments
Closed

cmd/compile: comparable does not match each comparable type #51392

blaubaer opened this issue Feb 28, 2022 · 7 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.18

Comments

@blaubaer
Copy link

blaubaer commented Feb 28, 2022 •
edited
Loading

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

$ go version
go version devel go1.18-a064a4f29a Sat Feb 26 01:16:03 2022 +0000 windows/amd64

Does this issue reproduce with the latest release?

With the latest revision on master (a064a4f29a97a4fc7398d1ac9d7c53c5ba0bc646).

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

go env Output
$ go env
set GO111MODULE=on
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\protected\AppData\Local\go-build
set GOENV=C:\Users\protected\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\protected\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\protected\go
set GOPRIVATE=
set GOWORK=
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -fmessage-length=0 -fdebug-prefix-map=C:\Users\protected\AppData\Local\Temp\go-build4145725796=/tmp/go-build -gno-record-gcc-switches

What did you do?

language.Tag as Map key

package main

import (
	"fmt"

	"golang.org/x/text/language"
)

func main() {
	aMap := map[language.Tag]string{
		language.German: "foo",
	}
	fmt.Println(aMap)
}

language.Tag as comparable generic parameter

package main

import (
	"fmt"
	"golang.org/x/text/language"
)

func doSomething[K comparable](with map[K]string) {
	fmt.Println("comparable", with)
}

func main() {
	aMap := map[language.Tag]string{
		language.German: "foo",
	}
	doSomething(aMap)
}

The following code

What did you expect to see?

  1. language.Tag as Map key: Is accepted by the compiler.
  2. language.Tag as comparable generic parameter: Is accepted by the compiler.

What did you see instead?

  1. language.Tag as Map key: Is accepted by the compiler.
  2. language.Tag as comparable generic parameter: Compiler refuses compiling with 
    foo_broken.go:16:13: "golang.org/x/text/language".Tag does not implement comparable

ℹ️ I'm pretty sure that this was working at least ~1 week ago with the master. With revision a289e9ce7514a34cd930469322395bf0e89b59ea) it was working.
@thanm thanm added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 28, 2022
@thanm
Copy link
Contributor

thanm commented Feb 28, 2022

Biesction points to 163da6feb525a98dab5c1f01d81b2c705ead51ea as the bad commit, introduced in CL 387055.

@griesemer @findleyr

@findleyr
Copy link
Member

The reason for this is that Tag contains fullTag, which is an interface. This is discussed in more detail in #51257 (comment)

This is a confusing UX, particularly when the interface is buried deep inside the type. Here the interface is only one level deep, but one can imagine how confusing it could be if the interface were inside several layers of nested structs.

IMO this warrants reconsideration.

@findleyr findleyr added this to the Go1.18 milestone Feb 28, 2022
@blaubaer
Copy link
Author

Please correct me if I'm wrong. The specs defines the same rules for map keys as for stuff that implements comparable, or? If so either the map implementation in Go is broken in general or how 163da6feb525a98dab5c1f01d81b2c705ead51ea change it was too restrictive. I would highly recommend to align both behaviors.

@findleyr
Copy link
Member

Please correct me if I'm wrong. The specs defines the same rules for map keys as for stuff that implements comparable

To clarify: no, map keys treat all interfaces as comparable, whereas the comparable builtin treats only interfaces that are statically known to be comparable as comparable.

@ianlancetaylor
Copy link
Member

ianlancetaylor commented Feb 28, 2022 •
edited
Loading

Closing as dup of #50646 and #51257 and #51338.

@changkun

This comment was marked as outdated.

Sign in to view
@ianlancetaylor

This comment was marked as outdated.

Sign in to view
@golang golang locked and limited conversation to collaborators Mar 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.18
Development

No branches or pull requests
6 participants
@blaubaer @ianlancetaylor @changkun @gopherbot @thanm @findleyr