Skip to content

Commit b8db56a

Browse files
committed
go/types: cleanup of assignment checks
Also: - cleaner handling of constants w/ unknown value - removed several TODOs R=adonovan CC=golang-dev https://golang.org/cl/7473043
1 parent 0a71a5b commit b8db56a

File tree

10 files changed

+133
-128
lines changed

10 files changed

+133
-128
lines changed

src/pkg/go/types/check.go

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -202,20 +202,22 @@ func (check *checker) object(obj Object, cycleOk bool) {
202202
return // already checked
203203
}
204204
// The obj.Val field for constants is initialized to its respective
205-
// iota value by the parser.
206-
// The object's fields can be in one of the following states:
207-
// Type != nil => the constant value is Val
208-
// Type == nil => the constant is not typechecked yet, and Val can be:
209-
// Val is int => Val is the value of iota for this declaration
210-
// Val == nil => the object's expression is being evaluated
211-
if obj.Val == nil {
212-
check.errorf(obj.GetPos(), "illegal cycle in initialization of %s", obj.Name)
205+
// iota value (type int) by the parser.
206+
// If the object's type is Typ[Invalid], the object value is ignored.
207+
// If the object's type is valid, the object value must be a legal
208+
// constant value; it may be nil to indicate that we don't know the
209+
// value of the constant (e.g., in: "const x = float32("foo")" we
210+
// know that x is a constant and has type float32, but we don't
211+
// have a value due to the error in the conversion).
212+
if obj.visited {
213+
check.errorf(obj.GetPos(), "illegal cycle in initialization of constant %s", obj.Name)
213214
obj.Type = Typ[Invalid]
214215
return
215216
}
217+
obj.visited = true
216218
spec := obj.spec
217219
iota := obj.Val.(int)
218-
obj.Val = nil // mark obj as "visited" for cycle detection
220+
obj.Val = nil // set to a valid (but unknown) constant value
219221
// determine spec for type and initialization expressions
220222
init := spec
221223
if len(init.Values) == 0 {
@@ -228,7 +230,7 @@ func (check *checker) object(obj Object, cycleOk bool) {
228230
return // already checked
229231
}
230232
if obj.visited {
231-
check.errorf(obj.GetPos(), "illegal cycle in initialization of %s", obj.Name)
233+
check.errorf(obj.GetPos(), "illegal cycle in initialization of variable %s", obj.Name)
232234
obj.Type = Typ[Invalid]
233235
return
234236
}

src/pkg/go/types/const.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919

2020
// Representation of constant values.
2121
//
22+
// invalid -> nil (i.e., we don't know the constant value; this can only happen in erroneous programs)
2223
// bool -> bool (true, false)
2324
// numeric -> int64, *big.Int, *big.Rat, Complex (ordered by increasing data structure "size")
2425
// string -> string
@@ -159,6 +160,8 @@ func makeStringConst(lit string) interface{} {
159160
func toImagConst(x interface{}) interface{} {
160161
var im *big.Rat
161162
switch x := x.(type) {
163+
case nil:
164+
im = rat0
162165
case int64:
163166
im = big.NewRat(x, 1)
164167
case *big.Int:
@@ -184,6 +187,8 @@ func isZeroConst(x interface{}) bool {
184187
//
185188
func isNegConst(x interface{}) bool {
186189
switch x := x.(type) {
190+
case nil:
191+
return false
187192
case int64:
188193
return x < 0
189194
case *big.Int:
@@ -200,6 +205,10 @@ func isNegConst(x interface{}) bool {
200205
// of precision.
201206
//
202207
func isRepresentableConst(x interface{}, ctxt *Context, as BasicKind) bool {
208+
if x == nil {
209+
return true // avoid spurious errors
210+
}
211+
203212
switch x := x.(type) {
204213
case bool:
205214
return as == Bool || as == UntypedBool
@@ -387,6 +396,10 @@ func is63bit(x int64) bool {
387396

388397
// unaryOpConst returns the result of the constant evaluation op x where x is of the given type.
389398
func unaryOpConst(x interface{}, ctxt *Context, op token.Token, typ *Basic) interface{} {
399+
if x == nil {
400+
return nil
401+
}
402+
390403
switch op {
391404
case token.ADD:
392405
return x // nothing to do
@@ -437,6 +450,10 @@ func unaryOpConst(x interface{}, ctxt *Context, op token.Token, typ *Basic) inte
437450
// division. Division by zero leads to a run-time panic.
438451
//
439452
func binaryOpConst(x, y interface{}, op token.Token, typ *Basic) interface{} {
453+
if x == nil || y == nil {
454+
return nil
455+
}
456+
440457
x, y = matchConst(x, y)
441458

442459
switch x := x.(type) {
@@ -591,6 +608,9 @@ func binaryOpConst(x, y interface{}, op token.Token, typ *Basic) interface{} {
591608
//
592609
func shiftConst(x interface{}, s uint, op token.Token) interface{} {
593610
switch x := x.(type) {
611+
case nil:
612+
return nil
613+
594614
case int64:
595615
switch op {
596616
case token.SHL:
@@ -619,6 +639,10 @@ func shiftConst(x interface{}, s uint, op token.Token) interface{} {
619639
// or NilType).
620640
//
621641
func compareConst(x, y interface{}, op token.Token) (z bool) {
642+
if x == nil || y == nil {
643+
return false
644+
}
645+
622646
x, y = matchConst(x, y)
623647

624648
// x == y => x == y

src/pkg/go/types/expr.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -580,7 +580,11 @@ func (check *checker) binary(x *operand, lhs, rhs ast.Expr, op token.Token, iota
580580
}
581581

582582
if !IsIdentical(x.typ, y.typ) {
583-
check.invalidOp(x.pos(), "mismatched types %s and %s", x.typ, y.typ)
583+
// only report an error if we have valid types
584+
// (otherwise we had an error reported elsewhere already)
585+
if x.typ != Typ[Invalid] && y.typ != Typ[Invalid] {
586+
check.invalidOp(x.pos(), "mismatched types %s and %s", x.typ, y.typ)
587+
}
584588
x.mode = invalid
585589
return
586590
}
@@ -823,8 +827,8 @@ func (check *checker) rawExpr(x *operand, e ast.Expr, hint Type, iota int, cycle
823827
check.errorf(e.Pos(), "use of package %s not in selector", obj.Name)
824828
goto Error
825829
case *Const:
826-
if obj.Val == nil {
827-
goto Error // cycle detected
830+
if obj.Type == Typ[Invalid] {
831+
goto Error
828832
}
829833
x.mode = constant
830834
if obj == universeIota {
@@ -834,7 +838,7 @@ func (check *checker) rawExpr(x *operand, e ast.Expr, hint Type, iota int, cycle
834838
}
835839
x.val = int64(iota)
836840
} else {
837-
x.val = obj.Val
841+
x.val = obj.Val // may be nil if we don't know the constant value
838842
}
839843
case *TypeName:
840844
x.mode = typexpr

src/pkg/go/types/objects.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,10 @@ type Const struct {
3838
Pkg *Package
3939
Name string
4040
Type Type
41-
Val interface{}
41+
Val interface{} // nil means unknown constant value due to type error
4242

43-
spec *ast.ValueSpec
43+
visited bool // for initialization cycle detection
44+
spec *ast.ValueSpec
4445
}
4546

4647
// A TypeName represents a declared type.

src/pkg/go/types/stmt.go

Lines changed: 76 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -35,149 +35,123 @@ func (check *checker) assignment(x *operand, to Type) bool {
3535
}
3636

3737
// assign1to1 typechecks a single assignment of the form lhs = rhs (if rhs != nil),
38-
// or lhs = x (if rhs == nil). If decl is set, the lhs operand must be an identifier.
39-
// If its type is not set, it is deduced from the type or value of x. If lhs has a
40-
// type it is used as a hint when evaluating rhs, if present.
38+
// or lhs = x (if rhs == nil). If decl is set, the lhs operand must be an identifier;
39+
// if its type is not set, it is deduced from the type of x or set to Typ[Invalid] in
40+
// case of an error.
4141
//
4242
func (check *checker) assign1to1(lhs, rhs ast.Expr, x *operand, decl bool, iota int) {
43-
ident, _ := lhs.(*ast.Ident)
43+
// Start with rhs so we have an expression type
44+
// for declarations with implicit type.
4445
if x == nil {
45-
assert(rhs != nil)
4646
x = new(operand)
47-
}
48-
49-
if ident != nil && ident.Name == "_" {
50-
// anything can be assigned to a blank identifier - check rhs only, if present
51-
if rhs != nil {
52-
check.expr(x, rhs, nil, iota)
47+
check.expr(x, rhs, nil, iota)
48+
// don't exit for declarations - we need the lhs obj first
49+
if x.mode == invalid && !decl {
50+
return
5351
}
54-
return
5552
}
53+
// x.mode == valid || decl
54+
55+
// lhs may be an identifier
56+
ident, _ := lhs.(*ast.Ident)
5657

58+
// regular assignment; we know x is valid
5759
if !decl {
58-
// regular assignment - start with lhs to obtain a type hint
59-
// TODO(gri) clean this up - we don't need type hints anymore
60+
// anything can be assigned to the blank identifier
61+
if ident != nil && ident.Name == "_" {
62+
return
63+
}
64+
6065
var z operand
6166
check.expr(&z, lhs, nil, -1)
6267
if z.mode == invalid {
63-
z.typ = nil // so we can proceed with rhs
64-
}
65-
66-
if rhs != nil {
67-
check.expr(x, rhs, z.typ, -1)
68-
if x.mode == invalid {
69-
return
70-
}
71-
}
72-
73-
if x.mode == invalid || z.mode == invalid {
7468
return
7569
}
7670

77-
if !check.assignment(x, z.typ) {
71+
// TODO(gri) verify that all other z.mode values
72+
// that may appear here are legal
73+
if z.mode == constant || !check.assignment(x, z.typ) {
7874
if x.mode != invalid {
7975
check.errorf(x.pos(), "cannot assign %s to %s", x, &z)
8076
}
81-
return
82-
}
83-
if z.mode == constant {
84-
check.errorf(x.pos(), "cannot assign %s to %s", x, &z)
8577
}
8678
return
8779
}
8880

89-
// declaration - lhs must be an identifier
81+
// declaration with initialization; lhs must be an identifier
9082
if ident == nil {
9183
check.errorf(lhs.Pos(), "cannot declare %s", lhs)
9284
return
9385
}
9486

95-
// lhs may or may not be typed yet
96-
obj := check.lookup(ident)
87+
// Determine typ of lhs: If the object doesn't have a type
88+
// yet, determine it from the type of x; if x is invalid,
89+
// set the object type to Typ[Invalid].
9790
var typ Type
98-
if t := obj.GetType(); t != nil {
99-
typ = t
100-
}
91+
obj := check.lookup(ident)
92+
switch obj := obj.(type) {
93+
default:
94+
unreachable()
10195

102-
if rhs != nil {
103-
check.expr(x, rhs, typ, iota)
104-
// continue even if x.mode == invalid
105-
}
96+
case nil:
97+
// TODO(gri) is this really unreachable?
98+
unreachable()
10699

107-
if typ == nil {
108-
// determine lhs type from rhs expression;
109-
// for variables, convert untyped types to
110-
// default types
111-
typ = Typ[Invalid]
112-
if x.mode != invalid {
113-
typ = x.typ
114-
if _, ok := obj.(*Var); ok && isUntyped(typ) {
115-
if x.isNil() {
116-
check.errorf(x.pos(), "use of untyped nil")
117-
x.mode = invalid
118-
} else {
100+
case *Const:
101+
typ = obj.Type // may already be Typ[Invalid]
102+
if typ == nil {
103+
typ = Typ[Invalid]
104+
if x.mode != invalid {
105+
typ = x.typ
106+
}
107+
obj.Type = typ
108+
}
109+
110+
case *Var:
111+
typ = obj.Type // may already be Typ[Invalid]
112+
if typ == nil {
113+
typ = Typ[Invalid]
114+
if x.mode != invalid {
115+
typ = x.typ
116+
if isUntyped(typ) {
117+
// convert untyped types to default types
118+
if typ == Typ[UntypedNil] {
119+
check.errorf(x.pos(), "use of untyped nil")
120+
obj.Type = Typ[Invalid]
121+
return
122+
}
119123
typ = defaultType(typ)
120124
}
121125
}
122-
}
123-
switch obj := obj.(type) {
124-
case *Const:
125-
obj.Type = typ
126-
case *Var:
127126
obj.Type = typ
128-
default:
129-
unreachable()
130127
}
131128
}
132129

133-
if x.mode != invalid {
134-
if !check.assignment(x, typ) {
135-
if x.mode != invalid {
136-
switch obj.(type) {
137-
case *Const:
138-
check.errorf(x.pos(), "cannot assign %s to variable of type %s", x, typ)
139-
case *Var:
140-
check.errorf(x.pos(), "cannot initialize constant of type %s with %s", typ, x)
141-
default:
142-
unreachable()
143-
}
144-
x.mode = invalid
130+
// nothing else to check if we don't have a valid lhs or rhs
131+
if typ == Typ[Invalid] || x.mode == invalid {
132+
return
133+
}
134+
135+
if !check.assignment(x, typ) {
136+
if x.mode != invalid {
137+
if x.typ != Typ[Invalid] && typ != Typ[Invalid] {
138+
check.errorf(x.pos(), "cannot initialize %s (type %s) with %s", ident.Name, typ, x)
145139
}
146140
}
141+
return
147142
}
148143

149144
// for constants, set their value
150-
if obj, ok := obj.(*Const); ok {
151-
assert(obj.Val == nil)
152-
if x.mode != invalid {
153-
if x.mode == constant {
154-
if isConstType(x.typ) {
155-
obj.Val = x.val
156-
} else {
157-
check.errorf(x.pos(), "%s has invalid constant type", x)
158-
}
159-
} else {
160-
check.errorf(x.pos(), "%s is not constant", x)
161-
}
162-
}
163-
if obj.Val == nil {
164-
// set the constant to its type's zero value to reduce spurious errors
165-
switch typ := underlying(obj.Type); {
166-
case typ == Typ[Invalid]:
167-
// ignore
168-
case isBoolean(typ):
169-
obj.Val = false
170-
case isNumeric(typ):
171-
obj.Val = int64(0)
172-
case isString(typ):
173-
obj.Val = ""
174-
case hasNil(typ):
175-
obj.Val = nilConst
176-
default:
177-
// in all other cases just prevent use of the constant
178-
// TODO(gri) re-evaluate this code
179-
obj.Val = nilConst
145+
if obj, _ := obj.(*Const); obj != nil {
146+
obj.Val = nil // failure case: we don't know the constant value
147+
if x.mode == constant {
148+
if isConstType(x.typ) {
149+
obj.Val = x.val
150+
} else if x.typ != Typ[Invalid] {
151+
check.errorf(x.pos(), "%s has invalid constant type", x)
180152
}
153+
} else if x.mode != invalid {
154+
check.errorf(x.pos(), "%s is not constant", x)
181155
}
182156
}
183157
}
@@ -494,6 +468,7 @@ func (check *checker) stmt(s ast.Stmt) {
494468
check.expr(&x, tag, nil, -1)
495469

496470
check.multipleDefaults(s.Body.List)
471+
// TODO(gri) check also correct use of fallthrough
497472
seen := make(map[interface{}]token.Pos)
498473
for _, s := range s.Body.List {
499474
clause, _ := s.(*ast.CaseClause)

0 commit comments

Comments
 (0)