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: does not match inferred type #60460

Closed
johejo opened this issue May 26, 2023 · 7 comments
Closed

cmd/compile: does not match inferred type #60460

johejo opened this issue May 26, 2023 · 7 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

@johejo
Copy link

johejo commented May 26, 2023

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

$ go version
go version devel go1.21-f89d575d9e Fri May 26 13:43:41 2023 +0000 linux/amd64

Does this issue reproduce with the latest release?

No, tip only.

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

go env Output
$ go env
GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/mitsuoheijo/.cache/go-build'
GOENV='/home/mitsuoheijo/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/mitsuoheijo/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/mitsuoheijo/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/mitsuoheijo/repos/github.com/golang/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/home/mitsuoheijo/repos/github.com/golang/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='devel go1.21-f89d575d9e Fri May 26 13:43:41 2023 +0000'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/mitsuoheijo/repos/github.com/johejo/sandbox/go.mod'
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-build1238354243=/tmp/go-build -gno-record-gcc-switches'

What did you do?

https://go.dev/play/p/3wuGsYeb_DI

What did you expect to see?

compile succeeds

What did you see instead?

./prog.go:5:9: type Rules[int] of r does not match inferred type Rules[int] for Rules[T]
./prog.go:7:9: type Rules[string] of r2 does not match inferred type Rules[string] for Rules[T]
./prog.go:21:16: type Rules[T] of s.rules does not match inferred type Rules[T] for Rules[T]

Might be related #60377 and 1dd24d8
Same error occurs in https://github.com/zclconf/go-cty with tip.

I couldn't verbalize the title of the issue well. I would appreciate if someone could correct it.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label May 26, 2023
@ianlancetaylor
Copy link
Contributor

CC @griesemer @findleyr

@ianlancetaylor
Copy link
Contributor

The test cases compiles and runs with Go 1.20, but not with current HEAD. It does build with a compiler of a week or two ago. Also the error messages don't make a great deal of sense. Marking as a release blocker.

@ianlancetaylor ianlancetaylor added this to the Go1.21 milestone May 26, 2023
@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 26, 2023
@griesemer griesemer self-assigned this May 26, 2023
@griesemer
Copy link
Contributor

Simplified reproducer:

package p

func _() {
	f(R1{})
	f(R2{})
}

type S[T any] struct {
	rules R[T]
}

func f[T any](rules R[T]) {}

func (s S[T]) _() {
	f(s.rules)
}

type R[T any] interface {
	m(R[T])
}

type R1 struct{}

func (R1) m(R[int]) {}

type R2 struct{}

func (R2) m(R[string]) {}

This is related to CL 498315 (submitted yesterday) which addressed issue #60377 (which I believe was a bug).

Thus, I am not convinced that the code here is correct. Will think some more over the next few days.
Reverting CL 498315 will resolve this case, but let's not do it just yet.

@griesemer griesemer added NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels May 26, 2023
@griesemer
Copy link
Contributor

Reduced to one (representative) error:

package p

func _() {
	f(R1{})
}

func f[T any](R[T]) {}

type R[T any] interface {
	m(R[T])
}

type R1 struct{}

func (R1) m(R[int]) {}

@griesemer
Copy link
Contributor

griesemer commented May 26, 2023

Analysis: We're ending up in an endless recursion because we don't use exact unification when comparing component types (e.g. signature elements in this case).

== infer : [T₃](R[T₃]) ➞ [<nil>]
== function parameters: (p.R[T₃])
-- function arguments : [R1{} (value of type p.R1)]
.  p.R[T₃] ≡ p.R1
.  p.R1 ≡ p.R[T₃] (swap)
.  .  p.R1 ≡ interface{m(p.R[T₃])}
.  .  interface{m(p.R[T₃])} ≡ p.R1 (swap)
.  .  .  func(p.R[T₃]) ≡ func(p.R[int])   <<< FIRST OCCURRENCE
.  .  .  .  (p.R[T₃]) ≡ (p.R[int])
.  .  .  .  .  p.R[T₃] ≡ p.R[int]
.  .  .  .  .  p.R[int] ≡ p.R[T₃] (swap)
.  .  .  .  .  .  interface{m(p.R[int])} ≡ interface{m(p.R[T₃])}
.  .  .  .  .  .  .  func(p.R[int]) ≡ func(p.R[T₃])
.  .  .  .  .  .  .  .  (p.R[int]) ≡ (p.R[T₃])
.  .  .  .  .  .  .  .  .  p.R[int] ≡ p.R[T₃]
.  .  .  .  .  .  .  .  .  p.R[T₃] ≡ p.R[int] (swap)
.  .  .  .  .  .  .  .  .  .  interface{m(p.R[T₃])} ≡ interface{m(p.R[int])}
.  .  .  .  .  .  .  .  .  .  .  func(p.R[T₃]) ≡ func(p.R[int])   <<< SEEN ABOVE

It would be good to fix the underlying problem here rather than roll back CL 498315.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/498895 mentions this issue: go/types, types2: use exact unification for component types

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/498955 mentions this issue: go/types, types2: add unifyMode to unifier, pass it through

@griesemer griesemer added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels May 27, 2023
gopherbot pushed a commit that referenced this issue May 30, 2023
This change defines two unification modes used to control unification:

- assign  set when unifying types involved in an assignment
- exact   if set, types unify if they can be made identical

Currently, unification is inexact: when a defined type is compared
against a type literal, the underlying type of the defined type is
considered. When channel types are compared, the channel direction
is ignored. And when defined types are compared where one (or both)
are interfaces, interface unification is used.

By contrast, exact unification requires types to match exactly:
if they can be unified, the types must be identical (with suitable
type arguments).

Exact unification is required when comparing component types.
For instance, when unifying func(x P) with func(x Q), the two
signatures unify only if P is identical to Q per Go's assignment
rules.

Until now we have ignored exact unification and made due with inexact
unification everywhere, even for component types. In some cases this
led to infinite recursions in the unifier, which we guarded against
with a depth limit (and unification failure).

Go's assignmemt rules allow inexact matching at the top-level but
require exact matching for element types.

This change passes 'assign' to the unifier when unifying parameter
against argument types because those follow assignment rules.
When comparing constraints, inexact unification is used as before.

In 'assign' mode, when comparing element types, the unifyier is
called recursively, this time with the 'exact' mode set, causing
element types to be compared exactly. If unification succeeds for
element types, they are identical (with suitable type arguments).

This change fixes #60460. It also fixes a bug in the test for
issue #60377. We also don't need to rely anymore on the recursion
depth limit (a temporary fix) for #59740. Finally, because we use
exact unification when comparing element types which are channels,
errors caused by assignment failures (due to inexact inference which
succeeded when it shouldn't have) now produce the correct inference
error.

Fixes #60460.
For #60377.
For #59740.

Change-Id: Icb6a9b4dbd34294f99328a06d52135cb499cab85
Reviewed-on: https://go-review.googlesource.com/c/go/+/498895
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators May 29, 2024
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

4 participants