From 37dd89e3af765ed41f0ad6c1c3b007e726e54eaf Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Wed, 24 Sep 2014 11:09:05 -0700 Subject: [PATCH] go.tools/go/types: reduce spurious errors after missing identifiers Fixes golang/go#8799. LGTM=adonovan R=adonovan CC=golang-codereviews https://golang.org/cl/143560043 --- go/types/assignments.go | 12 ++++++--- go/types/builtins.go | 7 +++++ go/types/call.go | 51 ++++++++++++++++++++++-------------- go/types/testdata/expr0.src | 4 +-- go/types/testdata/expr3.src | 2 +- go/types/testdata/issues.src | 13 +++++++++ go/types/typexpr.go | 3 +++ 7 files changed, 67 insertions(+), 25 deletions(-) diff --git a/go/types/assignments.go b/go/types/assignments.go index 11cf1115..b21d4e06 100644 --- a/go/types/assignments.go +++ b/go/types/assignments.go @@ -206,14 +206,17 @@ func (check *Checker) assignVar(lhs ast.Expr, x *operand) Type { func (check *Checker) initVars(lhs []*Var, rhs []ast.Expr, returnPos token.Pos) { l := len(lhs) get, r, commaOk := unpack(func(x *operand, i int) { check.expr(x, rhs[i]) }, len(rhs), l == 2 && !returnPos.IsValid()) - if l != r { + if get == nil || l != r { // invalidate lhs and use rhs for _, obj := range lhs { if obj.typ == nil { obj.typ = Typ[Invalid] } } - check.use(rhs...) + if get == nil { + return // error reported by unpack + } + check.useGetter(get, r) if returnPos.IsValid() { check.errorf(returnPos, "wrong number of return values (want %d, got %d)", l, r) return @@ -242,9 +245,12 @@ func (check *Checker) initVars(lhs []*Var, rhs []ast.Expr, returnPos token.Pos) func (check *Checker) assignVars(lhs, rhs []ast.Expr) { l := len(lhs) get, r, commaOk := unpack(func(x *operand, i int) { check.expr(x, rhs[i]) }, len(rhs), l == 2) + if get == nil { + return // error reported by unpack + } if l != r { + check.useGetter(get, r) check.errorf(rhs[0].Pos(), "assignment count mismatch (%d vs %d)", l, r) - check.use(rhs...) return } diff --git a/go/types/builtins.go b/go/types/builtins.go index 43e808f4..33d7ca39 100644 --- a/go/types/builtins.go +++ b/go/types/builtins.go @@ -46,6 +46,10 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b default: // make argument getter arg, nargs, _ = unpack(func(x *operand, i int) { check.expr(x, call.Args[i]) }, nargs, false) + if arg == nil { + x.mode = invalid + return + } // evaluate first argument, if present if nargs > 0 { arg(x, 0) @@ -372,6 +376,7 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b if T == Typ[Invalid] { return } + var min int // minimum number of arguments switch T.Underlying().(type) { case *Slice: @@ -483,10 +488,12 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b check.use(arg0) return } + check.expr(x, selx.X) if x.mode == invalid { return } + base := derefStructPtr(x.typ) sel := selx.Sel.Name obj, index, indirect := LookupFieldOrMethod(base, false, check.pkg, sel) diff --git a/go/types/call.go b/go/types/call.go index d92c0699..78826041 100644 --- a/go/types/call.go +++ b/go/types/call.go @@ -62,6 +62,12 @@ func (check *Checker) call(x *operand, e *ast.CallExpr) exprKind { } arg, n, _ := unpack(func(x *operand, i int) { check.expr(x, e.Args[i]) }, len(e.Args), false) + if arg == nil { + x.mode = invalid + x.expr = e + return statement + } + check.arguments(x, e, sig, arg, n) // determine result @@ -92,27 +98,39 @@ func (check *Checker) use(arg ...ast.Expr) { } } +// useGetter is like use, but takes a getter instead of a list of expressions. +// It should be called instead of use if a getter is present to avoid repeated +// evaluation of the first argument (since the getter was likely obtained via +// unpack, which may have evaluated the first argument already). +func (check *Checker) useGetter(get getter, n int) { + var x operand + for i := 0; i < n; i++ { + get(&x, i) + } +} + // A getter sets x as the i'th operand, where 0 <= i < n and n is the total // number of operands (context-specific, and maintained elsewhere). A getter // type-checks the i'th operand; the details of the actual check are getter- // specific. type getter func(x *operand, i int) -// unpack takes a getter get and a number of operands n. If n == 1 and the -// first operand is a function call, or a comma,ok expression and allowCommaOk -// is set, the result is a new getter and operand count providing access to the -// function results, or comma,ok values, respectively. The third result value -// reports if it is indeed the comma,ok case. In all other cases, the incoming -// getter and operand count are returned unchanged, and the third result value -// is false. +// unpack takes a getter get and a number of operands n. If n == 1, unpack +// calls the incoming getter for the first operand. If that operand is +// invalid, unpack returns (nil, 0, false). Otherwise, if that operand is a +// function call, or a comma-ok expression and allowCommaOk is set, the result +// is a new getter and operand count providing access to the function results, +// or comma-ok values, respectively. The third result value reports if it +// is indeed the comma-ok case. In all other cases, the incoming getter and +// operand count are returned unchanged, and the third result value is false. // -// In other words, if there's exactly one operand that - after type-checking by -// calling get - stands for multiple operands, the resulting getter provides access -// to those operands instead. +// In other words, if there's exactly one operand that - after type-checking +// by calling get - stands for multiple operands, the resulting getter provides +// access to those operands instead. // -// Note that unpack may call get(..., 0); but if the result getter is called -// at most once for a given operand index i (including i == 0), that operand -// is guaranteed to cause only one call of the incoming getter with that i. +// If the returned getter is called at most once for a given operand index i +// (including i == 0), that operand is guaranteed to cause only one call of +// the incoming getter with that i. // func unpack(get getter, n int, allowCommaOk bool) (getter, int, bool) { if n == 1 { @@ -120,12 +138,7 @@ func unpack(get getter, n int, allowCommaOk bool) (getter, int, bool) { var x0 operand get(&x0, 0) if x0.mode == invalid { - return func(x *operand, i int) { - if i != 0 { - unreachable() - } - x.mode = invalid - }, 1, false + return nil, 0, false } if t, ok := x0.typ.(*Tuple); ok { diff --git a/go/types/testdata/expr0.src b/go/types/testdata/expr0.src index 9a2c9933..5afb5d73 100644 --- a/go/types/testdata/expr0.src +++ b/go/types/testdata/expr0.src @@ -116,10 +116,10 @@ var ( s2 = -s0 /* ERROR "not defined" */ s3 = !s0 /* ERROR "not defined" */ s4 = ^s0 /* ERROR "not defined" */ - s5 = *s4 /* ERROR "cannot indirect" */ + s5 = *s4 s6 = &s4 s7 = *s6 - s8 = <-s7 /* ERROR "cannot receive" */ + s8 = <-s7 // channel ch chan int diff --git a/go/types/testdata/expr3.src b/go/types/testdata/expr3.src index 43825dd4..50ae7c4c 100644 --- a/go/types/testdata/expr3.src +++ b/go/types/testdata/expr3.src @@ -424,7 +424,7 @@ func _calls() { f2(3.14) /* ERROR "too few arguments" */ f2(3.14, "foo") f2(x /* ERROR "cannot pass" */ , "foo") - f2(g0 /* ERROR "used as value" */ ()) /* ERROR "too few arguments" */ + f2(g0 /* ERROR "used as value" */ ()) f2(g1 /* ERROR "cannot pass" */ ()) /* ERROR "too few arguments" */ f2(g2()) diff --git a/go/types/testdata/issues.src b/go/types/testdata/issues.src index 143eca35..58c450f8 100644 --- a/go/types/testdata/issues.src +++ b/go/types/testdata/issues.src @@ -22,3 +22,16 @@ func issue8066() { _ = float32(340282356779733661637539395458142568448 /* ERROR cannot convert */ ) ) } + +// Check that a missing identifier doesn't lead to a spurious error cascade. +func issue8799a() { + x, ok := missing /* ERROR undeclared */ () + _ = !ok + _ = x +} + +func issue8799b(x int, ok bool) { + x, ok = missing /* ERROR undeclared */ () + _ = !ok + _ = x +} diff --git a/go/types/typexpr.go b/go/types/typexpr.go index b939bd20..f47b8c85 100644 --- a/go/types/typexpr.go +++ b/go/types/typexpr.go @@ -88,6 +88,9 @@ func (check *Checker) ident(x *operand, e *ast.Ident, def *Named, path []*TypeNa case *Var: obj.used = true check.addDeclDep(obj) + if typ == Typ[Invalid] { + return + } x.mode = variable case *Func: