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: incorrect type set intersection computation #57486

Closed
go101 opened this issue Dec 28, 2022 · 27 comments
Closed

cmd/compile: incorrect type set intersection computation #57486

go101 opened this issue Dec 28, 2022 · 27 comments
Assignees
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone

Comments

@go101
Copy link

go101 commented Dec 28, 2022

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

$ go version
go version go1.19.4 linux/amd64

Does this issue reproduce with the latest release?

Yes, also for tip.

What did you do?

package main

func main() {}

type C1 interface {
	comparable
}

type C2 interface {
	comparable
	[2]any | int
}

func G1[T C1](t T) {}
func G2[T C2](t T) {}

func F1[V [2]any](v V) {
	_ = G1[V] // error: V does not implement comparable
}

func F2[V [2]any](v V) {
	_ = G2[V] // okay (but should not?)
}

What did you expect to see?

Both F1 and F2 fail to compile.

What did you see instead?

F2 compiles okay.

My understanding is that the type set of C2 only contains int, because [2]any is not strictly comparable, though my understanding might be wrong.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Dec 28, 2022
@cuonglm
Copy link
Member

cuonglm commented Dec 28, 2022

F2 compile ok, because C2 is an empty type set.

There is no type that both implement comparable and is an array [2]any.

@ianlancetaylor
Copy link
Contributor

Note that in 1.20 C2 is satisfied by [2]any, per #56548.

It's not clear to me why this compiles with 1.18 and 1.19. CC @griesemer .

@cuonglm
Copy link
Member

cuonglm commented Dec 28, 2022

@ianlancetaylor C2 is an empty type set, the same as:

type C2 interface {
    string
    int
}

@go101
Copy link
Author

go101 commented Dec 28, 2022

@cuonglm

,,, C2 is an empty type set.
There is no type that both implement comparable and is an array [2]any.

But why doesn't the type set of C2 contain int?

I looks the calculated type set of C2 in the following code is indeed [[2]any, struct{x any}, int], which is obviously wrong.

package main

func main() {
	var x struct{x any}
	var y [2]any
	G2(x) // okay
	G2(y) // okay
	var z int
	G2(z) // okay
}

type C2 interface {
	comparable
	[2]any | int | struct{x any}
}

func G2[T C2](t T) {
	// 1.20rc1 error message
	_ = t == t // invalid operation: t == t (incomparable types in type set)
}

@cuonglm
Copy link
Member

cuonglm commented Dec 28, 2022

@go101 the semantic for go1.20 was changed, see Ian's comment above.

@go101
Copy link
Author

go101 commented Dec 28, 2022

I read the new semantic changes several times. Maybe I still haven't got it.
I don't get why the type set of C2 is not [int]. I don't think it is relevant to the 1.20 semantic changes.

And per @ianlancetaylor 's

Note that in 1.20 C2 is satisfied by [2]any,

My understanding is that C2 is only satisfied by [2]any only when [2]any is used as a value type (but for the above example, it is not anyway), not a type constraint. (That is why the F1 function fails to compile, which is an expected behavior.)

@cuonglm
Copy link
Member

cuonglm commented Dec 28, 2022

You can see C2 as:

type non_comparable interface {
	[2]any | int 
}

type C2 interface {
	comparable
	non_comparable
}

non_comparable does not implement comparable, because not all types in the type set of non_comparable are comparable. So there's no type that both implement comparable and does not implement comparable, so C2's type set is empty.

See: https://go.dev/ref/spec#Type_constraints

@go101
Copy link
Author

go101 commented Dec 28, 2022

I don't think the interpretation is right, if the compilation behavior is right for the following program.

package main

func main() {
	var x int
	G2(x) // okay
	var y []byte
	G2(y) // error: []byte does not implement C2 ([]byte missing in ~int)
}

type C2 interface {
	comparable
	~int | ~[]byte
}

func G2[T C2](t T) {
	_ = t == t // okay
}

