From bf9240681dec2664f6acc1695e517e985d2b85d3 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Wed, 3 Oct 2018 17:50:02 -0700 Subject: [PATCH] go/types: prepare for delayed type-checking of methods to when they are used Remove assumption that methods associated to concrete (non-interface) types have a fully set up signature. Such methods are found through LookupFieldOrMethod or lookupMethod, or indexed method access from a Named type. Make sure that the method's signature is type-checked before use in those cases. (MethodSets also hold methods but the type checker is not using them but for internal verification. API clients will be using it after all methods have been type-checked.) Some functions such as MissingMethod may now have to type-check a method and for that they need a *Checker. Add helper functions as necessary to provide the additional (receiver) parameter but permit it to be nil if the respective functions are invoked through the API (at which point we know that all methods have a proper signature and thus we don't need the delayed type-check). Since all package-level objects eventually are type-checked through the top-level loop in Checker.packageObjects we are guaranteed that all methods will be type-checked as well. Updates #23203. Updates #26854. Change-Id: I6e48f0016cefd498aa70b776e84a48215a9042c5 Reviewed-on: https://go-review.googlesource.com/c/139425 Reviewed-by: Alan Donovan --- src/go/types/api.go | 6 +++--- src/go/types/assignments.go | 2 +- src/go/types/builtins.go | 4 ++-- src/go/types/call.go | 5 +++++ src/go/types/conversions.go | 11 +++++++---- src/go/types/decl.go | 4 ++++ src/go/types/expr.go | 17 +++++++++++++---- src/go/types/lookup.go | 27 ++++++++++++++++++++++++--- src/go/types/operand.go | 8 +++++--- src/go/types/type.go | 2 +- src/go/types/typexpr.go | 2 +- 11 files changed, 66 insertions(+), 22 deletions(-) diff --git a/src/go/types/api.go b/src/go/types/api.go index b1fcb2d10b..1252aade35 100644 --- a/src/go/types/api.go +++ b/src/go/types/api.go @@ -353,20 +353,20 @@ func (conf *Config) Check(path string, fset *token.FileSet, files []*ast.File, i // AssertableTo reports whether a value of type V can be asserted to have type T. func AssertableTo(V *Interface, T Type) bool { - m, _ := assertableTo(V, T) + m, _ := (*Checker)(nil).assertableTo(V, T) return m == nil } // AssignableTo reports whether a value of type V is assignable to a variable of type T. func AssignableTo(V, T Type) bool { x := operand{mode: value, typ: V} - return x.assignableTo(nil, T, nil) // config not needed for non-constant x + return x.assignableTo(nil, T, nil) // check not needed for non-constant x } // ConvertibleTo reports whether a value of type V is convertible to a value of type T. func ConvertibleTo(V, T Type) bool { x := operand{mode: value, typ: V} - return x.convertibleTo(nil, T) // config not needed for non-constant x + return x.convertibleTo(nil, T) // check not needed for non-constant x } // Implements reports whether type V implements interface T. diff --git a/src/go/types/assignments.go b/src/go/types/assignments.go index 27002f6699..efa0cbba50 100644 --- a/src/go/types/assignments.go +++ b/src/go/types/assignments.go @@ -57,7 +57,7 @@ func (check *Checker) assignment(x *operand, T Type, context string) { return } - if reason := ""; !x.assignableTo(check.conf, T, &reason) { + if reason := ""; !x.assignableTo(check, T, &reason) { if reason != "" { check.errorf(x.pos(), "cannot use %s as %s value in %s: %s", x, T, context, reason) } else { diff --git a/src/go/types/builtins.go b/src/go/types/builtins.go index d3f0c4d40d..882c773db4 100644 --- a/src/go/types/builtins.go +++ b/src/go/types/builtins.go @@ -95,7 +95,7 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b // spec: "As a special case, append also accepts a first argument assignable // to type []byte with a second argument of string type followed by ... . // This form appends the bytes of the string. - if nargs == 2 && call.Ellipsis.IsValid() && x.assignableTo(check.conf, NewSlice(universeByte), nil) { + if nargs == 2 && call.Ellipsis.IsValid() && x.assignableTo(check, NewSlice(universeByte), nil) { arg(x, 1) if x.mode == invalid { return @@ -345,7 +345,7 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b return } - if !x.assignableTo(check.conf, m.key, nil) { + if !x.assignableTo(check, m.key, nil) { check.invalidArg(x.pos(), "%s is not assignable to %s", x, m.key) return } diff --git a/src/go/types/call.go b/src/go/types/call.go index d5c196afe8..52f1ac31ce 100644 --- a/src/go/types/call.go +++ b/src/go/types/call.go @@ -383,6 +383,11 @@ func (check *Checker) selector(x *operand, e *ast.SelectorExpr) { goto Error } + // methods may not have a fully set up signature yet + if m, _ := obj.(*Func); m != nil { + check.objDecl(m, nil) + } + if x.mode == typexpr { // method expression m, _ := obj.(*Func) diff --git a/src/go/types/conversions.go b/src/go/types/conversions.go index 81a65838fe..fecb7b617f 100644 --- a/src/go/types/conversions.go +++ b/src/go/types/conversions.go @@ -18,7 +18,7 @@ func (check *Checker) conversion(x *operand, T Type) { case constArg && isConstType(T): // constant conversion switch t := T.Underlying().(*Basic); { - case representableConst(x.val, check.conf, t, &x.val): + case representableConst(x.val, check, t, &x.val): ok = true case isInteger(x.typ) && isString(t): codepoint := int64(-1) @@ -31,7 +31,7 @@ func (check *Checker) conversion(x *operand, T Type) { x.val = constant.MakeString(string(codepoint)) ok = true } - case x.convertibleTo(check.conf, T): + case x.convertibleTo(check, T): // non-constant conversion x.mode = value ok = true @@ -76,9 +76,12 @@ func (check *Checker) conversion(x *operand, T Type) { // is tricky because we'd have to run updateExprType on the argument first. // (Issue #21982.) -func (x *operand) convertibleTo(conf *Config, T Type) bool { +// convertibleTo reports whether T(x) is valid. +// The check parameter may be nil if convertibleTo is invoked through an +// exported API call, i.e., when all methods have been type-checked. +func (x *operand) convertibleTo(check *Checker, T Type) bool { // "x is assignable to T" - if x.assignableTo(conf, T, nil) { + if x.assignableTo(check, T, nil) { return true } diff --git a/src/go/types/decl.go b/src/go/types/decl.go index 0ff1fb058b..b4a1eec1ac 100644 --- a/src/go/types/decl.go +++ b/src/go/types/decl.go @@ -586,6 +586,10 @@ func (check *Checker) addMethodDecls(obj *TypeName) { } // type-check + // TODO(gri): This call is not needed anymore because the code can handle + // method signatures that have not yet been type-checked. + // Remove in separate CL to make it easy to isolate issues + // that might be introduced by this change. check.objDecl(m, nil) if base != nil { diff --git a/src/go/types/expr.go b/src/go/types/expr.go index fc4de98eb7..87769d1db0 100644 --- a/src/go/types/expr.go +++ b/src/go/types/expr.go @@ -187,11 +187,20 @@ func roundFloat64(x constant.Value) constant.Value { // representable floating-point and complex values, and to an Int // value for integer values; it is left alone otherwise. // It is ok to provide the addressof the first argument for rounded. -func representableConst(x constant.Value, conf *Config, typ *Basic, rounded *constant.Value) bool { +// +// The check parameter may be nil if representableConst is invoked +// (indirectly) through an exported API call (AssignableTo, ConvertibleTo) +// because we don't need the Checker's config for those calls. +func representableConst(x constant.Value, check *Checker, typ *Basic, rounded *constant.Value) bool { if x.Kind() == constant.Unknown { return true // avoid follow-up errors } + var conf *Config + if check != nil { + conf = check.conf + } + switch { case isInteger(typ): x := constant.ToInt(x) @@ -323,7 +332,7 @@ func representableConst(x constant.Value, conf *Config, typ *Basic, rounded *con // representable checks that a constant operand is representable in the given basic type. func (check *Checker) representable(x *operand, typ *Basic) { assert(x.mode == constant_) - if !representableConst(x.val, check.conf, typ, &x.val) { + if !representableConst(x.val, check, typ, &x.val) { var msg string if isNumeric(x.typ) && isNumeric(typ) { // numeric conversion : error msg @@ -576,7 +585,7 @@ func (check *Checker) comparison(x, y *operand, op token.Token) { // spec: "In any comparison, the first operand must be assignable // to the type of the second operand, or vice versa." err := "" - if x.assignableTo(check.conf, y.typ, nil) || y.assignableTo(check.conf, x.typ, nil) { + if x.assignableTo(check, y.typ, nil) || y.assignableTo(check, x.typ, nil) { defined := false switch op { case token.EQL, token.NEQ: @@ -1547,7 +1556,7 @@ func keyVal(x constant.Value) interface{} { // typeAssertion checks that x.(T) is legal; xtyp must be the type of x. func (check *Checker) typeAssertion(pos token.Pos, x *operand, xtyp *Interface, T Type) { - method, wrongType := assertableTo(xtyp, T) + method, wrongType := check.assertableTo(xtyp, T) if method == nil { return } diff --git a/src/go/types/lookup.go b/src/go/types/lookup.go index f31ef9cfe9..e6764f45a0 100644 --- a/src/go/types/lookup.go +++ b/src/go/types/lookup.go @@ -6,6 +6,11 @@ package types +// Internal use of LookupFieldOrMethod: If the obj result is a method +// associated with a concrete (non-interface) type, the method's signature +// may not be fully set up. Call Checker.objDecl(obj, nil) before accessing +// the method's type. + // LookupFieldOrMethod looks up a field or method with given package and name // in T and returns the corresponding *Var or *Func, an index sequence, and a // bool indicating if there were any pointer indirections on the path to the @@ -112,7 +117,7 @@ func lookupFieldOrMethod(T Type, addressable bool, pkg *Package, name string) (o // look for a matching attached method if i, m := lookupMethod(named.methods, pkg, name); m != nil { // potential match - assert(m.typ != nil) + // caution: method may not have a proper signature yet index = concat(e.index, i) if obj != nil || e.multiples { return nil, index, false // collision @@ -248,6 +253,14 @@ func lookupType(m map[Type]int, typ Type) (int, bool) { // x is of interface type V). // func MissingMethod(V Type, T *Interface, static bool) (method *Func, wrongType bool) { + return (*Checker)(nil).missingMethod(V, T, static) +} + +// missingMethod is like MissingMethod but accepts a receiver. +// The receiver may be nil if missingMethod is invoked through +// an exported API call (such as MissingMethod), i.e., when all +// methods have been type-checked. +func (check *Checker) missingMethod(V Type, T *Interface, static bool) (method *Func, wrongType bool) { // fast path for common case if T.Empty() { return @@ -275,11 +288,17 @@ func MissingMethod(V Type, T *Interface, static bool) (method *Func, wrongType b for _, m := range T.allMethods { obj, _, _ := lookupFieldOrMethod(V, false, m.pkg, m.name) + // we must have a method (not a field of matching function type) f, _ := obj.(*Func) if f == nil { return m, false } + // methods may not have a fully set up signature yet + if check != nil { + check.objDecl(f, nil) + } + if !Identical(f.typ, m.typ) { return m, true } @@ -291,14 +310,16 @@ func MissingMethod(V Type, T *Interface, static bool) (method *Func, wrongType b // assertableTo reports whether a value of type V can be asserted to have type T. // It returns (nil, false) as affirmative answer. Otherwise it returns a missing // method required by V and whether it is missing or just has the wrong type. -func assertableTo(V *Interface, T Type) (method *Func, wrongType bool) { +// The receiver may be nil if assertableTo is invoked through an exported API call +// (such as AssertableTo), i.e., when all methods have been type-checked. +func (check *Checker) assertableTo(V *Interface, T Type) (method *Func, wrongType bool) { // no static check is required if T is an interface // spec: "If T is an interface type, x.(T) asserts that the // dynamic type of x implements the interface T." if _, ok := T.Underlying().(*Interface); ok && !strict { return } - return MissingMethod(T, V, false) + return check.missingMethod(T, V, false) } // deref dereferences typ if it is a *Pointer and returns its base and true. diff --git a/src/go/types/operand.go b/src/go/types/operand.go index 07247bd6f5..97ca6c622f 100644 --- a/src/go/types/operand.go +++ b/src/go/types/operand.go @@ -201,7 +201,9 @@ func (x *operand) isNil() bool { // assignableTo reports whether x is assignable to a variable of type T. // If the result is false and a non-nil reason is provided, it may be set // to a more detailed explanation of the failure (result != ""). -func (x *operand) assignableTo(conf *Config, T Type, reason *string) bool { +// The check parameter may be nil if assignableTo is invoked through +// an exported API call, i.e., when all methods have been type-checked. +func (x *operand) assignableTo(check *Checker, T Type, reason *string) bool { if x.mode == invalid || T == Typ[Invalid] { return true // avoid spurious errors } @@ -226,7 +228,7 @@ func (x *operand) assignableTo(conf *Config, T Type, reason *string) bool { return true } if x.mode == constant_ { - return representableConst(x.val, conf, t, nil) + return representableConst(x.val, check, t, nil) } // The result of a comparison is an untyped boolean, // but may not be a constant. @@ -249,7 +251,7 @@ func (x *operand) assignableTo(conf *Config, T Type, reason *string) bool { // T is an interface type and x implements T if Ti, ok := Tu.(*Interface); ok { - if m, wrongType := MissingMethod(x.typ, Ti, true); m != nil /* Implements(x.typ, Ti) */ { + if m, wrongType := check.missingMethod(x.typ, Ti, true); m != nil /* Implements(x.typ, Ti) */ { if reason != nil { if wrongType { *reason = "wrong type for method " + m.Name() diff --git a/src/go/types/type.go b/src/go/types/type.go index d9399a6587..74b6bcfd67 100644 --- a/src/go/types/type.go +++ b/src/go/types/type.go @@ -424,7 +424,7 @@ func (c *Chan) Elem() Type { return c.elem } type Named struct { obj *TypeName // corresponding declared object underlying Type // possibly a *Named during setup; never a *Named once set up completely - methods []*Func // methods declared for this type (not the method set of this type) + methods []*Func // methods declared for this type (not the method set of this type); signatures are type-checked lazily } // NewNamed returns a new named type for the given type name, underlying type, and associated methods. diff --git a/src/go/types/typexpr.go b/src/go/types/typexpr.go index bcdbc5906d..eb0d8e8fb9 100644 --- a/src/go/types/typexpr.go +++ b/src/go/types/typexpr.go @@ -391,7 +391,7 @@ func (check *Checker) arrayLength(e ast.Expr) int64 { } if isUntyped(x.typ) || isInteger(x.typ) { if val := constant.ToInt(x.val); val.Kind() == constant.Int { - if representableConst(val, check.conf, Typ[Int], nil) { + if representableConst(val, check, Typ[Int], nil) { if n, ok := constant.Int64Val(val); ok && n >= 0 { return n } -- 2.50.0