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 <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/src/cmd/compile/internal/types2/decl.go b/src/cmd/compile/internal/types2/decl.go
index 9176358..b6f81aa 100644
--- a/src/cmd/compile/internal/types2/decl.go
+++ b/src/cmd/compile/internal/types2/decl.go
@@ -508,7 +508,7 @@
}
// 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 @@
}
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 422f520..2a3e88a 100644
--- a/src/cmd/compile/internal/types2/errors.go
+++ b/src/cmd/compile/internal/types2/errors.go
@@ -10,6 +10,7 @@
"bytes"
"cmd/compile/internal/syntax"
"fmt"
+ "runtime"
"strconv"
"strings"
)
@@ -20,7 +21,13 @@
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 a69a26b..bb90ab3 100644
--- a/src/cmd/compile/internal/types2/instantiate.go
+++ b/src/cmd/compile/internal/types2/instantiate.go
@@ -77,7 +77,7 @@
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 482b6bd..42cd338 100644
--- a/src/cmd/compile/internal/types2/lookup.go
+++ b/src/cmd/compile/internal/types2/lookup.go
@@ -134,7 +134,6 @@
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 0a150a4..849398a 100644
--- a/src/cmd/compile/internal/types2/named.go
+++ b/src/cmd/compile/internal/types2/named.go
@@ -38,7 +38,7 @@
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 @@
}
// 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 2dc4dd4..92d3aad 100644
--- a/src/cmd/compile/internal/types2/signature.go
+++ b/src/cmd/compile/internal/types2/signature.go
@@ -209,7 +209,6 @@
// 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 6cbe57d..6e41ebd 100644
--- a/src/cmd/compile/internal/types2/subst.go
+++ b/src/cmd/compile/internal/types2/subst.go
@@ -258,7 +258,6 @@
// 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 1610f8f..1f8b40d 100644
--- a/src/cmd/compile/internal/types2/typexpr.go
+++ b/src/cmd/compile/internal/types2/typexpr.go
@@ -448,7 +448,7 @@
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 7229104..123d296 100644
--- a/src/go/types/decl.go
+++ b/src/go/types/decl.go
@@ -565,7 +565,7 @@
}
// 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 @@
}
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 f3cb249..964f377 100644
--- a/src/go/types/errors.go
+++ b/src/go/types/errors.go
@@ -11,13 +11,20 @@
"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 4450847..964a4f9 100644
--- a/src/go/types/instantiate.go
+++ b/src/go/types/instantiate.go
@@ -77,7 +77,7 @@
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 22a6205..305b200 100644
--- a/src/go/types/lookup.go
+++ b/src/go/types/lookup.go
@@ -134,7 +134,6 @@
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 f8d319a..a82679e 100644
--- a/src/go/types/named.go
+++ b/src/go/types/named.go
@@ -38,7 +38,7 @@
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 @@
}
// 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 9e7b63b..4b63f0e 100644
--- a/src/go/types/signature.go
+++ b/src/go/types/signature.go
@@ -216,7 +216,6 @@
// 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 b1794ac..63849b9 100644
--- a/src/go/types/subst.go
+++ b/src/go/types/subst.go
@@ -258,7 +258,6 @@
// 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 d5fe9a5..7afc66a 100644
--- a/src/go/types/typexpr.go
+++ b/src/go/types/typexpr.go
@@ -433,7 +433,7 @@
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)
}
