-
Notifications
You must be signed in to change notification settings - Fork 18.4k
Description
I was just studying the syntax checker and parser and noticed that for label reuse errors in
go/src/cmd/compile/internal/syntax/branches.go
Lines 74 to 80 in 5c08b9e
name := s.Label.Value | |
labels := ls.labels | |
if labels == nil { | |
labels = make(map[string]*label) | |
ls.labels = labels | |
} else if alt := labels[name]; alt != nil { | |
ls.err(s.Pos(), "label %s already defined at %s", name, alt.lstmt.Label.Pos().String()) |
while printing the error, we use a label's parsed statement position, of which that position is until the last token that delimits a label aka ":" instead of actually where the label starts from. However, for the original label, we actually use the label's start position. I believe this issue might affect people whose code editors jump to line numbers/positions on errors.
A simple exhibit below https://play.golang.org/p/Oi-4doqbEN1 or inlined below:
package main
func main() {
foo:
foo:
}
gives the error
prog.go:4:1: label foo defined and not used
prog.go:5:4: label foo already defined at prog.go:4:1
but notice the error message
prog.go:5:4: label foo already defined at prog.go:4:1
the position for the first label is correctly reported as 4:1
but for the redefined one as 5:4
yet it should be 5:1
I am filling this issue for the purposes of asking if this is the intended behavior and if not, for purposes of tracking the bug. This issue is not a regression and exists also in Go1.10
The fix seems straightforward to me, which is to use s.Label.Pos()
instead of s.Pos()
aka
diff --git a/src/cmd/compile/internal/syntax/branches.go b/src/cmd/compile/internal/syntax/branches.go
index a03e2734d2..cdeec863bc 100644
--- a/src/cmd/compile/internal/syntax/branches.go
+++ b/src/cmd/compile/internal/syntax/branches.go
@@ -77,7 +77,7 @@ func (ls *labelScope) declare(b *block, s *LabeledStmt) *label {
labels = make(map[string]*label)
ls.labels = labels
} else if alt := labels[name]; alt != nil {
- ls.err(s.Pos(), "label %s already defined at %s", name, alt.lstmt.Label.Pos().String())
+ ls.err(s.Label.Pos(), "label %s already defined at %s", name, alt.lstmt.Label.Pos())
return alt
}
l := &label{b, s, false}
which will then correctly print
$ go tool compile branches.go
prog.go:4:1: label foo defined and not used
prog.go:5:1: label foo already defined at prog.go:4:1
/cc @griesemer