Navigation Menu

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

go/types: "duplicate case in expression switch" error could be formatted better (perhaps) #26357

Closed
mvdan opened this issue Jul 12, 2018 · 7 comments
Closed

go/types: "duplicate case in expression switch" error could be formatted better (perhaps) #26357

mvdan opened this issue Jul 12, 2018 · 7 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone
Unplanned

Comments

@mvdan
Copy link
Member

mvdan commented Jul 12, 2018 

$ go version
go version devel +3df1f00bfc Thu Jul 12 08:22:37 2018 +0000 linux/amd64
$ cd src/go/types
$ go build gotype.go
$ cat f.go
package p

func f(r rune) {
        switch r {
        case 'a':
        case 'a':
        }
}
$ ./gotype f.go
f.go:6:7: duplicate case 'a' (constant 97 of type rune) in expression switch
f.go:5:7:       previous case

Haven't digged into why this error appears to be broken.

/cc @griesemer
@mvdan mvdan added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 12, 2018
@mvdan mvdan added this to the Go1.12 milestone Jul 12, 2018
@mvdan
Copy link
Member Author

mvdan commented Jul 12, 2018

Forgot to mention - the compiler gets this right, as expected:

$ go build f.go
# command-line-arguments
./f.go:6:2: duplicate case 'a' (value 97) in switch
        previous case at ./f.go:5:7

@ianlancetaylor
Copy link
Contributor

Sorry, I don't understand what you are after here. The go/types error does say "previous case" at f.go:5:7. What should it do instead?

@mvdan
Copy link
Member Author

mvdan commented Jul 12, 2018

Oh, the position is at the left. I completely missed that, even after reading the error multiple times.

Maybe that's just me, or maybe the error could be made clearer. I definitely find the compiler's version easier to understand.

@ianlancetaylor
Copy link
Contributor

The advantage of putting the position at the left is that it works well with tools, such as editors, which can cycle through the locations. Maybe we can improve the "previous case" text to make that clearer.

@griesemer
Copy link
Contributor

What @ianlancetaylor said.

@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 Aug 8, 2018
@griesemer griesemer modified the milestones: Go1.12, Unplanned Aug 8, 2018
@griesemer griesemer changed the title go/types: missing previous case in "duplicate case in expression switch" error go/types: "duplicate case in expression switch" error could be formatted better (perhaps) Aug 8, 2018
@ALTree
Copy link
Member

ALTree commented Sep 14, 2020

Current error message is

test.go:6:7: duplicate case 'a' (value 97) in switch
        previous case at test.go:5:7

So the 2nd line has been clarified, but the position is no longer of the left.

@griesemer
Copy link
Contributor

I think this ok as is. The 2nd line is really just clarifying the error, it's not another error. Closing as working as intended.

@golang golang locked and limited conversation to collaborators Sep 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
5 participants
@ianlancetaylor @mvdan @ALTree @griesemer @gopherbot