1
0
mirror of https://github.com/golang/go synced 2024-10-01 03:38:32 -06:00

go.tools/go/types: clean up assignment checks (round 1)

Various bug fixes:
- don't allow := to redeclare non-variables
- don't permit a comma-ok expression as a two-value function return

Lots of dead code removed.

Fixes golang/go#5500.

R=adonovan
CC=golang-dev
https://golang.org/cl/10792044
This commit is contained in:
Robert Griesemer 2013-07-08 09:40:30 -07:00
parent a82eaff6b7
commit cc52b8b7f8
7 changed files with 198 additions and 266 deletions

View File

@ -280,7 +280,7 @@ func BitLen(x Value) int {
// Sign returns -1, 0, or 1 depending on whether
// x < 0, x == 0, or x > 0. For complex values z,
// the sign is 0 if z == 0, otherwise it is != 0.
// The result is 0 for unknown values.
// The result is 1 for unknown values.
func Sign(x Value) int {
switch x := x.(type) {
case int64Val:
@ -298,7 +298,7 @@ func Sign(x Value) int {
case complexVal:
return x.re.Sign() | x.im.Sign()
case unknownVal:
return 0
return 1 // avoid spurious division by zero errors
}
panic(fmt.Sprintf("invalid Sign(%v)", x))
}

View File

@ -10,90 +10,92 @@ import (
"code.google.com/p/go.tools/go/exact"
)
// TODO(gri) initialize is very close to the 2nd half of assign1to1.
func (check *checker) assign(obj Object, x *operand) {
// Determine typ of lhs: If the object doesn't have a type
// yet, determine it from the type of x; if x is invalid,
// set the object type to Typ[Invalid].
var typ Type
switch obj := obj.(type) {
default:
unreachable()
func (check *checker) initConst(lhs *Const, x *operand) {
lhs.val = exact.MakeUnknown()
case *Const:
typ = obj.typ // may already be Typ[Invalid]
if typ == nil {
typ = Typ[Invalid]
if x.mode != invalid {
typ = x.typ
}
obj.typ = typ
}
case *Var:
typ = obj.typ // may already be Typ[Invalid]
if typ == nil {
typ = Typ[Invalid]
if x.mode != invalid {
typ = x.typ
if isUntyped(typ) {
// convert untyped types to default types
if typ == Typ[UntypedNil] {
check.errorf(x.pos(), "use of untyped nil")
typ = Typ[Invalid]
} else {
typ = defaultType(typ)
}
}
}
obj.typ = typ
if x.mode == invalid || x.typ == Typ[Invalid] || lhs.typ == Typ[Invalid] {
if lhs.typ == nil {
lhs.typ = Typ[Invalid]
}
return // nothing else to check
}
// nothing else to check if we don't have a valid lhs or rhs
if typ == Typ[Invalid] || x.mode == invalid {
return
// If the lhs doesn't have a type yet, use the type of x.
if lhs.typ == nil {
lhs.typ = x.typ
}
if !check.assignment(x, typ) {
if !check.assignment(x, lhs.typ) {
if x.mode != invalid {
if x.typ != Typ[Invalid] && typ != Typ[Invalid] {
check.errorf(x.pos(), "cannot initialize %s (type %s) with %s", obj.Name(), typ, x)
}
check.errorf(x.pos(), "cannot define constant %s (type %s) as %s", lhs.Name(), lhs.typ, x)
}
return
}
// for constants, set their value
if obj, _ := obj.(*Const); obj != nil {
obj.val = exact.MakeUnknown() // failure case: we don't know the constant value
if x.mode == constant {
if isConstType(x.typ) {
obj.val = x.val
} else if x.typ != Typ[Invalid] {
check.errorf(x.pos(), "%s has invalid constant type", x)
// rhs must be a constant
if x.mode != constant {
check.errorf(x.pos(), "%s is not constant", x)
return
}
// rhs type must be a valid constant type
if !isConstType(x.typ) {
check.errorf(x.pos(), "%s has invalid constant type", x)
return
}
lhs.val = x.val
}
func (check *checker) initVar(lhs *Var, x *operand) {
typ := x.typ
if x.mode == invalid || typ == Typ[Invalid] || lhs.typ == Typ[Invalid] {
if lhs.typ == nil {
lhs.typ = Typ[Invalid]
}
return // nothing else to check
}
// If the lhs doesn't have a type yet, use the type of x.
if lhs.typ == nil {
if isUntyped(typ) {
// convert untyped types to default types
if typ == Typ[UntypedNil] {
check.errorf(x.pos(), "use of untyped nil")
lhs.typ = Typ[Invalid]
return // nothing else to check
}
} else if x.mode != invalid {
check.errorf(x.pos(), "%s is not constant", x)
typ = defaultType(typ)
}
lhs.typ = typ
}
if !check.assignment(x, lhs.typ) {
if x.mode != invalid {
check.errorf(x.pos(), "cannot initialize variable %s (type %s) with %s", lhs.Name(), lhs.typ, x)
}
}
}
func (check *checker) assignMulti(lhs []Object, rhs []ast.Expr) {
func invalidateVars(list []*Var) {
for _, obj := range list {
if obj.typ == nil {
obj.typ = Typ[Invalid]
}
}
}
func (check *checker) initVars(lhs []*Var, rhs []ast.Expr, allowCommaOk bool) {
assert(len(lhs) > 0)
const decl = false
// If the lhs and rhs have corresponding expressions, treat each
// matching pair as an individual pair.
// If the lhs and rhs have corresponding expressions,
// treat each matching pair as an individual pair.
if len(lhs) == len(rhs) {
var x operand
for i, e := range rhs {
check.expr(&x, e)
if x.mode == invalid {
goto Error
}
check.assign(lhs[i], &x)
check.initVar(lhs[i], &x)
}
return
}
@ -108,49 +110,85 @@ func (check *checker) assignMulti(lhs []Object, rhs []ast.Expr) {
var x operand
check.expr(&x, rhs[0])
if x.mode == invalid {
goto Error
invalidateVars(lhs)
return
}
if t, ok := x.typ.(*Tuple); ok && len(lhs) == t.Len() {
// function result
x.mode = value
for i := 0; i < len(lhs); i++ {
obj := t.At(i)
x.expr = nil // TODO(gri) should do better here
x.typ = obj.typ
check.assign(lhs[i], &x)
for i, lhs := range lhs {
x.mode = value
x.expr = rhs[0]
x.typ = t.At(i).typ
check.initVar(lhs, &x)
}
return
}
if x.mode == valueok && len(lhs) == 2 {
if allowCommaOk && x.mode == valueok && len(lhs) == 2 {
// comma-ok expression
x.mode = value
check.assign(lhs[0], &x)
check.initVar(lhs[0], &x)
x.mode = value
x.typ = Typ[UntypedBool]
check.assign(lhs[1], &x)
check.initVar(lhs[1], &x)
return
}
}
check.errorf(lhs[0].Pos(), "assignment count mismatch: %d = %d", len(lhs), len(rhs))
// lhs variables may be function parameters (return assignment);
// use rhs position information for properly located error messages
check.errorf(rhs[0].Pos(), "assignment count mismatch: %d = %d", len(lhs), len(rhs))
invalidateVars(lhs)
}
Error:
// In case of a declaration, set all lhs types to Typ[Invalid].
for _, obj := range lhs {
switch obj := obj.(type) {
case *Const:
if obj.typ == nil {
obj.typ = Typ[Invalid]
func (check *checker) shortVarDecl(lhs, rhs []ast.Expr) {
scope := check.topScope
// collect lhs variables
vars := make([]*Var, len(lhs))
for i, lhs := range lhs {
var obj *Var
if ident, _ := lhs.(*ast.Ident); ident != nil {
// Use the correct obj if the ident is redeclared. The
// variable's scope starts after the declaration; so we
// must use Scope.Lookup here and call Scope.Insert later.
if alt := scope.Lookup(nil, ident.Name); alt != nil {
// redeclared object must be a variable
if alt, _ := alt.(*Var); alt != nil {
obj = alt
} else {
check.errorf(lhs.Pos(), "cannot assign to %s", lhs)
}
} else {
// declare new variable
obj = NewVar(ident.Pos(), check.pkg, ident.Name, nil)
}
obj.val = exact.MakeUnknown()
case *Var:
if obj.typ == nil {
obj.typ = Typ[Invalid]
}
default:
unreachable()
check.callIdent(ident, obj) // obj may be nil
} else {
check.errorf(lhs.Pos(), "cannot declare %s", lhs)
}
if obj == nil {
obj = NewVar(lhs.Pos(), check.pkg, "_", nil) // dummy variable
}
vars[i] = obj
}
check.initVars(vars, rhs, true)
// declare variables
n := 0 // number of new variables
for _, obj := range vars {
if obj.name == "_" {
obj.setParent(scope)
continue // blank identifiers are not visible
}
if scope.Insert(obj) == nil {
n++ // new declaration
}
}
if n == 0 {
check.errorf(vars[0].Pos(), "no new variables on left side of :=")
}
}

View File

@ -29,22 +29,6 @@ func (check *checker) declare(scope *Scope, id *ast.Ident, obj Object) {
}
}
func (check *checker) declareShort(scope *Scope, list []Object) {
n := 0 // number of new objects
for _, obj := range list {
if obj.Name() == "_" {
obj.setParent(scope)
continue // blank identifiers are not visible
}
if scope.Insert(obj) == nil {
n++ // new declaration
}
}
if n == 0 {
check.errorf(list[0].Pos(), "no new variables on left side of :=")
}
}
// A decl describes a package-level const, type, var, or func declaration.
type decl struct {
file *Scope // scope of file containing this declaration
@ -393,7 +377,7 @@ func (check *checker) declareConst(obj *Const, typ, init ast.Expr) {
goto Error
}
check.assign(obj, &x)
check.initConst(obj, &x)
check.iota = nil
return
@ -448,7 +432,7 @@ func (check *checker) declareVar(obj *Var, typ, init ast.Expr) {
for i, lhs := range proj.lhs {
x.expr = nil // TODO(gri) should do better here
x.typ = t.At(i).typ
check.assign(lhs, &x)
check.initVar(lhs, &x)
}
return
}
@ -456,10 +440,10 @@ func (check *checker) declareVar(obj *Var, typ, init ast.Expr) {
if x.mode == valueok && len(proj.lhs) == 2 {
// comma-ok expression
x.mode = value
check.assign(proj.lhs[0], &x)
check.initVar(proj.lhs[0], &x)
x.typ = Typ[UntypedBool]
check.assign(proj.lhs[1], &x)
check.initVar(proj.lhs[1], &x)
return
}
@ -468,7 +452,7 @@ func (check *checker) declareVar(obj *Var, typ, init ast.Expr) {
goto Error
}
check.assign(obj, &x)
check.initVar(obj, &x)
return
Error:

View File

@ -119,10 +119,12 @@ func (s *Scope) LookupParent(name string) Object {
// If s already contains an object with the same package path
// and name, Insert leaves s unchanged and returns that object.
// Otherwise it inserts obj, sets the object's scope to s, and
// returns nil.
// returns nil. The object must not have the blank _ name.
//
func (s *Scope) Insert(obj Object) Object {
if alt := s.Lookup(obj.Pkg(), obj.Name()); alt != nil {
name := obj.Name()
assert(name != "_")
if alt := s.Lookup(obj.Pkg(), name); alt != nil {
return alt
}
s.entries = append(s.entries, obj)

View File

@ -9,8 +9,6 @@ package types
import (
"go/ast"
"go/token"
"code.google.com/p/go.tools/go/exact"
)
// assigment reports whether x can be assigned to a variable of type 'to',
@ -36,14 +34,15 @@ func (check *checker) assignment(x *operand, to Type) bool {
return x.mode != invalid && x.isAssignable(check.ctxt, to)
}
// TODO(gri) assign1to1 is only used in one place now with decl = true.
// Remove and consolidate with other assignment functions.
// assign1to1 typechecks a single assignment of the form lhs = rhs (if rhs != nil), or
// lhs = x (if rhs == nil). If decl is set, the lhs expression must be an identifier;
// if its type is not set, it is deduced from the type of x or set to Typ[Invalid] in
// case of an error.
//
func (check *checker) assign1to1(lhs, rhs ast.Expr, x *operand, decl bool, isConst bool) {
assert(!isConst || decl)
func (check *checker) assign1to1(lhs, rhs ast.Expr, x *operand, decl bool) {
// Start with rhs so we have an expression type
// for declarations with implicit type.
if x == nil {
@ -91,52 +90,25 @@ func (check *checker) assign1to1(lhs, rhs ast.Expr, x *operand, decl bool, isCon
return
}
// Determine typ of lhs: If the object doesn't have a type
// yet, determine it from the type of x; if x is invalid,
// set the object type to Typ[Invalid].
var obj Object
var typ Type
if isConst {
obj = NewConst(ident.Pos(), check.pkg, ident.Name, nil, nil)
} else {
obj = NewVar(ident.Pos(), check.pkg, ident.Name, nil)
}
// Determine the type of lhs from the type of x;
// if x is invalid, set the object type to Typ[Invalid].
obj := NewVar(ident.Pos(), check.pkg, ident.Name, nil)
defer check.declare(check.topScope, ident, obj)
// TODO(gri) remove this switch, combine with code above
switch obj := obj.(type) {
default:
unreachable()
case *Const:
typ = obj.typ // may already be Typ[Invalid]
if typ == nil {
typ = Typ[Invalid]
if x.mode != invalid {
typ = x.typ
var typ Type = Typ[Invalid]
if x.mode != invalid {
typ = x.typ
if isUntyped(typ) {
// convert untyped types to default types
if typ == Typ[UntypedNil] {
check.errorf(x.pos(), "use of untyped nil")
typ = Typ[Invalid]
} else {
typ = defaultType(typ)
}
obj.typ = typ
}
case *Var:
typ = obj.typ // may already be Typ[Invalid]
if typ == nil {
typ = Typ[Invalid]
if x.mode != invalid {
typ = x.typ
if isUntyped(typ) {
// convert untyped types to default types
if typ == Typ[UntypedNil] {
check.errorf(x.pos(), "use of untyped nil")
typ = Typ[Invalid]
} else {
typ = defaultType(typ)
}
}
}
obj.typ = typ
}
}
obj.typ = typ
// nothing else to check if we don't have a valid lhs or rhs
if typ == Typ[Invalid] || x.mode == invalid {
@ -151,38 +123,22 @@ func (check *checker) assign1to1(lhs, rhs ast.Expr, x *operand, decl bool, isCon
}
return
}
// for constants, set their value
if obj, _ := obj.(*Const); obj != nil {
obj.val = exact.MakeUnknown() // failure case: we don't know the constant value
if x.mode == constant {
if isConstType(x.typ) {
obj.val = x.val
} else if x.typ != Typ[Invalid] {
check.errorf(x.pos(), "%s has invalid constant type", x)
}
} else if x.mode != invalid {
check.errorf(x.pos(), "%s is not constant", x)
}
}
}
// TODO(gri) assignNtoM is only used in one place now. remove and consolidate with other assignment functions.
// TODO(gri) assignNtoM is only used in one place now.
// Remove and consolidate with other assignment functions.
// assignNtoM typechecks a general assignment. If decl is set, the lhs expressions
// must be identifiers; if their types are not set, they are deduced from the types
// of the corresponding rhs expressions, or set to Typ[Invalid] in case of an error.
// assignNtoM typechecks a regular assignment.
// Precondition: len(lhs) > 0 .
//
func (check *checker) assignNtoM(lhs, rhs []ast.Expr, decl bool, isConst bool) {
func (check *checker) assignNtoM(lhs, rhs []ast.Expr) {
assert(len(lhs) > 0)
assert(!isConst || decl)
// If the lhs and rhs have corresponding expressions, treat each
// matching pair as an individual pair.
if len(lhs) == len(rhs) {
for i, e := range rhs {
check.assign1to1(lhs[i], e, nil, decl, isConst)
check.assign1to1(lhs[i], e, nil, false)
}
return
}
@ -197,7 +153,7 @@ func (check *checker) assignNtoM(lhs, rhs []ast.Expr, decl bool, isConst bool) {
var x operand
check.expr(&x, rhs[0])
if x.mode == invalid {
goto Error
return
}
if t, ok := x.typ.(*Tuple); ok && len(lhs) == t.Len() {
@ -207,7 +163,7 @@ func (check *checker) assignNtoM(lhs, rhs []ast.Expr, decl bool, isConst bool) {
obj := t.At(i)
x.expr = nil // TODO(gri) should do better here
x.typ = obj.typ
check.assign1to1(lhs[i], nil, &x, decl, isConst)
check.assign1to1(lhs[i], nil, &x, false)
}
return
}
@ -215,44 +171,15 @@ func (check *checker) assignNtoM(lhs, rhs []ast.Expr, decl bool, isConst bool) {
if x.mode == valueok && len(lhs) == 2 {
// comma-ok expression
x.mode = value
check.assign1to1(lhs[0], nil, &x, decl, isConst)
check.assign1to1(lhs[0], nil, &x, false)
x.typ = Typ[UntypedBool]
check.assign1to1(lhs[1], nil, &x, decl, isConst)
check.assign1to1(lhs[1], nil, &x, false)
return
}
}
check.errorf(lhs[0].Pos(), "assignment count mismatch: %d = %d", len(lhs), len(rhs))
Error:
// In case of a declaration, set all lhs types to Typ[Invalid].
if decl {
for _, e := range lhs {
ident, _ := e.(*ast.Ident)
if ident == nil {
check.errorf(e.Pos(), "cannot declare %s", e)
continue
}
var obj Object
if isConst {
obj = NewConst(ident.Pos(), check.pkg, ident.Name, nil, nil)
} else {
obj = NewVar(ident.Pos(), check.pkg, ident.Name, nil)
}
defer check.declare(check.topScope, ident, obj)
switch obj := obj.(type) {
case *Const:
obj.typ = Typ[Invalid]
case *Var:
obj.typ = Typ[Invalid]
default:
unreachable()
}
}
}
}
func (check *checker) optionalStmt(s ast.Stmt) {
@ -390,7 +317,7 @@ func (check *checker) stmt(s ast.Stmt) {
if x.mode == invalid {
return
}
check.assign1to1(s.X, nil, &x, false, false)
check.assign1to1(s.X, nil, &x, false)
case *ast.AssignStmt:
switch s.Tok {
@ -400,29 +327,10 @@ func (check *checker) stmt(s ast.Stmt) {
return
}
if s.Tok == token.DEFINE {
// short variable declaration
lhs := make([]Object, len(s.Lhs))
for i, x := range s.Lhs {
var obj Object
if ident, ok := x.(*ast.Ident); ok {
// use the correct obj if the ident is redeclared
obj = NewVar(ident.Pos(), check.pkg, ident.Name, nil)
if alt := check.topScope.Lookup(nil, ident.Name); alt != nil {
obj = alt
}
check.callIdent(ident, obj)
} else {
check.errorf(x.Pos(), "cannot declare %s", x)
// create a dummy variable
obj = NewVar(x.Pos(), check.pkg, "_", nil)
}
lhs[i] = obj
}
check.assignMulti(lhs, s.Rhs)
check.declareShort(check.topScope, lhs) // scope starts after the assignment
check.shortVarDecl(s.Lhs, s.Rhs)
} else {
check.assignNtoM(s.Lhs, s.Rhs, s.Tok == token.DEFINE, false)
// regular assignment
check.assignNtoM(s.Lhs, s.Rhs)
}
default:
@ -465,7 +373,7 @@ func (check *checker) stmt(s ast.Stmt) {
if x.mode == invalid {
return
}
check.assign1to1(s.Lhs[0], nil, &x, false, false)
check.assign1to1(s.Lhs[0], nil, &x, false)
}
case *ast.GoStmt:
@ -482,37 +390,18 @@ func (check *checker) stmt(s ast.Stmt) {
sig := check.funcsig
if n := sig.results.Len(); n > 0 {
// determine if the function has named results
{
named := false
lhs := make([]Object, len(sig.results.vars))
for i, res := range sig.results.vars {
if res.name != "" {
// a blank (_) result parameter is a named result
named = true
}
lhs[i] = res
}
if len(s.Results) > 0 || !named {
check.assignMulti(lhs, s.Results)
return
}
}
// TODO(gri) should not have to compute lhs, named every single time - clean this up
lhs := make([]ast.Expr, n)
named := false // if set, function has named results
named := false
lhs := make([]*Var, n)
for i, res := range sig.results.vars {
if len(res.name) > 0 {
if res.name != "" {
// a blank (_) result parameter is a named result
named = true
}
name := ast.NewIdent(res.name)
name.NamePos = s.Pos()
lhs[i] = name
lhs[i] = res
}
if len(s.Results) > 0 || !named {
// TODO(gri) assignNtoM should perhaps not require len(lhs) > 0
check.assignNtoM(lhs, s.Results, false, false)
check.initVars(lhs, s.Results, false)
return
}
} else if len(s.Results) > 0 {
check.errorf(s.Pos(), "no result values expected")
@ -797,7 +686,7 @@ func (check *checker) stmt(s ast.Stmt) {
if s.Key != nil {
x.typ = key
x.expr = s.Key
check.assign1to1(s.Key, nil, &x, decl, false)
check.assign1to1(s.Key, nil, &x, decl)
} else {
check.invalidAST(s.Pos(), "range clause requires index iteration variable")
// ok to continue
@ -805,7 +694,7 @@ func (check *checker) stmt(s ast.Stmt) {
if s.Value != nil {
x.typ = val
x.expr = s.Value
check.assign1to1(s.Value, nil, &x, decl, false)
check.assign1to1(s.Value, nil, &x, decl)
}
check.stmt(s.Body)

View File

@ -14,7 +14,7 @@ func assignments() {
c = s /* ERROR "cannot assign" */
s = b /* ERROR "cannot assign" */
v0 /* ERROR "mismatch" */, v1, v2 := 1, 2, 3, 4
v0, v1, v2 := 1 /* ERROR "mismatch" */ , 2, 3, 4
b = true
@ -45,6 +45,24 @@ func assignments() {
_ map[int]string = nil
_ chan int = nil
)
// test cases for issue 5500
_ = func() (int, bool) {
var m map[int]int
return m /* ERROR "assignment count mismatch" */ [0]
}
g := func(int, bool){}
var m map[int]int
g/* ERROR "too few arguments" */ (m[0])
}
func shortVarDecls() {
const c = 0
type d int
a, b, c /* ERROR "cannot assign" */ , d /* ERROR "cannot assign" */ := 1, "zwei", 3.0, 4
var _ int = a // a is of type int
var _ string = b // b is of type string
}
func incdecs() {

View File

@ -41,9 +41,10 @@ func appendArgs() ([]string, string) {
return []string{"foo"}, "bar"
}
func h() (interface{}, bool) {
func h() (i interface{}, ok bool) {
m := map[int]string{1: "hi"}
return m[1] // string->interface{} conversion within multi-valued expression
i, ok = m[1] // string->interface{} conversion within multi-valued expression
return
}
func main() {