qbit/go
1
0
Fork 0
mirror of https://github.com/golang/go synced 2024-11-26 15:56:57 -07:00
Code Releases Activity

go/types, types2: don't build unnecessary error strings in implements

Browse Source 
When accessing (*Checker).implements from types.AssignableTo or
types.ComparableTo, we don't need to build error strings -- they won't
be used.

This string manipulation showed up as a hot spot in gopls completion,
which checks a lot of type predicates when searching for candidate
completions.

This CL yields the following results for gopls' completion benchmarks:

StructCompletion-8         24.7ms ±34%  26.0ms ±17%     ~     (p=0.447 n=10+9)
ImportCompletion-8         1.41ms ± 2%  1.45ms ± 4%   +2.42%  (p=0.027 n=8+9)
SliceCompletion-8          27.0ms ±18%  25.2ms ± 3%   -6.67%  (p=0.008 n=9+8)
FuncDeepCompletion-8       57.6ms ± 4%  22.4ms ± 4%  -61.18%  (p=0.000 n=8+9)
CompletionFollowingEdit-8   157ms ±13%   103ms ±15%  -34.70%  (p=0.000 n=10+10)

Notably, deep completion (which searches many candidates) is almost 3x
faster after this change.

Fixes #54172

Change-Id: If8303a411aed3a20bd91f7b61e346d703084166c
Reviewed-on: https://go-review.googlesource.com/c/go/+/423360
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
This commit is contained in:
Robert Findley 2022-08-12 17:51:55 -04:00
parent 7b45edb450
commit de0f4d190f
8 changed files with 112 additions and 80 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/cmd/compile/internal/types2/api.go
View File

@ -429,7 +429,7 @@ func AssertableTo(V *Interface, T Type) bool {
if T.Underlying() == Typ[Invalid] {
return false
}
return (*Checker)(nil).newAssertableTo(V, T) == nil
return (*Checker)(nil).newAssertableTo(V, T)
}
// AssignableTo reports whether a value of type V is assignable to a variable
@ -467,7 +467,7 @@ func Implements(V Type, T *Interface) bool {
if V.Underlying() == Typ[Invalid] {
return false
}
return (*Checker)(nil).implements(V, T) == nil
return (*Checker)(nil).implements(V, T, nil)
}
// Identical reports whether x and y are identical types.

79
src/cmd/compile/internal/types2/instantiate.go
View File

@ -174,28 +174,27 @@ func (check *Checker) verify(pos syntax.Pos, tparams []*TypeParam, targs []Type,
// need to instantiate it with the type arguments with which we instantiated
// the parameterized type.
bound := check.subst(pos, tpar.bound, smap, nil, ctxt)
if err := check.implements(targs[i], bound); err != nil {
return i, err
var reason string
if !check.implements(targs[i], bound, &reason) {
return i, errors.New(reason)
}
}
return -1, nil
}
// implements checks if V implements T and reports an error if it doesn't.
// The receiver may be nil if implements is called through an exported
// API call such as AssignableTo.
func (check *Checker) implements(V, T Type) error {
// implements checks if V implements T. The receiver may be nil if implements
// is called through an exported API call such as AssignableTo.
//
// If the provided reason is non-nil, it may be set to an error string
// explaining why V does not implement T.
func (check *Checker) implements(V, T Type, reason *string) bool {
Vu := under(V)
Tu := under(T)
if Vu == Typ[Invalid] || Tu == Typ[Invalid] {
return nil // avoid follow-on errors
return true // avoid follow-on errors
}
if p, _ := Vu.(*Pointer); p != nil && under(p.base) == Typ[Invalid] {
return nil // avoid follow-on errors (see issue #49541 for an example)
}
errorf := func(format string, args ...interface{}) error {
return errors.New(check.sprintf(format, args...))
return true // avoid follow-on errors (see issue #49541 for an example)
}
Ti, _ := Tu.(*Interface)
@ -206,12 +205,15 @@ func (check *Checker) implements(V, T Type) error {
} else {
cause = check.sprintf("%s is not an interface", T)
}
return errorf("%s does not implement %s (%s)", V, T, cause)
if reason != nil {
*reason = check.sprintf("%s does not implement %s (%s)", V, T, cause)
}
return false
}
// Every type satisfies the empty interface.
if Ti.Empty() {
return nil
return true
}
// T is not the empty interface (i.e., the type set of T is restricted)
@ -219,31 +221,42 @@ func (check *Checker) implements(V, T Type) error {
// (The empty set is a subset of any set.)
Vi, _ := Vu.(*Interface)
if Vi != nil && Vi.typeSet().IsEmpty() {
return nil
return true
}
// type set of V is not empty
// No type with non-empty type set satisfies the empty type set.
if Ti.typeSet().IsEmpty() {
return errorf("cannot implement %s (empty type set)", T)
if reason != nil {
*reason = check.sprintf("cannot implement %s (empty type set)", T)
}
return false
}
// V must implement T's methods, if any.
if m, wrong := check.missingMethod(V, Ti, true); m != nil /* !Implements(V, Ti) */ {
return errorf("%s does not implement %s %s", V, T, check.missingMethodReason(V, T, m, wrong))
if reason != nil {
*reason = check.sprintf("%s does not implement %s %s", V, T, check.missingMethodReason(V, T, m, wrong))
}
return false
}
// If T is comparable, V must be comparable.
// Remember as a pending error and report only if we don't have a more specific error.
var pending error
if Ti.IsComparable() && !comparable(V, false, nil, nil) {
pending = errorf("%s does not implement comparable", V)
// Only check comparability if we don't have a more specific error.
checkComparability := func() bool {
// If T is comparable, V must be comparable.
if Ti.IsComparable() && !comparable(V, false, nil, nil) {
if reason != nil {
*reason = check.sprintf("%s does not implement comparable", V)
}
return false
}
return true
}
// V must also be in the set of types of T, if any.
// Constraints with empty type sets were already excluded above.
if !Ti.typeSet().hasTerms() {
return pending // nothing to do
return checkComparability() // nothing to do
}
// If V is itself an interface, each of its possible types must be in the set
@ -252,9 +265,12 @@ func (check *Checker) implements(V, T Type) error {
if Vi != nil {
if !Vi.typeSet().subsetOf(Ti.typeSet()) {
// TODO(gri) report which type is missing
return errorf("%s does not implement %s", V, T)
if reason != nil {
*reason = check.sprintf("%s does not implement %s", V, T)
}
return false
}
return pending
return checkComparability()
}
// Otherwise, V's type must be included in the iface type set.
@ -275,12 +291,15 @@ func (check *Checker) implements(V, T Type) error {
}
return false
}) {
if alt != nil {
return errorf("%s does not implement %s (possibly missing ~ for %s in constraint %s)", V, T, alt, T)
} else {
return errorf("%s does not implement %s (%s missing in %s)", V, T, V, Ti.typeSet().terms)
if reason != nil {
if alt != nil {
*reason = check.sprintf("%s does not implement %s (possibly missing ~ for %s in constraint %s)", V, T, alt, T)
} else {
*reason = check.sprintf("%s does not implement %s (%s missing in %s)", V, T, V, Ti.typeSet().terms)
}
}
return false
}
return pending
return checkComparability()
}

6
src/cmd/compile/internal/types2/lookup.go
View File

@ -449,14 +449,14 @@ func (check *Checker) assertableTo(V *Interface, T Type) (method, wrongType *Fun
// newAssertableTo reports whether a value of type V can be asserted to have type T.
// It also implements behavior for interfaces that currently are only permitted
// in constraint position (we have not yet defined that behavior in the spec).
func (check *Checker) newAssertableTo(V *Interface, T Type) error {
func (check *Checker) newAssertableTo(V *Interface, T Type) 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 IsInterface(T) {
return nil
return true
}
return check.implements(T, V)
return check.implements(T, V, nil)
}
// deref dereferences typ if it is a *Pointer and returns its base and true.

7
src/cmd/compile/internal/types2/operand.go
View File

@ -288,10 +288,7 @@ func (x *operand) assignableTo(check *Checker, T Type, reason *string) (bool, er
// T is an interface type and x implements T and T is not a type parameter.
// Also handle the case where T is a pointer to an interface.
if _, ok := Tu.(*Interface); ok && Tp == nil || isInterfacePtr(Tu) {
if err := check.implements(V, T); err != nil {
if reason != nil {
*reason = err.Error()
}
if !check.implements(V, T, reason) {
return false, _InvalidIfaceAssign
}
return true, 0
@ -299,7 +296,7 @@ func (x *operand) assignableTo(check *Checker, T Type, reason *string) (bool, er
// If V is an interface, check if a missing type assertion is the problem.
if Vi, _ := Vu.(*Interface); Vi != nil && Vp == nil {
if check.implements(T, V) == nil {
if check.implements(T, V, nil) {
// T implements V, so give hint about type assertion.
if reason != nil {
*reason = "need type assertion"

4
src/go/types/api.go
View File

@ -424,7 +424,7 @@ func AssertableTo(V *Interface, T Type) bool {
if T.Underlying() == Typ[Invalid] {
return false
}
return (*Checker)(nil).newAssertableTo(V, T) == nil
return (*Checker)(nil).newAssertableTo(V, T)
}
// AssignableTo reports whether a value of type V is assignable to a variable
@ -462,7 +462,7 @@ func Implements(V Type, T *Interface) bool {
if V.Underlying() == Typ[Invalid] {
return false
}
return (*Checker)(nil).implements(V, T) == nil
return (*Checker)(nil).implements(V, T, nil)
}
// Identical reports whether x and y are identical types.

79
src/go/types/instantiate.go
View File

@ -174,28 +174,27 @@ func (check *Checker) verify(pos token.Pos, tparams []*TypeParam, targs []Type,
// need to instantiate it with the type arguments with which we instantiated
// the parameterized type.
bound := check.subst(pos, tpar.bound, smap, nil, ctxt)
if err := check.implements(targs[i], bound); err != nil {
return i, err
var reason string
if !check.implements(targs[i], bound, &reason) {
return i, errors.New(reason)
}
}
return -1, nil
}
// implements checks if V implements T and reports an error if it doesn't.
// The receiver may be nil if implements is called through an exported
// API call such as AssignableTo.
func (check *Checker) implements(V, T Type) error {
// implements checks if V implements T. The receiver may be nil if implements
// is called through an exported API call such as AssignableTo.
//
// If the provided reason is non-nil, it may be set to an error string
// explaining why V does not implement T.
func (check *Checker) implements(V, T Type, reason *string) bool {
Vu := under(V)
Tu := under(T)
if Vu == Typ[Invalid] || Tu == Typ[Invalid] {
return nil // avoid follow-on errors
return true // avoid follow-on errors
}
if p, _ := Vu.(*Pointer); p != nil && under(p.base) == Typ[Invalid] {
return nil // avoid follow-on errors (see issue #49541 for an example)
}
errorf := func(format string, args ...any) error {
return errors.New(check.sprintf(format, args...))
return true // avoid follow-on errors (see issue #49541 for an example)
}
Ti, _ := Tu.(*Interface)
@ -206,12 +205,15 @@ func (check *Checker) implements(V, T Type) error {
} else {
cause = check.sprintf("%s is not an interface", T)
}
return errorf("%s does not implement %s (%s)", V, T, cause)
if reason != nil {
*reason = check.sprintf("%s does not implement %s (%s)", V, T, cause)
}
return false
}
// Every type satisfies the empty interface.
if Ti.Empty() {
return nil
return true
}
// T is not the empty interface (i.e., the type set of T is restricted)
@ -219,31 +221,42 @@ func (check *Checker) implements(V, T Type) error {
// (The empty set is a subset of any set.)
Vi, _ := Vu.(*Interface)
if Vi != nil && Vi.typeSet().IsEmpty() {
return nil
return true
}
// type set of V is not empty
// No type with non-empty type set satisfies the empty type set.
if Ti.typeSet().IsEmpty() {
return errorf("cannot implement %s (empty type set)", T)
if reason != nil {
*reason = check.sprintf("cannot implement %s (empty type set)", T)
}
return false
}
// V must implement T's methods, if any.
if m, wrong := check.missingMethod(V, Ti, true); m != nil /* !Implements(V, Ti) */ {
return errorf("%s does not implement %s %s", V, T, check.missingMethodReason(V, T, m, wrong))
if reason != nil {
*reason = check.sprintf("%s does not implement %s %s", V, T, check.missingMethodReason(V, T, m, wrong))
}
return false
}
// If T is comparable, V must be comparable.
// Remember as a pending error and report only if we don't have a more specific error.
var pending error
if Ti.IsComparable() && !comparable(V, false, nil, nil) {
pending = errorf("%s does not implement comparable", V)
// Only check comparability if we don't have a more specific error.
checkComparability := func() bool {
// If T is comparable, V must be comparable.
if Ti.IsComparable() && !comparable(V, false, nil, nil) {
if reason != nil {
*reason = check.sprintf("%s does not implement comparable", V)
}
return false
}
return true
}
// V must also be in the set of types of T, if any.
// Constraints with empty type sets were already excluded above.
if !Ti.typeSet().hasTerms() {
return pending // nothing to do
return checkComparability() // nothing to do
}
// If V is itself an interface, each of its possible types must be in the set
@ -252,9 +265,12 @@ func (check *Checker) implements(V, T Type) error {
if Vi != nil {
if !Vi.typeSet().subsetOf(Ti.typeSet()) {
// TODO(gri) report which type is missing
return errorf("%s does not implement %s", V, T)
if reason != nil {
*reason = check.sprintf("%s does not implement %s", V, T)
}
return false
}
return pending
return checkComparability()
}
// Otherwise, V's type must be included in the iface type set.
@ -275,12 +291,15 @@ func (check *Checker) implements(V, T Type) error {
}
return false
}) {
if alt != nil {
return errorf("%s does not implement %s (possibly missing ~ for %s in constraint %s)", V, T, alt, T)
} else {
return errorf("%s does not implement %s (%s missing in %s)", V, T, V, Ti.typeSet().terms)
if reason != nil {
if alt != nil {
*reason = check.sprintf("%s does not implement %s (possibly missing ~ for %s in constraint %s)", V, T, alt, T)
} else {
*reason = check.sprintf("%s does not implement %s (%s missing in %s)", V, T, V, Ti.typeSet().terms)
}
}
return false
}
return pending
return checkComparability()
}

6
src/go/types/lookup.go
View File

@ -448,14 +448,14 @@ func (check *Checker) assertableTo(V *Interface, T Type) (method, wrongType *Fun
// newAssertableTo reports whether a value of type V can be asserted to have type T.
// It also implements behavior for interfaces that currently are only permitted
// in constraint position (we have not yet defined that behavior in the spec).
func (check *Checker) newAssertableTo(V *Interface, T Type) error {
func (check *Checker) newAssertableTo(V *Interface, T Type) 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 IsInterface(T) {
return nil
return true
}
return check.implements(T, V)
return check.implements(T, V, nil)
}
// deref dereferences typ if it is a *Pointer and returns its base and true.

7
src/go/types/operand.go
View File

@ -277,10 +277,7 @@ func (x *operand) assignableTo(check *Checker, T Type, reason *string) (bool, er
// T is an interface type and x implements T and T is not a type parameter.
// Also handle the case where T is a pointer to an interface.
if _, ok := Tu.(*Interface); ok && Tp == nil || isInterfacePtr(Tu) {
if err := check.implements(V, T); err != nil {
if reason != nil {
*reason = err.Error()
}
if !check.implements(V, T, reason) {
return false, _InvalidIfaceAssign
}
return true, 0
@ -288,7 +285,7 @@ func (x *operand) assignableTo(check *Checker, T Type, reason *string) (bool, er
// If V is an interface, check if a missing type assertion is the problem.
if Vi, _ := Vu.(*Interface); Vi != nil && Vp == nil {
if check.implements(T, V) == nil {
if check.implements(T, V, nil) {
// T implements V, so give hint about type assertion.
if reason != nil {
*reason = "need type assertion"