go/types, types2: use new type inference algorithm exclusively

The primary change is that type inference now always reports
an error if a unification step fails (rather than ignoring that
case, see infer2.go). This brings the implementation closely to
the description in #58650; but the implementation is more direct
by always maintaining a simple (type parameter => type) mapping.

To make this work, there are two small but subtle changes in the
unifier:

1) When deciding whether to proceed with the underlying type of
   a defined type, we also use the underlying type if the other
   type is a basic type (switch from !hasName(x) to isTypeLit(x)
   in unifier.go). This makes the case in issue #53650 work out.
   See the comment in the code for a detailed explanation of this
   change.

2) When we unify against an unbound type parameter, we always
   proceed with its core type (if any).
   Again, see the comment in the code for a detailed explanation
   of this change.

The remaining changes are comment and test adjustments. Because
the new logic now results in failing type inference where it
succeeded before or vice versa, and then instatiation or parameter
passing failed, a handful of error messages changed.
As desired, we still have the same number of errors for the same
programs.

Also, because type inference now produces different results, we
cannot easily compare against infer1 anymore (also infer1 won't
work correctly anymore due to the changes in the unifier). This
comparison (together with infer1) is now disabled.

Because some errors and their positions have changed, we need a
slightly larger error position tolerance for types2 (which produces
less accurate error positions than go/types). Hence the change in
types2/check_test.go.

Finally, because type inference is now slightly more relaxed,
issue #51139 doesn't produce a type unification failure anymore
for a (previously correctly) inferred type argument.

Fixes #51139.

Change-Id: Id796eea42f1b706a248843ad855d9d429d077bd1
Reviewed-on: https://go-review.googlesource.com/c/go/+/470916
Reviewed-by: Robert Griesemer <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
Run-TryBot: Robert Griesemer <[email protected]>
Auto-Submit: Robert Griesemer <[email protected]>

Author: griesemer

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

