go.tools/go/types: implement label checks

R=adonovan
CC=golang-dev
https://golang.org/cl/14036046
This commit is contained in:
Robert Griesemer 2013-09-30 11:05:30 -07:00
parent 06c4192423
commit 3daa579643
10 changed files with 1010 additions and 12 deletions

View File

@ -121,8 +121,6 @@ type Info struct {
// in package clauses, blank identifiers on the lhs of assignments, or
// symbolic variables t in t := x.(type) of type switch headers), the
// corresponding objects are nil.
// BUG(gri) Label identifiers in break, continue, or goto statements
// are not yet mapped.
Objects map[*ast.Ident]Object
// Implicits maps nodes to their implicitly declared objects, if any.
@ -184,6 +182,5 @@ func IsAssignableTo(V, T Type) bool {
}
// BUG(gri): Some built-ins don't check parameters fully, yet (e.g. append).
// BUG(gri): Use of labels is only partially checked.
// BUG(gri): Interface vs non-interface comparisons are not correctly implemented.
// BUG(gri): Switch statements don't check duplicate cases for all types for which it is required.

View File

@ -66,6 +66,8 @@ var tests = [][]string{
{"testdata/conversions.src"},
{"testdata/stmt0.src"},
{"testdata/stmt1.src"},
{"testdata/gotos.src"},
{"testdata/labels.src"},
}
var fset = token.NewFileSet()

265
go/types/labels.go Normal file
View File

@ -0,0 +1,265 @@
// Copyright 2013 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
package types
import (
"go/ast"
"go/token"
)
// labels checks correct label use in body.
func (check *checker) labels(body *ast.BlockStmt) {
// set of all labels in this body
all := NewScope(nil)
fwdJumps := check.blockBranches(all, nil, nil, body.List)
// If there are any forward jumps left, no label was found for
// the corresponding goto statements. Either those labels were
// never defined, or they are inside blocks and not reachable
// for the respective gotos.
for _, jmp := range fwdJumps {
var msg string
name := jmp.Label.Name
if alt := all.Lookup(name); alt != nil {
msg = "goto %s jumps into block"
alt.(*Label).used = true // avoid another error
} else {
msg = "label %s not declared"
}
check.errorf(jmp.Label.Pos(), msg, name)
}
// spec: "It is illegal to define a label that is never used."
for _, obj := range all.elems {
if lbl := obj.(*Label); !lbl.used {
check.errorf(lbl.pos, "label %s declared but not used", lbl.name)
}
}
}
// A block tracks label declarations in a block and its enclosing blocks.
type block struct {
parent *block // enclosing block
lstmt *ast.LabeledStmt // labeled statement to which this block belongs, or nil
labels map[string]*ast.LabeledStmt // allocated lazily
}
// insert records a new label declaration for the current block.
// The label must not have been declared before in any block.
func (b *block) insert(s *ast.LabeledStmt) {
name := s.Label.Name
if debug {
assert(b.gotoTarget(name) == nil)
}
labels := b.labels
if labels == nil {
labels = make(map[string]*ast.LabeledStmt)
b.labels = labels
}
labels[name] = s
}
// gotoTarget returns the labeled statement in the current
// or an enclosing block with the given label name, or nil.
func (b *block) gotoTarget(name string) *ast.LabeledStmt {
for s := b; s != nil; s = s.parent {
if t := s.labels[name]; t != nil {
return t
}
}
return nil
}
// enclosingTarget returns the innermost enclosing labeled
// statement with the given label name, or nil.
func (b *block) enclosingTarget(name string) *ast.LabeledStmt {
for s := b; s != nil; s = s.parent {
if t := s.lstmt; t != nil && t.Label.Name == name {
return t
}
}
return nil
}
// blockBranches processes a block's statement list and returns the set of outgoing forward jumps.
// all is the scope of all declared labels, parent the set of labels declared in the immediately
// enclosing block, and lstmt is the labeled statement this block is associated with (or nil).
func (check *checker) blockBranches(all *Scope, parent *block, lstmt *ast.LabeledStmt, list []ast.Stmt) []*ast.BranchStmt {
b := &block{parent: parent, lstmt: lstmt}
var (
varDeclPos token.Pos
fwdJumps, badJumps []*ast.BranchStmt
)
// All forward jumps jumping over a variable declaration are possibly
// invalid (they may still jump out of the block and be ok).
// recordVarDecl records them for the given position.
recordVarDecl := func(pos token.Pos) {
varDeclPos = pos
badJumps = append(badJumps[:0], fwdJumps...) // copy fwdJumps to badJumps
}
jumpsOverVarDecl := func(jmp *ast.BranchStmt) bool {
if varDeclPos.IsValid() {
for _, bad := range badJumps {
if jmp == bad {
return true
}
}
}
return false
}
var stmtBranches func(ast.Stmt)
stmtBranches = func(s ast.Stmt) {
switch s := s.(type) {
case *ast.DeclStmt:
if d, _ := s.Decl.(*ast.GenDecl); d != nil && d.Tok == token.VAR {
recordVarDecl(d.Pos())
}
case *ast.LabeledStmt:
// declare label
name := s.Label.Name
lbl := NewLabel(s.Label.Pos(), name)
if alt := all.Insert(lbl); alt != nil {
check.errorf(lbl.pos, "label %s already declared", name)
check.reportAltDecl(alt)
// ok to continue
} else {
b.insert(s)
check.recordObject(s.Label, lbl)
}
// resolve matching forward jumps and remove them from fwdJumps
i := 0
for _, jmp := range fwdJumps {
if jmp.Label.Name == name {
// match
lbl.used = true
check.recordObject(jmp.Label, lbl)
if jumpsOverVarDecl(jmp) {
check.errorf(
jmp.Label.Pos(),
"goto %s jumps over variable declaration at line %d",
name,
check.fset.Position(varDeclPos).Line,
)
// ok to continue
}
} else {
// no match - record new forward jump
fwdJumps[i] = jmp
i++
}
}
fwdJumps = fwdJumps[:i]
lstmt = s
stmtBranches(s.Stmt)
case *ast.BranchStmt:
if s.Label == nil {
return // checked in 1st pass (check.stmt)
}
// determine and validate target
name := s.Label.Name
switch s.Tok {
case token.BREAK:
// spec: "If there is a label, it must be that of an enclosing
// "for", "switch", or "select" statement, and that is the one
// whose execution terminates."
t := b.enclosingTarget(name)
if t == nil {
check.errorf(s.Label.Pos(), "break label not declared: %s", name)
return
}
switch t.Stmt.(type) {
case *ast.SwitchStmt, *ast.TypeSwitchStmt, *ast.SelectStmt, *ast.ForStmt, *ast.RangeStmt:
// ok
default:
check.errorf(s.Label.Pos(), "invalid break label %s", name)
// continue and mark label as used to avoid another error
}
case token.CONTINUE:
// spec: "If there is a label, it must be that of an enclosing
// "for" statement, and that is the one whose execution advances."
t := b.enclosingTarget(name)
if t == nil {
check.errorf(s.Label.Pos(), "continue label not declared: %s", name)
return
}
switch t.Stmt.(type) {
case *ast.ForStmt, *ast.RangeStmt:
// ok
default:
check.errorf(s.Label.Pos(), "invalid continue label %s", name)
// continue and mark label as used to avoid another error
}
case token.GOTO:
if b.gotoTarget(name) == nil {
// label may be declared later - add to forward jumps
fwdJumps = append(fwdJumps, s)
return
}
default:
check.invalidAST(s.Pos(), "branch statement: %s %s", s.Tok, name)
return
}
// record label use
obj := all.Lookup(name)
obj.(*Label).used = true
check.recordObject(s.Label, obj)
case *ast.AssignStmt:
if s.Tok == token.DEFINE {
recordVarDecl(s.Pos())
}
case *ast.BlockStmt:
// Unresolved forward jumps inside the nested block
// become forward jumps in the current block.
fwdJumps = append(fwdJumps, check.blockBranches(all, b, lstmt, s.List)...)
case *ast.IfStmt:
stmtBranches(s.Body)
if s.Else != nil {
stmtBranches(s.Else)
}
case *ast.CaseClause:
fwdJumps = append(fwdJumps, check.blockBranches(all, b, nil, s.Body)...)
case *ast.SwitchStmt:
stmtBranches(s.Body)
case *ast.TypeSwitchStmt:
stmtBranches(s.Body)
case *ast.CommClause:
fwdJumps = append(fwdJumps, check.blockBranches(all, b, nil, s.Body)...)
case *ast.SelectStmt:
stmtBranches(s.Body)
case *ast.ForStmt:
stmtBranches(s.Body)
case *ast.RangeStmt:
stmtBranches(s.Body)
}
}
for _, s := range list {
stmtBranches(s)
}
return fwdJumps
}

View File

@ -385,6 +385,10 @@ func (check *checker) resolveFiles(files []*ast.File) {
// Note: funcList may grow while iterating through it - cannot use range clause.
for i := 0; i < len(check.funcList); i++ {
// TODO(gri) Factor out this code into a dedicated function
// with its own context so that it can be run concurrently
// eventually.
f := check.funcList[i]
if trace {
s := "<function literal>"
@ -400,7 +404,7 @@ func (check *checker) resolveFiles(files []*ast.File) {
check.stmtList(0, f.body.List)
if check.hasLabel {
// TODO(gri) check label use
check.labels(f.body)
}
if f.sig.results.Len() > 0 && !check.isTerminating(f.body, "") {

View File

@ -60,6 +60,21 @@ var sources = []string{
func (T) _() {}
func (T) _() {}
`,
`
package p
func _() {
L0:
L1:
goto L0
for {
goto L1
}
if true {
goto L2
}
L2:
}
`,
}
var pkgnames = []string{

View File

@ -117,8 +117,7 @@ func testTestDir(t *testing.T, path string, ignore ...string) {
func TestStdtest(t *testing.T) {
testTestDir(t, filepath.Join(runtime.GOROOT(), "test"),
"cmplxdivide.go", // also needs file cmplxdivide1.go - ignore
"goto.go", "label.go", "label1.go", // TODO(gri) implement missing label checks
"cmplxdivide.go", // also needs file cmplxdivide1.go - ignore
"mapnan.go", "sigchld.go", // don't work on Windows; testTestDir should consult build tags
"sizeof.go", "switch.go", // TODO(gri) tone down duplicate checking in expr switches
"typeswitch2.go", // TODO(gri) implement duplicate checking in type switches

View File

@ -243,7 +243,7 @@ func (check *checker) stmt(ctxt stmtContext, s ast.Stmt) {
case *ast.BranchStmt:
if s.Label != nil {
check.hasLabel = true
return // checks handled in separate pass
return // checked in 2nd pass (check.labels)
}
switch s.Tok {
case token.BREAK:
@ -254,14 +254,12 @@ func (check *checker) stmt(ctxt stmtContext, s ast.Stmt) {
if ctxt&inContinuable == 0 {
check.errorf(s.Pos(), "continue not in for statement")
}
case token.GOTO:
check.invalidAST(s.Pos(), "goto without label")
case token.FALLTHROUGH:
if ctxt&fallthroughOk == 0 {
check.errorf(s.Pos(), "fallthrough statement out of place")
}
default:
check.invalidAST(s.Pos(), "unknown branch statement (%s)", s.Tok)
check.invalidAST(s.Pos(), "branch statement: %s", s.Tok)
}
case *ast.BlockStmt:

560
go/types/testdata/gotos.src vendored Normal file
View File

@ -0,0 +1,560 @@
// Copyright 2011 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
// This file is a modified copy of $GOROOT/test/goto.go.
package gotos
var (
i, n int
x []int
c chan int
m map[int]int
s string
)
// goto after declaration okay
func _() {
x := 1
goto L
L:
_ = x
}
// goto before declaration okay
func _() {
goto L
L:
x := 1
_ = x
}
// goto across declaration not okay
func _() {
goto L /* ERROR "goto L jumps over variable declaration at line 36" */
x := 1
_ = x
L:
}
// goto across declaration in inner scope okay
func _() {
goto L
{
x := 1
_ = x
}
L:
}
// goto across declaration after inner scope not okay
func _() {
goto L /* ERROR "goto L jumps over variable declaration at line 58" */
{
x := 1
_ = x
}
x := 1
_ = x
L:
}
// goto across declaration in reverse okay
func _() {
L:
x := 1
_ = x
goto L
}
func _() {
L: L1:
x := 1
_ = x
goto L
goto L1
}
// error shows first offending variable
func _() {
goto L /* ERROR "goto L jumps over variable declaration at line 84" */
x := 1
_ = x
y := 1
_ = y
L:
}
// goto not okay even if code path is dead
func _() {
goto L /* ERROR "goto L jumps over variable declaration" */
x := 1
_ = x
y := 1
_ = y
return
L:
}
// goto into outer block okay
func _() {
{
goto L
}
L:
}
func _() {
{
goto L
goto L1
}
L: L1:
}
// goto backward into outer block okay
func _() {
L:
{
goto L
}
}
func _() {
L: L1:
{
goto L
goto L1
}
}
// goto into inner block not okay
func _() {
goto L /* ERROR "goto L jumps into block" */
{
L:
}
}
func _() {
goto L /* ERROR "goto L jumps into block" */
goto L1 /* ERROR "goto L1 jumps into block" */
{
L: L1:
}
}
// goto backward into inner block still not okay
func _() {
{
L:
}
goto L /* ERROR "goto L jumps into block" */
}
func _() {
{
L: L1:
}
goto L /* ERROR "goto L jumps into block" */
goto L1 /* ERROR "goto L1 jumps into block" */
}
// error shows first (outermost) offending block
func _() {
goto L /* ERROR "goto L jumps into block" */
{
{
{
L:
}
}
}
}
// error prefers block diagnostic over declaration diagnostic
func _() {
goto L /* ERROR "goto L jumps into block" */
x := 1
_ = x
{
L:
}
}
// many kinds of blocks, all invalid to jump into or among,
// but valid to jump out of
// if
func _() {
L:
if true {
goto L
}
}
func _() {
L:
if true {
goto L
} else {
}
}
func _() {
L:
if false {
} else {
goto L
}
}
func _() {
goto L /* ERROR "goto L jumps into block" */
if true {
L:
}
}
func _() {
goto L /* ERROR "goto L jumps into block" */
if true {
L:
} else {
}
}
func _() {
goto L /* ERROR "goto L jumps into block" */
if true {
} else {
L:
}
}
func _() {
if false {
L:
} else {
goto L /* ERROR "goto L jumps into block" */
}
}
func _() {
if true {
goto L /* ERROR "goto L jumps into block" */
} else {
L:
}
}
func _() {
if true {
goto L /* ERROR "goto L jumps into block" */
} else if false {
L:
}
}
func _() {
if true {
goto L /* ERROR "goto L jumps into block" */
} else if false {
L:
} else {
}
}
func _() {
if true {
goto L /* ERROR "goto L jumps into block" */
} else if false {
} else {
L:
}
}
func _() {
if true {
goto L /* ERROR "goto L jumps into block" */
} else {
L:
}
}
func _() {
if true {
L:
} else {
goto L /* ERROR "goto L jumps into block" */
}
}
// for
func _() {
for {
goto L
}
L:
}
func _() {
for {
goto L
L:
}
}
func _() {
for {
L:
}
goto L /* ERROR "goto L jumps into block" */
}
func _() {
for {
goto L
L1:
}
L:
goto L1 /* ERROR "goto L1 jumps into block" */
}
func _() {
for i < n {
L:
}
goto L /* ERROR "goto L jumps into block" */
}
func _() {
for i = 0; i < n; i++ {
L:
}
goto L /* ERROR "goto L jumps into block" */
}
func _() {
for i = range x {
L:
}
goto L /* ERROR "goto L jumps into block" */
}
func _() {
for i = range c {
L:
}
goto L /* ERROR "goto L jumps into block" */
}
func _() {
for i = range m {
L:
}
goto L /* ERROR "goto L jumps into block" */
}
func _() {
for i = range s {
L:
}
goto L /* ERROR "goto L jumps into block" */
}
// switch
func _() {
L:
switch i {
case 0:
goto L
}
}
func _() {
L:
switch i {
case 0:
default:
goto L
}
}
func _() {
switch i {
case 0:
default:
L:
goto L
}
}
func _() {
switch i {
case 0:
default:
goto L
L:
}
}
func _() {
switch i {
case 0:
goto L
L:
;
default:
}
}
func _() {
goto L /* ERROR "goto L jumps into block" */
switch i {
case 0:
L:
}
}
func _() {
goto L /* ERROR "goto L jumps into block" */
switch i {
case 0:
L:
;
default:
}
}
func _() {
goto L /* ERROR "goto L jumps into block" */
switch i {
case 0:
default:
L:
}
}
func _() {
switch i {
default:
goto L /* ERROR "goto L jumps into block" */
case 0:
L:
}
}
func _() {
switch i {
case 0:
L:
;
default:
goto L /* ERROR "goto L jumps into block" */
}
}
// select
// different from switch. the statement has no implicit block around it.
func _() {
L:
select {
case <-c:
goto L
}
}
func _() {
L:
select {
case c <- 1:
default:
goto L
}
}
func _() {
select {
case <-c:
default:
L:
goto L
}
}
func _() {
select {
case c <- 1:
default:
goto L
L:
}
}
func _() {
select {
case <-c:
goto L
L:
;
default:
}
}
func _() {
goto L /* ERROR "goto L jumps into block" */
select {
case c <- 1:
L:
}
}
func _() {
goto L /* ERROR "goto L jumps into block" */
select {
case c <- 1:
L:
;
default:
}
}
func _() {
goto L /* ERROR "goto L jumps into block" */
select {
case <-c:
default:
L:
}
}
func _() {
select {
default:
goto L /* ERROR "goto L jumps into block" */
case <-c:
L:
}
}
func _() {
select {
case <-c:
L:
;
default:
goto L /* ERROR "goto L jumps into block" */
}
}

144
go/types/testdata/labels.src vendored Normal file
View File

@ -0,0 +1,144 @@
// Copyright 2011 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
// This file is a modified concatenation of the files
// $GOROOT/test/label.go and $GOROOT/test/label1.go.
package labels
var x int
func f0() {
L1 /* ERROR "label L1 declared but not used" */ :
for {
}
L2 /* ERROR "label L2 declared but not used" */ :
select {
}
L3 /* ERROR "label L3 declared but not used" */ :
switch {
}
L4 /* ERROR "label L4 declared but not used" */ :
if true {
}
L5 /* ERROR "label L5 declared but not used" */ :
f0()
L6:
f0()
L6 /* ERROR "label L6 already declared" */ :
f0()
if x == 20 {
goto L6
}
L7:
for {
break L7
}
L8:
for {
if x == 21 {
continue L8
}
}
L9:
switch {
case true:
break L9
defalt /* ERROR "label defalt declared but not used" */ :
}
L10:
select {
default:
break L10
}
}
func f1() {
L1:
for {
if x == 0 {
break L1
}
if x == 1 {
continue L1
}
goto L1
}
L2:
select {
default:
if x == 0 {
break L2
}
if x == 1 {
continue L2 /* ERROR "invalid continue label L2" */
}
goto L2
}
L3:
switch {
case x > 10:
if x == 11 {
break L3
}
if x == 12 {
continue L3 /* ERROR "invalid continue label L3" */
}
goto L3
}
L4:
if true {
if x == 13 {
break L4 /* ERROR "invalid break label L4" */
}
if x == 14 {
continue L4 /* ERROR "invalid continue label L4" */
}
if x == 15 {
goto L4
}
}
L5:
f1()
if x == 16 {
break L5 /* ERROR "invalid break label L5" */
}
if x == 17 {
continue L5 /* ERROR "invalid continue label L5" */
}
if x == 18 {
goto L5
}
for {
if x == 19 {
break L1 /* ERROR "break label not declared: L1" */
}
if x == 20 {
continue L1 /* ERROR "continue label not declared: L1" */
}
if x == 21 {
goto L1
}
}
}
// Additional tests not in the original files.
func f2() {
L1:
if x == 0 {
for {
continue L1 /* ERROR "invalid continue label L1" */
}
}
}

View File

@ -378,16 +378,24 @@ func switches1() {
switch x {
case 0:
goto L1
L1: fallthrough
case 1:
goto L2
goto L3
goto L4
L2: L3: L4: fallthrough
default:
}
switch x {
case 0:
goto L5
L5: fallthrough
default:
goto L6
goto L7
goto L8
L6: L7: L8: fallthrough /* ERROR "fallthrough statement out of place" */
}
@ -605,14 +613,20 @@ func rangeloops() {
}
func labels0() {
goto L0
goto L1
L0:
L1:
L1:
L1 /* ERROR "already declared" */ :
if true {
goto L2
L2:
L0:
L0 /* ERROR "already declared" */ :
}
_ = func() {
goto L0
goto L1
goto L2
L0:
L1:
L2: