]> Cypherpunks repositories - gostls13.git/commitdiff
go/types: cleanup of assignment checks
authorRobert Griesemer <gri@golang.org>
Thu, 7 Mar 2013 00:14:07 +0000 (16:14 -0800)
committerRobert Griesemer <gri@golang.org>
Thu, 7 Mar 2013 00:14:07 +0000 (16:14 -0800)
Also:
- cleaner handling of constants w/ unknown value
- removed several TODOs

R=adonovan
CC=golang-dev
https://golang.org/cl/7473043

src/pkg/go/types/check.go
src/pkg/go/types/const.go
src/pkg/go/types/expr.go
src/pkg/go/types/objects.go
src/pkg/go/types/stmt.go
src/pkg/go/types/testdata/builtins.src
src/pkg/go/types/testdata/const0.src
src/pkg/go/types/testdata/decls2a.src
src/pkg/go/types/testdata/expr3.src
src/pkg/go/types/universe.go

index 19f4c34d1161085ca87880211ff2a55e89baa339..8d45d2ea81bfc32b1433cb71bb8966ab9113e686 100644 (file)
@@ -202,20 +202,22 @@ func (check *checker) object(obj Object, cycleOk bool) {
                        return // already checked
                }
                // The obj.Val field for constants is initialized to its respective
-               // iota value by the parser.
-               // The object's fields can be in one of the following states:
-               // Type != nil  =>  the constant value is Val
-               // Type == nil  =>  the constant is not typechecked yet, and Val can be:
-               // Val  is int  =>  Val is the value of iota for this declaration
-               // Val  == nil  =>  the object's expression is being evaluated
-               if obj.Val == nil {
-                       check.errorf(obj.GetPos(), "illegal cycle in initialization of %s", obj.Name)
+               // iota value (type int) by the parser.
+               // If the object's type is Typ[Invalid], the object value is ignored.
+               // If the object's type is valid, the object value must be a legal
+               // constant value; it may be nil to indicate that we don't know the
+               // value of the constant (e.g., in: "const x = float32("foo")" we
+               // know that x is a constant and has type float32, but we don't
+               // have a value due to the error in the conversion).
+               if obj.visited {
+                       check.errorf(obj.GetPos(), "illegal cycle in initialization of constant %s", obj.Name)
                        obj.Type = Typ[Invalid]
                        return
                }
+               obj.visited = true
                spec := obj.spec
                iota := obj.Val.(int)
-               obj.Val = nil // mark obj as "visited" for cycle detection
+               obj.Val = nil // set to a valid (but unknown) constant value
                // determine spec for type and initialization expressions
                init := spec
                if len(init.Values) == 0 {
@@ -228,7 +230,7 @@ func (check *checker) object(obj Object, cycleOk bool) {
                        return // already checked
                }
                if obj.visited {
-                       check.errorf(obj.GetPos(), "illegal cycle in initialization of %s", obj.Name)
+                       check.errorf(obj.GetPos(), "illegal cycle in initialization of variable %s", obj.Name)
                        obj.Type = Typ[Invalid]
                        return
                }
index 503652e75acb6304f3d27de989766136982197b5..e8e86e4fb8926a326d0d619ff2e3221dc7e166af 100644 (file)
@@ -19,6 +19,7 @@ import (
 
 // Representation of constant values.
 //
+// invalid  ->  nil (i.e., we don't know the constant value; this can only happen in erroneous programs)
 // bool     ->  bool (true, false)
 // numeric  ->  int64, *big.Int, *big.Rat, Complex (ordered by increasing data structure "size")
 // string   ->  string
@@ -159,6 +160,8 @@ func makeStringConst(lit string) interface{} {
 func toImagConst(x interface{}) interface{} {
        var im *big.Rat
        switch x := x.(type) {
+       case nil:
+               im = rat0
        case int64:
                im = big.NewRat(x, 1)
        case *big.Int:
@@ -184,6 +187,8 @@ func isZeroConst(x interface{}) bool {
 //
 func isNegConst(x interface{}) bool {
        switch x := x.(type) {
+       case nil:
+               return false
        case int64:
                return x < 0
        case *big.Int:
@@ -200,6 +205,10 @@ func isNegConst(x interface{}) bool {
 // of precision.
 //
 func isRepresentableConst(x interface{}, ctxt *Context, as BasicKind) bool {
+       if x == nil {
+               return true // avoid spurious errors
+       }
+
        switch x := x.(type) {
        case bool:
                return as == Bool || as == UntypedBool
@@ -387,6 +396,10 @@ func is63bit(x int64) bool {
 
 // unaryOpConst returns the result of the constant evaluation op x where x is of the given type.
 func unaryOpConst(x interface{}, ctxt *Context, op token.Token, typ *Basic) interface{} {
+       if x == nil {
+               return nil
+       }
+
        switch op {
        case token.ADD:
                return x // nothing to do
@@ -437,6 +450,10 @@ func unaryOpConst(x interface{}, ctxt *Context, op token.Token, typ *Basic) inte
 // division. Division by zero leads to a run-time panic.
 //
 func binaryOpConst(x, y interface{}, op token.Token, typ *Basic) interface{} {
+       if x == nil || y == nil {
+               return nil
+       }
+
        x, y = matchConst(x, y)
 
        switch x := x.(type) {
@@ -591,6 +608,9 @@ func binaryOpConst(x, y interface{}, op token.Token, typ *Basic) interface{} {
 //
 func shiftConst(x interface{}, s uint, op token.Token) interface{} {
        switch x := x.(type) {
+       case nil:
+               return nil
+
        case int64:
                switch op {
                case token.SHL:
@@ -619,6 +639,10 @@ func shiftConst(x interface{}, s uint, op token.Token) interface{} {
 // or NilType).
 //
 func compareConst(x, y interface{}, op token.Token) (z bool) {
+       if x == nil || y == nil {
+               return false
+       }
+
        x, y = matchConst(x, y)
 
        // x == y  =>  x == y
index 86d782d483621271e244860e14a7dcd8946aeee9..3b0625239f23795e7b012c443a00da8deda50aff 100644 (file)
@@ -580,7 +580,11 @@ func (check *checker) binary(x *operand, lhs, rhs ast.Expr, op token.Token, iota
        }
 
        if !IsIdentical(x.typ, y.typ) {
-               check.invalidOp(x.pos(), "mismatched types %s and %s", x.typ, y.typ)
+               // only report an error if we have valid types
+               // (otherwise we had an error reported elsewhere already)
+               if x.typ != Typ[Invalid] && y.typ != Typ[Invalid] {
+                       check.invalidOp(x.pos(), "mismatched types %s and %s", x.typ, y.typ)
+               }
                x.mode = invalid
                return
        }
@@ -823,8 +827,8 @@ func (check *checker) rawExpr(x *operand, e ast.Expr, hint Type, iota int, cycle
                        check.errorf(e.Pos(), "use of package %s not in selector", obj.Name)
                        goto Error
                case *Const:
-                       if obj.Val == nil {
-                               goto Error // cycle detected
+                       if obj.Type == Typ[Invalid] {
+                               goto Error
                        }
                        x.mode = constant
                        if obj == universeIota {
@@ -834,7 +838,7 @@ func (check *checker) rawExpr(x *operand, e ast.Expr, hint Type, iota int, cycle
                                }
                                x.val = int64(iota)
                        } else {
-                               x.val = obj.Val
+                               x.val = obj.Val // may be nil if we don't know the constant value
                        }
                case *TypeName:
                        x.mode = typexpr
index 02291d34c56ae9d05fec62acb5a2cc7e86abe7c6..73301c6ca4781dce747a1b5fbe5f7c8b25985be6 100644 (file)
@@ -38,9 +38,10 @@ type Const struct {
        Pkg  *Package
        Name string
        Type Type
-       Val  interface{}
+       Val  interface{} // nil means unknown constant value due to type error
 
-       spec *ast.ValueSpec
+       visited bool // for initialization cycle detection
+       spec    *ast.ValueSpec
 }
 
 // A TypeName represents a declared type.
index 53c46a167cd6be63db86645e7fc8b816bdc5abb9..44160659f5c11c917a2f25d8b298edf4e8ad8976 100644 (file)
@@ -35,149 +35,123 @@ func (check *checker) assignment(x *operand, to Type) bool {
 }
 
 // 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 operand must be an identifier.
-// If its type is not set, it is deduced from the type or value of x. If lhs has a
-// type it is used as a hint when evaluating rhs, if present.
+// or lhs = x (if rhs == nil). If decl is set, the lhs operand 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, iota int) {
-       ident, _ := lhs.(*ast.Ident)
+       // Start with rhs so we have an expression type
+       // for declarations with implicit type.
        if x == nil {
-               assert(rhs != nil)
                x = new(operand)
-       }
-
-       if ident != nil && ident.Name == "_" {
-               // anything can be assigned to a blank identifier - check rhs only, if present
-               if rhs != nil {
-                       check.expr(x, rhs, nil, iota)
+               check.expr(x, rhs, nil, iota)
+               // don't exit for declarations - we need the lhs obj first
+               if x.mode == invalid && !decl {
+                       return
                }
-               return
        }
+       // x.mode == valid || decl
+
+       // lhs may be an identifier
+       ident, _ := lhs.(*ast.Ident)
 
+       // regular assignment; we know x is valid
        if !decl {
-               // regular assignment - start with lhs to obtain a type hint
-               // TODO(gri) clean this up - we don't need type hints anymore
+               // anything can be assigned to the blank identifier
+               if ident != nil && ident.Name == "_" {
+                       return
+               }
+
                var z operand
                check.expr(&z, lhs, nil, -1)
                if z.mode == invalid {
-                       z.typ = nil // so we can proceed with rhs
-               }
-
-               if rhs != nil {
-                       check.expr(x, rhs, z.typ, -1)
-                       if x.mode == invalid {
-                               return
-                       }
-               }
-
-               if x.mode == invalid || z.mode == invalid {
                        return
                }
 
-               if !check.assignment(x, z.typ) {
+               // TODO(gri) verify that all other z.mode values
+               //           that may appear here are legal
+               if z.mode == constant || !check.assignment(x, z.typ) {
                        if x.mode != invalid {
                                check.errorf(x.pos(), "cannot assign %s to %s", x, &z)
                        }
-                       return
-               }
-               if z.mode == constant {
-                       check.errorf(x.pos(), "cannot assign %s to %s", x, &z)
                }
                return
        }
 
-       // declaration - lhs must be an identifier
+       // declaration with initialization; lhs must be an identifier
        if ident == nil {
                check.errorf(lhs.Pos(), "cannot declare %s", lhs)
                return
        }
 
-       // lhs may or may not be typed yet
-       obj := check.lookup(ident)
+       // 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
-       if t := obj.GetType(); t != nil {
-               typ = t
-       }
+       obj := check.lookup(ident)
+       switch obj := obj.(type) {
+       default:
+               unreachable()
 
-       if rhs != nil {
-               check.expr(x, rhs, typ, iota)
-               // continue even if x.mode == invalid
-       }
+       case nil:
+               // TODO(gri) is this really unreachable?
+               unreachable()
 
-       if typ == nil {
-               // determine lhs type from rhs expression;
-               // for variables, convert untyped types to
-               // default types
-               typ = Typ[Invalid]
-               if x.mode != invalid {
-                       typ = x.typ
-                       if _, ok := obj.(*Var); ok && isUntyped(typ) {
-                               if x.isNil() {
-                                       check.errorf(x.pos(), "use of untyped nil")
-                                       x.mode = invalid
-                               } else {
+       case *Const:
+               typ = obj.Type // may already be Typ[Invalid]
+               if typ == nil {
+                       typ = Typ[Invalid]
+                       if x.mode != invalid {
+                               typ = x.typ
+                       }
+                       obj.Type = typ
+               }
+
+       case *Var:
+               typ = obj.Type // 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")
+                                               obj.Type = Typ[Invalid]
+                                               return
+                                       }
                                        typ = defaultType(typ)
                                }
                        }
-               }
-               switch obj := obj.(type) {
-               case *Const:
-                       obj.Type = typ
-               case *Var:
                        obj.Type = typ
-               default:
-                       unreachable()
                }
        }
 
-       if x.mode != invalid {
-               if !check.assignment(x, typ) {
-                       if x.mode != invalid {
-                               switch obj.(type) {
-                               case *Const:
-                                       check.errorf(x.pos(), "cannot assign %s to variable of type %s", x, typ)
-                               case *Var:
-                                       check.errorf(x.pos(), "cannot initialize constant of type %s with %s", typ, x)
-                               default:
-                                       unreachable()
-                               }
-                               x.mode = invalid
+       // nothing else to check if we don't have a valid lhs or rhs
+       if typ == Typ[Invalid] || x.mode == invalid {
+               return
+       }
+
+       if !check.assignment(x, typ) {
+               if x.mode != invalid {
+                       if x.typ != Typ[Invalid] && typ != Typ[Invalid] {
+                               check.errorf(x.pos(), "cannot initialize %s (type %s) with %s", ident.Name, typ, x)
                        }
                }
+               return
        }
 
        // for constants, set their value
-       if obj, ok := obj.(*Const); ok {
-               assert(obj.Val == nil)
-               if x.mode != invalid {
-                       if x.mode == constant {
-                               if isConstType(x.typ) {
-                                       obj.Val = x.val
-                               } else {
-                                       check.errorf(x.pos(), "%s has invalid constant type", x)
-                               }
-                       } else {
-                               check.errorf(x.pos(), "%s is not constant", x)
-                       }
-               }
-               if obj.Val == nil {
-                       // set the constant to its type's zero value to reduce spurious errors
-                       switch typ := underlying(obj.Type); {
-                       case typ == Typ[Invalid]:
-                               // ignore
-                       case isBoolean(typ):
-                               obj.Val = false
-                       case isNumeric(typ):
-                               obj.Val = int64(0)
-                       case isString(typ):
-                               obj.Val = ""
-                       case hasNil(typ):
-                               obj.Val = nilConst
-                       default:
-                               // in all other cases just prevent use of the constant
-                               // TODO(gri) re-evaluate this code
-                               obj.Val = nilConst
+       if obj, _ := obj.(*Const); obj != nil {
+               obj.Val = nil // 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)
                }
        }
 }
@@ -494,6 +468,7 @@ func (check *checker) stmt(s ast.Stmt) {
                check.expr(&x, tag, nil, -1)
 
                check.multipleDefaults(s.Body.List)
+               // TODO(gri) check also correct use of fallthrough
                seen := make(map[interface{}]token.Pos)
                for _, s := range s.Body.List {
                        clause, _ := s.(*ast.CaseClause)
index c08c442ce3055d139783e9c2b43458d1d05634ba..6fe46550890ef95b01a1f968dce7eca94867459f 100644 (file)
@@ -154,7 +154,6 @@ func _len() {
        assert /* ERROR "failed" */ (n == 10)
        var ch <-chan int
        const nn = len /* ERROR "not constant" */ (hash[<-ch][len(t)])
-       _ = nn // TODO(gri) remove this once unused constants get type-checked
 
        // issue 4744
        type T struct{ a [10]int }
index a2ca344c788e6c0eb98090d6f62e68bbfa3caea5..788c6f51adbe268a94cfedc530e1719eb3f0732f 100644 (file)
@@ -111,8 +111,8 @@ const (
        ti5 = ti0 /* ERROR "mismatched types" */ + ti1
        ti6 = ti1 - ti1
        ti7 = ti2 /* ERROR "mismatched types" */ * ti1
-       //ti8 = ti3 / ti3 // TODO(gri) enable this
-       //ti9 = ti3 % ti3 // TODO(gri) enable this
+       ti8 = ti3 / ti3
+       ti9 = ti3 % ti3
 
        ti10 = 1 / 0 /* ERROR "division by zero" */
        ti11 = ti1 / 0 /* ERROR "division by zero" */
@@ -135,7 +135,7 @@ const (
        tf5 = tf0 + tf1
        tf6 = tf1 - tf1
        tf7 = tf2 /* ERROR "mismatched types" */ * tf1
-       // tf8 = tf3 / tf3 // TODO(gri) enable this
+       tf8 = tf3 / tf3
        tf9 = tf3 /* ERROR "not defined" */ % tf3
 
        tf10 = 1 / 0 /* ERROR "division by zero" */
index 3867be737653d80c4f59ed9de8e6a9552e05cc88..c15ac917d8849c343d3c95293283f37fca62040b 100644 (file)
@@ -28,10 +28,9 @@ type T2 struct {
 func (undeclared /* ERROR "undeclared" */) m() {}
 func (x *undeclared /* ERROR "undeclared" */) m() {}
 
-// TODO(gri) try to get rid of double error reporting here
 func (pi /* ERROR "not a type" */) m1() {}
 func (x pi /* ERROR "not a type" */) m2() {}
-func (x *pi /* ERROR "not a type" */ ) m3() {} // TODO(gri) not closing the last /* comment crashes the system
+func (x *pi /* ERROR "not a type" */ ) m3() {}
 
 // Blank types.
 type _ struct { m int }
index ff17f2eee44ba97330aa2f64f5f2c54d91e84c3d..f5963ca117ded2077c2b5d870d6dd620843a9a75 100644 (file)
@@ -125,9 +125,10 @@ func shifts4() {
        }
 }
 
-// TODO(gri) The error messages below depond on adjusting the spec
+// TODO(gri) The error messages below depend on adjusting the spec
 //           to reflect what gc is doing at the moment (the spec
 //           asks for run-time errors at the moment - see issue 4231).
+// TODO(gri) This has been fixed in the spec. Fix this.
 //
 func indexes() {
        _ = 1 /* ERROR "cannot index" */ [0]
index b218525c1c470bfee68de97cac550aaaf06c99ad..cae18fab0966f316b866fb6698055dd1bafd5b70 100644 (file)
@@ -55,10 +55,10 @@ var aliases = [...]*Basic{
 }
 
 var predeclaredConstants = [...]*Const{
-       {nil, "true", Typ[UntypedBool], true, nil},
-       {nil, "false", Typ[UntypedBool], false, nil},
-       {nil, "iota", Typ[UntypedInt], zeroConst, nil},
-       {nil, "nil", Typ[UntypedNil], nilConst, nil},
+       {Name: "true", Type: Typ[UntypedBool], Val: true},
+       {Name: "false", Type: Typ[UntypedBool], Val: false},
+       {Name: "iota", Type: Typ[UntypedInt], Val: zeroConst},
+       {Name: "nil", Type: Typ[UntypedNil], Val: nilConst},
 }
 
 var predeclaredFunctions = [...]*builtin{