@@ -326,7 +326,7 @@ func TestCheck(t *testing.T) {
 }
 func TestSpec(t *testing.T) { testDirFiles(t, "../../../../internal/types/testdata/spec", 0, false) }
 func TestExamples(t *testing.T) {
-	testDirFiles(t, "../../../../internal/types/testdata/examples", 50, false)
+	testDirFiles(t, "../../../../internal/types/testdata/examples", 60, false)
 } // TODO(gri) narrow column tolerance
 func TestFixedbugs(t *testing.T) {
 	testDirFiles(t, "../../../../internal/types/testdata/fixedbugs", 100, false)

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

@@ -13,7 +13,7 @@ import (
 
 // If compareWithInfer1, infer2 results must match infer1 results.
 // Disable before releasing Go 1.21.
-const compareWithInfer1 = true
+const compareWithInfer1 = false
 
 // infer attempts to infer the complete set of type arguments for generic function instantiation/call
 // based on the given type parameters tparams, type arguments targs, function parameters params, and
@@ -76,6 +76,10 @@ func (check *Checker) infer2(pos syntax.Pos, tparams []*TypeParam, targs []Type,
 	// Rename type parameters to avoid conflicts in recursive instantiation scenarios.
 	tparams, params = check.renameTParams(pos, tparams, params)
 
+	if traceInference {
+		check.dump("after rename: %s%s ➞ %s\n", tparams, params, targs)
+	}
+
 	// Make sure we have a "full" list of type arguments, some of which may
 	// be nil (unknown). Make a copy so as to not clobber the incoming slice.
 	if len(targs) < n {
@@ -222,39 +226,21 @@ func (check *Checker) infer2(pos syntax.Pos, tparams []*TypeParam, targs []Type,
 					// In this case, if the core type has a tilde, the type argument's underlying
 					// type must match the core type, otherwise the type argument and the core type
 					// must match.
-					// If tx is an external type parameter, don't consider its underlying type
-					// (which is an interface). Core type unification will attempt to unify against
-					// core.typ.
-					// Note also that even with inexact unification we cannot leave away the under
-					// call here because it's possible that both tx and core.typ are named types,
-					// with under(tx) being a (named) basic type matching core.typ. Such cases do
-					// not match with inexact unification.
+					// If tx is an (external) type parameter, don't consider its underlying type
+					// (which is an interface). The unifier will use the type parameter's core
+					// type automatically.
 					if core.tilde && !isTypeParam(tx) {
 						tx = under(tx)
 					}
-					// Unification may fail because it operates with limited information (core type),
-					// even if a given type argument satisfies the corresponding type constraint.
-					// For instance, given [P T1|T2, ...] where the type argument for P is (named
-					// type) T1, and T1 and T2 have the same built-in (named) type T0 as underlying
-					// type, the core type will be the named type T0, which doesn't match T1.
-					// Yet the instantiation of P with T1 is clearly valid (see go.dev/issue/53650).
-					// Reporting an error if unification fails would be incorrect in this case.
-					// On the other hand, it is safe to ignore failing unification during constraint
-					// type inference because if the failure is true, an error will be reported when
-					// checking instantiation.
-					// TODO(gri) we should be able to report an error here and fix the issue in
-					// unification
-					u.unify(tx, core.typ)
-
+					if !u.unify(tx, core.typ) {
+						check.errorf(pos, CannotInferTypeArgs, "%s does not match %s", tpar, core.typ)
+						return nil
+					}
 				case single && !core.tilde:
 					// The corresponding type argument tx is unknown and there's a single
 					// specific type and no tilde.
 					// In this case the type argument must be that single type; set it.
 					u.set(tpar, core.typ)
-
-				default:
-					// Unification is not possible and no progress was made.
-					continue
 				}
 			} else {
 				if traceInference {

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

@@ -3,6 +3,31 @@
 // license that can be found in the LICENSE file.
 
 // This file implements type unification.
+//
+// Type unification attempts to make two types x and y structurally
+// identical by determining the types for a given list of (bound)
+// type parameters which may occur within x and y. If x and y are
+// are structurally different (say []T vs chan T), or conflicting
+// types are determined for type parameters, unification fails.
+// If unification succeeds, as a side-effect, the types of the
+// bound type parameters may be determined.
+//
+// Unification typically requires multiple calls u.unify(x, y) to
+// a given unifier u, with various combinations of types x and y.
+// In each call, additional type parameter types may be determined
+// as a side effect. If a call fails (returns false), unification
+// fails.
+//
+// In the unification context, structural identity ignores the
+// difference between a defined type and its underlying type.
+// It also ignores the difference between an (external, unbound)
+// type parameter and its core type.
+// If two types are not structurally identical, they cannot be Go
+// identical types. On the other hand, if they are structurally
+// identical, they may be Go identical or at least assignable, or
+// they may be in the type set of a constraint.
+// Whether they indeed are identical or assignable is determined
+// upon instantiation and function argument passing.
 
 package types2
 
@@ -239,7 +264,7 @@ func (u *unifier) nify(x, y Type, p *ifacePair) (result bool) {
 
 	// Unification is symmetric, so we can swap the operands.
 	// Ensure that if we have at least one
-	// - defined type, make sure sure one is in y
+	// - defined type, make sure one is in y
 	// - type parameter recorded with u, make sure one is in x
 	if _, ok := x.(*Named); ok || u.asTypeParam(y) != nil {
 		if traceInference {
@@ -248,13 +273,24 @@ func (u *unifier) nify(x, y Type, p *ifacePair) (result bool) {
 		x, y = y, x
 	}
 
-	// If exact unification is known to fail because we attempt to
-	// match a defined type against an unnamed type literal, consider
-	// the underlying type of the defined type.
+	// Unification will fail if we match a defined type against a type literal.
+	// Per the (spec) assignment rules, assignments of values to variables with
+	// the same type structure are permitted as long as at least one of them
+	// is not a defined type. To accomodate for that possibility, we continue
+	// unification with the underlying type of a defined type if the other type
+	// is a type literal.
+	// We also continue if the other type is a basic type because basic types
+	// are valid underlying types and may appear as core types of type constraints.
+	// If we exclude them, inferred defined types for type parameters may not
+	// match against the core types of their constraints (even though they might
+	// correctly match against some of the types in the constraint's type set).
+	// Finally, if unification (incorrectly) succeeds by matching the underlying
+	// type of a defined type against a basic type (because we include basic types
+	// as type literals here), and if that leads to an incorrectly inferred type,
+	// we will fail at function instantiation or argument assignment time.
+	//
 	// If we have at least one defined type, there is one in y.
-	// (We use !hasName to exclude any type with a name, including
-	// basic types and type parameters; the rest are unamed types.)
-	if ny, _ := y.(*Named); ny != nil && !hasName(x) {
+	if ny, _ := y.(*Named); ny != nil && isTypeLit(x) {
 		if traceInference {
 			u.tracef("%s ≡ under %s", x, ny)
 		}
@@ -266,6 +302,10 @@ func (u *unifier) nify(x, y Type, p *ifacePair) (result bool) {
 
 	// Cases where at least one of x or y is a type parameter recorded with u.
 	// If we have at least one type parameter, there is one in x.
+	// If we have exactly one type parameter, because it is in x,
+	// isTypeLit(x) is false and y was not changed above. In other
+	// words, if y was a defined type, it is still a defined type
+	// (relevant for the logic below).
 	switch px, py := u.asTypeParam(x), u.asTypeParam(y); {
 	case px != nil && py != nil:
 		// both x and y are type parameters
@@ -296,8 +336,19 @@ func (u *unifier) nify(x, y Type, p *ifacePair) (result bool) {
 		return true
 	}
 
-	// If we get here and x or y is a type parameter, they are type parameters
-	// from outside our declaration list. Try to unify their core types, if any
+	// If we get here and x or y is a type parameter, they are unbound
+	// (not recorded with the unifier).
+	// By definition, a valid type argument must be in the type set of
+	// the respective type constraint. Therefore, the type argument's
+	// underlying type must be in the set of underlying types of that
+	// constraint. If there is a single such underlying type, it's the
+	// constraint's core type. It must match the type argument's under-
+	// lying type, irrespective of whether the actual type argument,
+	// which may be a defined type, is actually in the type set (that
+	// will be determined at instantiation time).
+	// Thus, if we have the core type of an unbound type parameter,
+	// we know the structure of the possible types satisfying such
+	// parameters. Use that core type for further unification
 	// (see go.dev/issue/50755 for a test case).
 	if enableCoreTypeUnification {
 		// swap x and y as needed
@@ -308,18 +359,15 @@ func (u *unifier) nify(x, y Type, p *ifacePair) (result bool) {
 			}
 			x, y = y, x
 		}
-		if isTypeParam(x) && !hasName(y) {
+		if isTypeParam(x) {
 			// When considering the type parameter for unification
-			// we look at the adjusted core term (adjusted core type
-			// with tilde information).
-			// If the adjusted core type is a named type N; the
-			// corresponding core type is under(N).
-			// Since y doesn't have a name, unification will end up
-			// comparing under(N) to y, so we can just use the core
-			// type instead. And we can ignore the tilde because we
-			// already look at the underlying types on both sides
-			// and we have known types on both sides.
-			// Optimization.
+			// we look at the core type.
+			// Because the core type is always an underlying type,
+			// unification will take care of matching against a
+			// defined or literal type automatically.
+			// If y is also an unbound type parameter, we will end
+			// up here again with x and y swapped, so we don't
+			// need to take care of that case separately.
 			if cx := coreType(x); cx != nil {
 				if traceInference {
 					u.tracef("core %s ≡ %s", x, y)