From d5ae7a64876630ab40f7bd04fdb5ad6a3733dae7 Mon Sep 17 00:00:00 2001 From: Robert Findley Date: Sat, 7 May 2022 15:50:05 -0400 Subject: [PATCH] go/types, types2: remove redundant calls to Named.resolve The resolved status of a Named type should be owned by its API, and callers should access resolved data via methods. Remove several instances where Named.resolve is explicitly invoked, only to be followed by a method that also resolves. Also make two minor cleanups: - Remove the tparams parameter to Checker.newNamed, as it was unused. - Include position information when assertions fail, so that one doesn't need to go digging in the panicking stack to find the assertion location. Updates #52728 Change-Id: Icbe8c89e9cfe02d60af7d9ba907eaebe1f00193e Reviewed-on: https://go-review.googlesource.com/c/go/+/404874 Run-TryBot: Robert Findley Reviewed-by: Robert Griesemer TryBot-Result: Gopher Robot --- src/cmd/compile/internal/types2/decl.go | 3 +-- src/cmd/compile/internal/types2/errors.go | 9 ++++++++- src/cmd/compile/internal/types2/instantiate.go | 2 +- src/cmd/compile/internal/types2/lookup.go | 1 - src/cmd/compile/internal/types2/named.go | 6 +++--- src/cmd/compile/internal/types2/signature.go | 1 - src/cmd/compile/internal/types2/subst.go | 1 - src/cmd/compile/internal/types2/typexpr.go | 2 +- src/go/types/decl.go | 3 +-- src/go/types/errors.go | 9 ++++++++- src/go/types/instantiate.go | 2 +- src/go/types/lookup.go | 1 - src/go/types/named.go | 6 +++--- src/go/types/signature.go | 1 - src/go/types/subst.go | 1 - src/go/types/typexpr.go | 2 +- 16 files changed, 28 insertions(+), 22 deletions(-) diff --git a/src/cmd/compile/internal/types2/decl.go b/src/cmd/compile/internal/types2/decl.go index 9176358dd5..b6f81aa8a5 100644 --- a/src/cmd/compile/internal/types2/decl.go +++ b/src/cmd/compile/internal/types2/decl.go @@ -508,7 +508,7 @@ func (check *Checker) typeDecl(obj *TypeName, tdecl *syntax.TypeDecl, def *Named } // type definition or generic type declaration - named := check.newNamed(obj, nil, nil, nil, nil) + named := check.newNamed(obj, nil, nil, nil) def.setUnderlying(named) if tdecl.TParamList != nil { @@ -671,7 +671,6 @@ func (check *Checker) collectMethods(obj *TypeName) { } if base != nil { - base.resolve(nil) // TODO(mdempsky): Probably unnecessary. base.AddMethod(m) } } diff --git a/src/cmd/compile/internal/types2/errors.go b/src/cmd/compile/internal/types2/errors.go index 422f520795..2a3e88a2fe 100644 --- a/src/cmd/compile/internal/types2/errors.go +++ b/src/cmd/compile/internal/types2/errors.go @@ -10,6 +10,7 @@ import ( "bytes" "cmd/compile/internal/syntax" "fmt" + "runtime" "strconv" "strings" ) @@ -20,7 +21,13 @@ func unimplemented() { func assert(p bool) { if !p { - panic("assertion failed") + msg := "assertion failed" + // Include information about the assertion location. Due to panic recovery, + // this location is otherwise buried in the middle of the panicking stack. + if _, file, line, ok := runtime.Caller(1); ok { + msg = fmt.Sprintf("%s:%d: %s", file, line, msg) + } + panic(msg) } } diff --git a/src/cmd/compile/internal/types2/instantiate.go b/src/cmd/compile/internal/types2/instantiate.go index a69a26ba64..bb90ab3736 100644 --- a/src/cmd/compile/internal/types2/instantiate.go +++ b/src/cmd/compile/internal/types2/instantiate.go @@ -77,7 +77,7 @@ func (check *Checker) instance(pos syntax.Pos, orig Type, targs []Type, ctxt *Co switch orig := orig.(type) { case *Named: tname := NewTypeName(pos, orig.obj.pkg, orig.obj.name, nil) - named := check.newNamed(tname, orig, nil, nil, nil) // underlying, tparams, and methods are set when named is resolved + named := check.newNamed(tname, orig, nil, nil) // underlying, tparams, and methods are set when named is resolved named.targs = newTypeList(targs) named.resolver = func(ctxt *Context, n *Named) (*TypeParamList, Type, *methodList) { return expandNamed(ctxt, n, pos) diff --git a/src/cmd/compile/internal/types2/lookup.go b/src/cmd/compile/internal/types2/lookup.go index 482b6bd8ef..42cd338e24 100644 --- a/src/cmd/compile/internal/types2/lookup.go +++ b/src/cmd/compile/internal/types2/lookup.go @@ -134,7 +134,6 @@ func lookupFieldOrMethod(T Type, addressable bool, pkg *Package, name string, fo seen.add(named) // look for a matching attached method - named.resolve(nil) if i, m := named.lookupMethod(pkg, name, foldCase); m != nil { // potential match // caution: method may not have a proper signature yet diff --git a/src/cmd/compile/internal/types2/named.go b/src/cmd/compile/internal/types2/named.go index 0a150a451c..849398a6f4 100644 --- a/src/cmd/compile/internal/types2/named.go +++ b/src/cmd/compile/internal/types2/named.go @@ -38,7 +38,7 @@ func NewNamed(obj *TypeName, underlying Type, methods []*Func) *Named { if _, ok := underlying.(*Named); ok { panic("underlying type must not be *Named") } - return (*Checker)(nil).newNamed(obj, nil, underlying, nil, newMethodList(methods)) + return (*Checker)(nil).newNamed(obj, nil, underlying, newMethodList(methods)) } func (t *Named) resolve(ctxt *Context) *Named { @@ -62,8 +62,8 @@ func (t *Named) resolve(ctxt *Context) *Named { } // newNamed is like NewNamed but with a *Checker receiver and additional orig argument. -func (check *Checker) newNamed(obj *TypeName, orig *Named, underlying Type, tparams *TypeParamList, methods *methodList) *Named { - typ := &Named{check: check, obj: obj, orig: orig, fromRHS: underlying, underlying: underlying, tparams: tparams, methods: methods} +func (check *Checker) newNamed(obj *TypeName, orig *Named, underlying Type, methods *methodList) *Named { + typ := &Named{check: check, obj: obj, orig: orig, fromRHS: underlying, underlying: underlying, methods: methods} if typ.orig == nil { typ.orig = typ } diff --git a/src/cmd/compile/internal/types2/signature.go b/src/cmd/compile/internal/types2/signature.go index 2dc4dd43f3..92d3aadf88 100644 --- a/src/cmd/compile/internal/types2/signature.go +++ b/src/cmd/compile/internal/types2/signature.go @@ -209,7 +209,6 @@ func (check *Checker) funcType(sig *Signature, recvPar *syntax.Field, tparams [] // as the method." switch T := rtyp.(type) { case *Named: - T.resolve(check.bestContext(nil)) // The receiver type may be an instantiated type referred to // by an alias (which cannot have receiver parameters for now). if T.TypeArgs() != nil && sig.RecvTypeParams() == nil { diff --git a/src/cmd/compile/internal/types2/subst.go b/src/cmd/compile/internal/types2/subst.go index 6cbe57dab0..6e41ebdf53 100644 --- a/src/cmd/compile/internal/types2/subst.go +++ b/src/cmd/compile/internal/types2/subst.go @@ -258,7 +258,6 @@ func (subst *subster) typ(typ Type) Type { // recursion. The position used here is irrelevant because validation only // occurs on t (we don't call validType on named), but we use subst.pos to // help with debugging. - t.orig.resolve(subst.ctxt) return subst.check.instance(subst.pos, t.orig, newTArgs, subst.ctxt) // Note that if we were to expose substitution more generally (not just in diff --git a/src/cmd/compile/internal/types2/typexpr.go b/src/cmd/compile/internal/types2/typexpr.go index 1610f8ff8f..1f8b40dba6 100644 --- a/src/cmd/compile/internal/types2/typexpr.go +++ b/src/cmd/compile/internal/types2/typexpr.go @@ -448,7 +448,7 @@ func (check *Checker) instantiatedType(x syntax.Expr, xlist []syntax.Expr, def * if inst == nil { // x may be a selector for an imported type; use its start pos rather than x.Pos(). tname := NewTypeName(syntax.StartPos(x), orig.obj.pkg, orig.obj.name, nil) - inst = check.newNamed(tname, orig, nil, nil, nil) // underlying, methods and tparams are set when named is resolved + inst = check.newNamed(tname, orig, nil, nil) // underlying, methods and tparams are set when named is resolved inst.targs = newTypeList(targs) inst = ctxt.update(h, orig, targs, inst).(*Named) } diff --git a/src/go/types/decl.go b/src/go/types/decl.go index 7229104190..123d296791 100644 --- a/src/go/types/decl.go +++ b/src/go/types/decl.go @@ -565,7 +565,7 @@ func (check *Checker) typeDecl(obj *TypeName, tdecl *ast.TypeSpec, def *Named) { } // type definition or generic type declaration - named := check.newNamed(obj, nil, nil, nil, nil) + named := check.newNamed(obj, nil, nil, nil) def.setUnderlying(named) if tdecl.TypeParams != nil { @@ -741,7 +741,6 @@ func (check *Checker) collectMethods(obj *TypeName) { } if base != nil { - base.resolve(nil) // TODO(mdempsky): Probably unnecessary. base.AddMethod(m) } } diff --git a/src/go/types/errors.go b/src/go/types/errors.go index f3cb249f5e..964f377984 100644 --- a/src/go/types/errors.go +++ b/src/go/types/errors.go @@ -11,13 +11,20 @@ import ( "fmt" "go/ast" "go/token" + "runtime" "strconv" "strings" ) func assert(p bool) { if !p { - panic("assertion failed") + msg := "assertion failed" + // Include information about the assertion location. Due to panic recovery, + // this location is otherwise buried in the middle of the panicking stack. + if _, file, line, ok := runtime.Caller(1); ok { + msg = fmt.Sprintf("%s:%d: %s", file, line, msg) + } + panic(msg) } } diff --git a/src/go/types/instantiate.go b/src/go/types/instantiate.go index 4450847dfd..964a4f907c 100644 --- a/src/go/types/instantiate.go +++ b/src/go/types/instantiate.go @@ -77,7 +77,7 @@ func (check *Checker) instance(pos token.Pos, orig Type, targs []Type, ctxt *Con switch orig := orig.(type) { case *Named: tname := NewTypeName(pos, orig.obj.pkg, orig.obj.name, nil) - named := check.newNamed(tname, orig, nil, nil, nil) // underlying, tparams, and methods are set when named is resolved + named := check.newNamed(tname, orig, nil, nil) // underlying, tparams, and methods are set when named is resolved named.targs = newTypeList(targs) named.resolver = func(ctxt *Context, n *Named) (*TypeParamList, Type, *methodList) { return expandNamed(ctxt, n, pos) diff --git a/src/go/types/lookup.go b/src/go/types/lookup.go index 22a62055d3..305b2003f7 100644 --- a/src/go/types/lookup.go +++ b/src/go/types/lookup.go @@ -134,7 +134,6 @@ func lookupFieldOrMethod(T Type, addressable bool, pkg *Package, name string, fo seen.add(named) // look for a matching attached method - named.resolve(nil) if i, m := named.lookupMethod(pkg, name, foldCase); m != nil { // potential match // caution: method may not have a proper signature yet diff --git a/src/go/types/named.go b/src/go/types/named.go index f8d319a5ec..a82679eb10 100644 --- a/src/go/types/named.go +++ b/src/go/types/named.go @@ -38,7 +38,7 @@ func NewNamed(obj *TypeName, underlying Type, methods []*Func) *Named { if _, ok := underlying.(*Named); ok { panic("underlying type must not be *Named") } - return (*Checker)(nil).newNamed(obj, nil, underlying, nil, newMethodList(methods)) + return (*Checker)(nil).newNamed(obj, nil, underlying, newMethodList(methods)) } func (t *Named) resolve(ctxt *Context) *Named { @@ -62,8 +62,8 @@ func (t *Named) resolve(ctxt *Context) *Named { } // newNamed is like NewNamed but with a *Checker receiver and additional orig argument. -func (check *Checker) newNamed(obj *TypeName, orig *Named, underlying Type, tparams *TypeParamList, methods *methodList) *Named { - typ := &Named{check: check, obj: obj, orig: orig, fromRHS: underlying, underlying: underlying, tparams: tparams, methods: methods} +func (check *Checker) newNamed(obj *TypeName, orig *Named, underlying Type, methods *methodList) *Named { + typ := &Named{check: check, obj: obj, orig: orig, fromRHS: underlying, underlying: underlying, methods: methods} if typ.orig == nil { typ.orig = typ } diff --git a/src/go/types/signature.go b/src/go/types/signature.go index 9e7b63b451..4b63f0e6f0 100644 --- a/src/go/types/signature.go +++ b/src/go/types/signature.go @@ -216,7 +216,6 @@ func (check *Checker) funcType(sig *Signature, recvPar *ast.FieldList, ftyp *ast // as the method." switch T := rtyp.(type) { case *Named: - T.resolve(check.bestContext(nil)) // The receiver type may be an instantiated type referred to // by an alias (which cannot have receiver parameters for now). if T.TypeArgs() != nil && sig.RecvTypeParams() == nil { diff --git a/src/go/types/subst.go b/src/go/types/subst.go index b1794ac32d..63849b9212 100644 --- a/src/go/types/subst.go +++ b/src/go/types/subst.go @@ -258,7 +258,6 @@ func (subst *subster) typ(typ Type) Type { // recursion. The position used here is irrelevant because validation only // occurs on t (we don't call validType on named), but we use subst.pos to // help with debugging. - t.orig.resolve(subst.ctxt) return subst.check.instance(subst.pos, t.orig, newTArgs, subst.ctxt) // Note that if we were to expose substitution more generally (not just in diff --git a/src/go/types/typexpr.go b/src/go/types/typexpr.go index d5fe9a5cc6..7afc66a925 100644 --- a/src/go/types/typexpr.go +++ b/src/go/types/typexpr.go @@ -433,7 +433,7 @@ func (check *Checker) instantiatedType(ix *typeparams.IndexExpr, def *Named) (re if inst == nil { // x may be a selector for an imported type; use its start pos rather than x.Pos(). tname := NewTypeName(ix.Pos(), orig.obj.pkg, orig.obj.name, nil) - inst = check.newNamed(tname, orig, nil, nil, nil) // underlying, methods and tparams are set when named is resolved + inst = check.newNamed(tname, orig, nil, nil) // underlying, methods and tparams are set when named is resolved inst.targs = newTypeList(targs) inst = ctxt.update(h, orig, targs, inst).(*Named) } -- 2.50.0