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

dev.go2go: translator generates incorrect code for type switches, leading to duplicate cases #42758

Open
tdakkota opened this issue Nov 21, 2020 · 8 comments

Comments

@tdakkota
Copy link

@tdakkota tdakkota commented Nov 21, 2020

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

$ go version
go version devel +165ceb09f9 Tue Nov 3 12:31:40 2020 -0500 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
  • go2go playground

What did you do?

https://go2goplay.golang.org/p/dTkkesE4GCX.
Error is printing when go1 compiler tries to compile translated code.
Also I ran on dev.typeparams

go tool compile -G ./issue.go

where issue.go is code from playground

Here instantiated createRequest[io.Writer] look something like:

func createRequest_ioWriter(w io.Writer) {
	switch (interface{})(w).(type) {
	case io.Writer:
		fmt.Println("W")
	case io.Writer:
		fmt.Println("io.Writer")
	}

}

What did you expect to see

An error from go2go like type parameter cannot be case of type switch.

What did you see instead?

Code passes go2go and dev.typeparams type checker.

@tdakkota tdakkota changed the title dev.typeparams, dev.go2go: type switch with generic type case can be a duplicate dev.typeparams, dev.go2go: type checker permits type switch with generic type case which can be a duplicate Nov 21, 2020
@tdakkota
Copy link
Author

@tdakkota tdakkota commented Nov 23, 2020

@griesemer
Copy link
Contributor

@griesemer griesemer commented Nov 24, 2020

The typeparams draft doesn't disallow type parameters in type switch cases (or type assertions for that matter). A type parameter is a type like any other, so it's not obvious that this should be prohibited.

At the moment I consider this a limitation of the source-to-source translator (prototype).

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Nov 24, 2020

I agree, but I don't plan to do any more work on this experimental tool. If anybody else wants to send a fix, that would be fine.

@gopherbot
Copy link

@gopherbot gopherbot commented Nov 25, 2020

Change https://golang.org/cl/273127 mentions this issue: [dev.go2go] go/types, cmd/compile/internal/types2: a type parameter is a valid type case in a type switch

gopherbot pushed a commit that referenced this issue Nov 25, 2020
…s a valid type case in a type switch

Likewise for type assertions.

Updates #42758.

Change-Id: Iea727639f348c6997b4d3aaba420ea242e985002
Reviewed-on: https://go-review.googlesource.com/c/go/+/273127
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
@gopherbot
Copy link

@gopherbot gopherbot commented Nov 25, 2020

Change https://golang.org/cl/273128 mentions this issue: [dev.typeparams] cmd/compile/internal/types2: a type parameter is a valid type case in a type switch

gopherbot pushed a commit that referenced this issue Nov 25, 2020
…alid type case in a type switch

Likewise for type assertions.

This is a port of https://golang.org/cl/273127 to dev.typeparams.

Updates #42758.

Change-Id: If93246371c3555e067b0043f0caefaac99101ebc
Reviewed-on: https://go-review.googlesource.com/c/go/+/273128
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
@griesemer
Copy link
Contributor

@griesemer griesemer commented Nov 25, 2020

Just for the record: The CLs above don't address this issue - they only clean up related handling of type parameters in type switches (which are carried forward into the dev.typeparams branch).

@griesemer griesemer changed the title dev.typeparams, dev.go2go: type checker permits type switch with generic type case which can be a duplicate dev.go2go: translator generates incorrect code for type switches, leading to duplicate cases Nov 25, 2020
@tdakkota
Copy link
Author

@tdakkota tdakkota commented Nov 26, 2020

The typeparams draft doesn't disallow type parameters in type switch cases (or type assertions for that matter). A type parameter is a type like any other, so it's not obvious that this should be prohibited.

I didn't fully understand,

var t io.Writer
switch t.(type) {
case io.Writer:
   return
case W:
    panic("W")
}

what behavior is expected when W is io.Writer?

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Nov 26, 2020

The first case should be chosen, and the function should return.

We recently updated the design draft for this. See https://go.googlesource.com/proposal/+/refs/heads/master/design/go2draft-type-parameters.md#generic-types-as-type-switch-cases .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants