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

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 <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Robert Griesemer <[email protected]>

Author: findleyr

src/cmd/compile/internal/types2/api.go

@@ -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.

src/cmd/compile/internal/types2/instantiate.go

@@ -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,44 +205,58 @@ 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)
 
 	// An interface V with an empty type set satisfies any interface.
 	// (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()
 }

src/cmd/compile/internal/types2/lookup.go

@@ -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.

src/cmd/compile/internal/types2/operand.go

@@ -288,18 +288,15 @@ 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
 	}
 
 	// 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"

src/go/types/api.go

@@ -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.

src/go/types/instantiate.go

@@ -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,44 +205,58 @@ 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)
 
 	// An interface V with an empty type set satisfies any interface.
 	// (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()
 }

src/go/types/lookup.go

@@ -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.