@atdiar
Copy link

atdiar commented Dec 28, 2022

@go101
(from my understanding)
The real constraint is interface{[2]any}
It has to implement comparable but does not implement the constraint strictly comparable. (easily understood since implementation is type set inclusion)

Strictly comparable is different from comparable in that it asserts that the members of its type set are themselves implementing comparable instead of just satisfying it.
(i.e. the members of stricly comparable are themselves strictly comparable)

According to my current reading of the spec at tip, strictly comparable does not matter here because [2]any is not an interface.
[2]any satisfies comparable because it has the comparison operators. We have type set inclusion. So the type parameter constraint you are using actually implements C2. All is fine.

The compiler is correct.

@go101
Copy link
Author

go101 commented Dec 28, 2022

Okay, now the only I don't understand is why the _ = t == t line doesn't compile? (edit: I'm still confused in several points, ;))

@cuonglm
Copy link
Member

cuonglm commented Dec 28, 2022

@go101 you're right. I was wrong that the type set is empty, it's actually contain [2]any and int. Running gotype with tracing enabled:

p.go:10:6:	-- checking func G2 (white, objPath = )
p.go:10:9:	.  type params = [T₁]
p.go:10:11:	.  -- type C2
p.go:10:11:	.  => C2 (under = interface{comparable; [2]any | int}) // *Named
p.go:10:17:	.  -- type T
p.go:5:6:	.  -- type set for interface{comparable; [2]any | int}
p.go:5:6:	.  => {[2]any | int}
p.go:10:17:	.  => T₁ (under = interface{comparable; [2]any | int}) // *TypeParam
p.go:10:6:	=> func G2[T C2](t T) (black)

@atdiar
Copy link

atdiar commented Dec 28, 2022

@go101 In that case, C2 is reducible/can be rewritten to interface{~int}.

Embedding the comparable constraint enforces that any type argument has to be(means "has to satisfy") comparable. Via the conjunction/embedding, C2 enforces that any potential type argument is comparable.

C2 is just not the canonical form of the expressed constraint. ~[]byte can be eliminated.

Passing a type inferred variable of type []byte to G2 doesn't compile (it's not comparable). It's normal. :)

@go101
Copy link
Author

go101 commented Dec 28, 2022

@atdiar
More specifically, I mean why the _ = t == t line in the following code doesn't compile.

type C2 interface {
	comparable
	[2]any | int | struct{x any}
}

func G2[T C2](t T) {
	// 1.20rc1 error message
	_ = t == t // invalid operation: t == t (incomparable types in type set)
}

@atdiar
Copy link

atdiar commented Dec 28, 2022

@go101 ah yes, you're right, I actually don't know. Looks like a bug to me as well.
cc @ianlancetaylor @griesemer

@go101
Copy link
Author

go101 commented Dec 28, 2022

Re-read the relevant sections in tip spec:

Type parameters are comparable if they are strictly comparable (see below).

Type parameters are strictly comparable if all types in their type set are strictly comparable.

So it is expected that the line _ = t == t doesn't compile, because the type of t is not comparable.

The weirdness still comes from the calculation of the type set of C2. It looks gc treats the following C2 and C3 as equivalent:

type C2 interface {
	comparable
	[2]any | int
}

type C3 interface {
	[2]any | int
}

What is the semantics of embedding comparable?

@go101
Copy link
Author

go101 commented Dec 28, 2022

Tip spec says:

The predeclared interface type comparable denotes the set of all non-interface types that are strictly comparable.

Struct types are strictly comparable if all their field types are strictly comparable.

Array types are strictly comparable if their array element types are strictly comparable.

So I think this is a bug of gc. Because the type set of C2 should be the intersection of the type sets of comparable and C3, a.k.a. [int].

@go101
Copy link
Author

go101 commented Dec 28, 2022

Re-read 1.19 spec. It looks it is an expected behavior to view C2 and C3 as equivalent by 1.19 spec.
1.19 spec views [2]any as a comparable type and thinks it implements comparable.

