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: panic compiling type switch with multiple nil cases #15898

Closed
josharian opened this issue May 30, 2016 · 9 comments
Closed

cmd/compile: panic compiling type switch with multiple nil cases #15898

josharian opened this issue May 30, 2016 · 9 comments
Labels
FrozenDueToAge
Milestone
Go1.7

Comments

@josharian
Copy link
Contributor 

package p

func f(e interface{}) {
    switch e.(type) {
    case nil, nil:
    }
}
$ go tool compile x.go
x.go:4: internal compiler error: typeSwitch with bad kind: 3

goroutine 1 [running]:
runtime/debug.Stack(0x0, 0x0, 0x0)
    runtime/debug/stack.go:24 +0x79
cmd/compile/internal/gc.Fatalf(0x4fbc90, 0x1c, 0xc42042f548, 0x1, 0x1)
    cmd/compile/internal/gc/subr.go:165 +0x15d
cmd/compile/internal/gc.(*typeSwitch).walk(0xc420430000, 0xc420420480)
    cmd/compile/internal/gc/swt.go:622 +0x7f1
cmd/compile/internal/gc.walkswitch(0xc420420480)
    cmd/compile/internal/gc/swt.go:202 +0x79
cmd/compile/internal/gc.walkstmt(0xc420420480, 0x49a240)
    cmd/compile/internal/gc/walk.go:358 +0xed7
cmd/compile/internal/gc.walkstmtlist(0xc420030790, 0x1, 0x1)
    cmd/compile/internal/gc/walk.go:80 +0x4d
cmd/compile/internal/gc.walk(0xc4204201b0)
    cmd/compile/internal/gc/walk.go:65 +0x1e4
cmd/compile/internal/gc.compile(0xc4204201b0)
    cmd/compile/internal/gc/pgen.go:391 +0x1d4
cmd/compile/internal/gc.funccompile(0xc4204201b0)
    cmd/compile/internal/gc/dcl.go:1287 +0x186
cmd/compile/internal/gc.Main()
    cmd/compile/internal/gc/main.go:465 +0x19af
cmd/compile/internal/amd64.Main()
    cmd/compile/internal/amd64/galign.go:93 +0x2fa
main.main()
    cmd/compile/main.go:33 +0x2a3

This is a regression from 1.6, probably caused by CL 19814. With Go 1.6 and before, it compiles quietly.

I'd like to make duplicate nil case clauses a compile time error. If that is ok (see #15896), then I can pull together a CL. If we want a smaller, less behavior-changing patch, I might leave this for @randall77.
@josharian josharian added this to the Go1.7 milestone May 30, 2016
@josharian
Copy link
Contributor Author

https://go-review.googlesource.com/#/c/19813/1/src/cmd/compile/internal/gc/swt.go@609 comes back to haunt/save us. :)

@randall77
Copy link
Contributor

The fix if duplicate nils are ok is pretty easy. Change the if to a for in:

    if len(cc) > 0 && cc[0].typ == caseKindTypeNil {
        typenil = cc[0].node.Right
        cc = cc[1:]
    }

We might want some sort of condition around the typenil assignment to make sure the first nil clause wins (we do want that, right?).

@gopherbot
Copy link

CL https://golang.org/cl/23573 mentions this issue.

@josharian
Copy link
Contributor Author

josharian commented May 31, 2016 •
edited

https://go-review.googlesource.com/23573 for rejecting multiple nils.

If we allow multiple nils, then yes I think the first nil clause needs to win.

@griesemer
Copy link
Contributor

Nils are not constants and I don't think the spec says anything that says
multiple Nils are not permitted.
On Mon, May 30, 2016 at 5:01 PM Josh Bleecher Snyder <
notifications@github.com> wrote:

https://go-review.googlesource.com/23573 for rejecting multiple nils.

If we allow multiple nils, then yes I think the first nil clause needs to
win (and we may as well not even compile the others).

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#15898 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AIIkTyCuxW3SF-QqS5lYM9Y3hMGwM8CVks5qG3pegaJpZM4IqE5k
.

@josharian
Copy link
Contributor Author

Ok. I'll update my CL tomorrow.

@gopherbot
Copy link

CL https://golang.org/cl/23580 mentions this issue.

@griesemer
Copy link
Contributor

@josharian Per a discussion with @randall77 and @ianlancetaylor just now, we agreed that we should disallow duplicate nils. See also my comment in #15896.

@josharian
Copy link
Contributor Author

OK. Will leave CL 23573 up for review as-is, then.

@golang golang locked and limited conversation to collaborators May 31, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.7
Development

No branches or pull requests
4 participants
@josharian @randall77 @griesemer @gopherbot