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

Jump to bottom

cmd/go2go: type checking for generic constraints doesn't work correctly #39754

Closed
tdakkota opened this issue Jun 22, 2020 · 9 comments
Closed

cmd/go2go: type checking for generic constraints doesn't work correctly #39754

tdakkota opened this issue Jun 22, 2020 · 9 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@tdakkota
Copy link

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

$ go version
go version devel +0a030888da Sat Jun 20 04:46:40 2020 +0000 windows/amd64

Does this issue reproduce with the latest release?

n/a

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

  • windows/amd64 with 0a03088
  • go2go playground

What did you do?

https://go2goplay.golang.org/p/QLrCSQoyvVb

What did you expect to see?

go2go's typechecker error for line 57.

What did you see instead?

Successful compilation.
@tdakkota
Copy link
Author

But

	_ = F2(string, Optional(int), Result(int))(OptionalFrom(int)(0), func(o Optional(int)) (r Result(int)) {
		return
	})

fails with

type checking failed for main
prog.go2:51:17: Optional(int) does not satisfy Box(V) (missing method Val)

@tdakkota
Copy link
Author

Another example
https://go2goplay.golang.org/p/Juv92RyNgwh

crashes on 0a03088 and runs on playground.

Partial stack trace:

runtime: goroutine stack exceeds 1000000000-byte limit
runtime: sp=0xc0201e1580 stack=[0xc0201e0000, 0xc0401e0000]
fatal error: stack overflow

runtime stack:
runtime.throw(0xd3743b, 0xe)
	G:/workspace/exprm/go/src/runtime/panic.go:1116 +0x79
runtime.newstack()
	G:/workspace/exprm/go/src/runtime/stack.go:1059 +0x80d
runtime.morestack()
	G:/workspace/exprm/go/src/runtime/asm_amd64.s:449 +0x97

goroutine 1 [running]:
go/types.(*Checker).completeInterface(0xc0001665a0, 0x0, 0xc00016e380)
	G:/workspace/exprm/go/src/go/types/typexpr.go:818 +0xaba fp=0xc0201e1590 sp=0xc0201e1588 pc=0xbc2a1a
go/types.(*TypeParam).Bound(0xc0001bc140, 0x0)
	G:/workspace/exprm/go/src/go/types/type.go:811 +0xe5 fp=0xc0201e15e0 sp=0xc0201e1590 pc=0xbb3185
go/types.optype(0xd6cdc0, 0xc0001bc140, 0x0, 0x0)
	G:/workspace/exprm/go/src/go/types/type.go:830 +0x66 fp=0xc0201e1648 sp=0xc0201e15e0 pc=0xbb3226
go/types.(*TypeParam).Interface(0xc0001bc140, 0x0)
	G:/workspace/exprm/go/src/go/types/type.go:848 +0x4c fp=0xc0201e1698 sp=0xc0201e1648 pc=0xbb38ac
go/types.(*TypeParam).Interface(0xc0001bc180, 0x0)
	G:/workspace/exprm/go/src/go/types/type.go:848 +0x6c fp=0xc0201e16e8 sp=0xc0201e1698 pc=0xbb38cc
go/types.(*TypeParam).Interface(0xc0001bc140, 0x0)
	G:/workspace/exprm/go/src/go/types/type.go:848 +0x6c fp=0xc0201e1738 sp=0xc0201e16e8 pc=0xbb38cc
go/types.(*TypeParam).Interface(0xc0001bc180, 0x0)
	G:/workspace/exprm/go/src/go/types/type.go:848 +0x6c fp=0xc0201e1788 sp=0xc0201e1738 pc=0xbb38cc
go/types.(*TypeParam).Interface(0xc0001bc140, 0x0)
	G:/workspace/exprm/go/src/go/types/type.go:848 +0x6c fp=0xc0201e17d8 sp=0xc0201e1788 pc=0xbb38cc
go/types.(*TypeParam).Interface(0xc0001bc180, 0x0)
...

@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 22, 2020
@cagedmantis cagedmantis added this to the Unreleased milestone Jun 22, 2020
@cagedmantis
Copy link
Contributor

/cc @griesemer @ianlancetaylor

@ianlancetaylor
Copy link
Contributor

Looks like the type checker fails to report an error.

Smaller test case:

package main

type Optional(type T) struct {
	p   T
	set bool
}

func (o Optional(T)) Val() (T, bool) {
	return o.p, true
}

type Box(type T) interface {
	Val() (T, bool)
}

func F1(type V interface{}, A, B Box(V))() {}

func main() {
	F1(int, Optional(int), Optional(string))()
}

This requires that both Optional(int) and Optional(string) implement Box(int). Optional(int) does implement it, but Optional(string) does not. The type checker passes this without error but probably should not.

@ianlancetaylor
Copy link
Contributor

@mmaedel I'm not sure what problem you are running into, but it's a different problem. Please open a separate issue. Thanks.

@mmaedel mmaedel mentioned this issue Jun 25, 2020
Closed
@tdakkota
Copy link
Author

tdakkota commented Jun 27, 2020 •
edited

https://go2goplay.golang.org/p/jmaTevSNW0X
Seems related. Type checker can't compare type of result of func New with I generic type

UPD.
This bug is related to optype function.
When type checker tries to assign result of func New to I, optype is called

func optype(typ Type) Type { // typ is I generic 
	if t := typ.TypeParam(); t != nil { // I is a type param
		if u := t.Bound().allTypes; u != nil && u != typ { // t bound only by method list, not by type list, so allTypes is nil
			return u.Under()
		}
		return theTop // here function returns
	}
	return typ
}

Tuple(A, B) becomes T top type, i.e. Any

tdakkota added a commit to tdakkota/go that referenced this issue Jun 28, 2020
@tdakkota
[dev.go2go] go/types: fix type check skipping for constraints when th…
bf418db 
…ere’s more than one generic constraint.

Fixes golang#39754.
@tdakkota tdakkota mentioned this issue Jun 28, 2020
Closed
@gopherbot
Copy link

Change https://golang.org/cl/240339 mentions this issue: [dev.go2go] go/types: fix type check skipping for constraints when th…

gopherbot pushed a commit that referenced this issue Jun 29, 2020
@tdakkota @griesemer
[dev.go2go] go/types: fix type check skipping for constraints when th…
71c7be5 
…ere’s more than one generic constraint.

Check of first constraint was breaking the loop, so other constraints was skipped

Fixes #39754.

Change-Id: I48caa380472371a438c2d2029d1383d03dbf22df
GitHub-Last-Rev: bf418db
GitHub-Pull-Request: #39902
Reviewed-on: https://go-review.googlesource.com/c/go/+/240339
Reviewed-by: Robert Griesemer <gri@golang.org>
@tdakkota
Copy link
Author

Seems to be fixed. Closing.

@tdakkota tdakkota mentioned this issue Jun 29, 2020
Closed
@griesemer griesemer mentioned this issue Jun 30, 2020
Closed
@gopherbot
Copy link

Change https://golang.org/cl/240478 mentions this issue: [dev.go2go] go/types: fix broken test case

gopherbot pushed a commit that referenced this issue Jun 30, 2020
@griesemer
[dev.go2go] go/types: fix broken test case
236ad5a 
While at it, simplified test case a bit.

Fixes #39935.
Updates #39754.

Change-Id: Ia3b51f23807d25e62113757c51a660c7e3a9b381
Reviewed-on: https://go-review.googlesource.com/c/go/+/240478
Reviewed-by: Robert Griesemer <gri@golang.org>
@golang golang locked and limited conversation to collaborators Jun 30, 2021
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
Unreleased
Development

No branches or pull requests
5 participants
@cagedmantis @ianlancetaylor @griesemer @gopherbot @tdakkota