The predeclared interface type comparable denotes the set of all non-interface types that are comparable. Specifically, a type T implements comparable if:

  • T is not an interface type and T supports the operations == and !=; or
  • T is an interface type and each type in T's type set implements comparable.

Struct values are comparable if all their fields are comparable.

Array values are comparable if values of the array element type are comparable.

So by 1.19 spec, the F1 and F2 functions should both compile okay; and by tip spec, the two functions should both fail to compile.

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 28, 2022
@dmitshur dmitshur added this to the Backlog milestone Dec 28, 2022
@rittneje
Copy link

The 1.20 compiler currently violates the language spec. https://go.dev/play/p/bqahFnk1xbV?v=gotip
The first two are taken directly from the table under Satisfying a type constraint.
Not sure if the underlying issue the same though.

@griesemer griesemer self-assigned this Dec 28, 2022
@griesemer griesemer modified the milestones: Backlog, Go1.21, Go1.20 Dec 28, 2022
@ianlancetaylor
Copy link
Contributor

I got confused earlier and missed the fact that we are instantiating G2 with a type parameter with a constraint, not with that constraint. I missed that even though it was the point of the issue. Sorry.

I agree that F2 should not compile today.

@griesemer
Copy link
Contributor

@rittneje The code (your example) https://go.dev/play/p/bqahFnk1xbV?v=gotip

package main

func foo[T comparable](t T) {}

func main() {
	_ = foo[struct{ f any }]
	_ = foo[any]
	_ = foo[[0]any]
}

compiles w/o errors when running locally with the latest version (go version devel go1.20-9123221ccf Wed Dec 28 15:34:23 2022 +0000 darwin/arm64). Not sure why the playground at tip behaves differently.

@griesemer
Copy link
Contributor

In the first example, it is correct that

func F1[V [2]any](v V) {
	_ = G1[V] // error: V does not implement comparable
}

V doesn't implement comparable: the array [2]any is not strictly comparable, V is a type parameter, and therefore V does not implement comparable (also _ = v == v is not permitted in F1).

I think the handling of F2 is incorrect, this is a bug in the type checker.

@griesemer
Copy link
Contributor

griesemer commented Dec 28, 2022

The type set of C2 should just be {int} but at the moment it is {[2]any, int} which is incorrect (it's the intersection of all strictly comparable types with the type set {[2]any, int}; [2]any is not strictly comparable and thus we should be left with int).

The intersection computation is using the wrong "comparable" predicate (not strictly comparable but "spec-comparable"). Fix forthcoming, later today.

@gopherbot
Copy link

Change https://go.dev/cl/459816 mentions this issue: go/types, types2: use strict comparability for type set intersection

@griesemer griesemer removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 28, 2022
@griesemer
Copy link
Contributor

This is now fixed (but not yet submitted). As @ianlancetaylor correctly noted, this should not have worked with Go 1.18 or Go 1.19.

@gopherbot please consider this for backport to 1.18 and 1.19. This is a compiler bug.

@gopherbot
Copy link

Backport issue(s) opened: #57498 (for 1.18), #57499 (for 1.19).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

@griesemer griesemer changed the title cmd/compile: incorrectly allow to use a type parameter as type argument (not very sure it is a bug or not) cmd/compile: incorrect type set intersection computation Dec 28, 2022
@dmitshur dmitshur added the NeedsFix The path to resolution is known, but the work has not been done. label Dec 28, 2022
@gopherbot
Copy link

Change https://go.dev/cl/460015 mentions this issue: [release-branch.go1.18] go/types, types2: use strict comparability for type set intersection

@gopherbot
Copy link

Change https://go.dev/cl/460016 mentions this issue: [release-branch.go1.19] go/types, types2: use strict comparability for type set intersection

@golang golang locked and limited conversation to collaborators Dec 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Development

No branches or pull requests

8 participants