go/types, types2: report type mismatch error when conversion is impossible

Rather than reporting an impossible conversion error when mixing an
untyped value with a pointer type in an operation, report a type
mismatch error. This fixes a regression in error quality compared
to pre-1.18.

For the fix, clean up the implementation of canMix, add documentation,
and give it a better name.

Adjust test case for corresponding error code bacause we now get a
better error message (and error code) for the old error code example.

Fixes #57160.

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

Author: griesemer

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

@@ -1103,26 +1103,50 @@ func (check *Checker) binary(x *operand, e syntax.Expr, lhs, rhs syntax.Expr, op
 		return
 	}
 
-	// TODO(gri) make canMix more efficient - called for each binary operation
-	canMix := func(x, y *operand) bool {
+	// mayConvert reports whether the operands x and y may
+	// possibly have matching types after converting one
+	// untyped operand to the type of the other.
+	// If mayConvert returns true, we try to convert the
+	// operands to each other's types, and if that fails
+	// we report a conversion failure.
+	// If mayConvert returns false, we continue without an
+	// attempt at conversion, and if the operand types are
+	// not compatible, we report a type mismatch error.
+	mayConvert := func(x, y *operand) bool {
+		// If both operands are typed, there's no need for an implicit conversion.
+		if isTyped(x.typ) && isTyped(y.typ) {
+			return false
+		}
+		// An untyped operand may convert to its default type when paired with an empty interface
+		// TODO(gri) This should only matter for comparisons (the only binary operation that is
+		//           valid with interfaces), but in that case the assignability check should take
+		//           care of the conversion. Verify and possibly eliminate this extra test.
 		if isNonTypeParamInterface(x.typ) || isNonTypeParamInterface(y.typ) {
 			return true
 		}
+		// A boolean type can only convert to another boolean type.
 		if allBoolean(x.typ) != allBoolean(y.typ) {
 			return false
 		}
+		// A string type can only convert to another string type.
 		if allString(x.typ) != allString(y.typ) {
 			return false
 		}
-		if x.isNil() && !hasNil(y.typ) {
-			return false
+		// Untyped nil can only convert to a type that has a nil.
+		if x.isNil() {
+			return hasNil(y.typ)
+		}
+		if y.isNil() {
+			return hasNil(x.typ)
 		}
-		if y.isNil() && !hasNil(x.typ) {
+		// An untyped operand cannot convert to a pointer.
+		// TODO(gri) generalize to type parameters
+		if isPointer(x.typ) || isPointer(y.typ) {
 			return false
 		}
 		return true
 	}
-	if canMix(x, &y) {
+	if mayConvert(x, &y) {
 		check.convertUntyped(x, y.typ)
 		if x.mode == invalid {
 			return

src/go/types/expr.go

@@ -1084,26 +1084,50 @@ func (check *Checker) binary(x *operand, e ast.Expr, lhs, rhs ast.Expr, op token
 		return
 	}
 
-	// TODO(gri) make canMix more efficient - called for each binary operation
-	canMix := func(x, y *operand) bool {
+	// mayConvert reports whether the operands x and y may
+	// possibly have matching types after converting one
+	// untyped operand to the type of the other.
+	// If mayConvert returns true, we try to convert the
+	// operands to each other's types, and if that fails
+	// we report a conversion failure.
+	// If mayConvert returns false, we continue without an
+	// attempt at conversion, and if the operand types are
+	// not compatible, we report a type mismatch error.
+	mayConvert := func(x, y *operand) bool {
+		// If both operands are typed, there's no need for an implicit conversion.
+		if isTyped(x.typ) && isTyped(y.typ) {
+			return false
+		}
+		// An untyped operand may convert to its default type when paired with an empty interface
+		// TODO(gri) This should only matter for comparisons (the only binary operation that is
+		//           valid with interfaces), but in that case the assignability check should take
+		//           care of the conversion. Verify and possibly eliminate this extra test.
 		if isNonTypeParamInterface(x.typ) || isNonTypeParamInterface(y.typ) {
 			return true
 		}
+		// A boolean type can only convert to another boolean type.
 		if allBoolean(x.typ) != allBoolean(y.typ) {
 			return false
 		}
+		// A string type can only convert to another string type.
 		if allString(x.typ) != allString(y.typ) {
 			return false
 		}
-		if x.isNil() && !hasNil(y.typ) {
-			return false
+		// Untyped nil can only convert to a type that has a nil.
+		if x.isNil() {
+			return hasNil(y.typ)
+		}
+		if y.isNil() {
+			return hasNil(x.typ)
 		}
-		if y.isNil() && !hasNil(x.typ) {
+		// An untyped operand cannot convert to a pointer.
+		// TODO(gri) generalize to type parameters
+		if isPointer(x.typ) || isPointer(y.typ) {
 			return false
 		}
 		return true
 	}
-	if canMix(x, &y) {
+	if mayConvert(x, &y) {
 		check.convertUntyped(x, y.typ)
 		if x.mode == invalid {
 			return

src/internal/types/errors/codes.go

@@ -882,7 +882,7 @@ const (
 	// context in which it is used.
 	//
 	// Example:
-	//  var _ = 1 + new(int)
+	//  var _ = 1 + []int{}
 	InvalidUntypedConversion
 
 	// BadOffsetofSyntax occurs when unsafe.Offsetof is called with an argument

src/internal/types/testdata/fixedbugs/issue57160.go

@@ -0,0 +1,10 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package p
+
+func _(x *int) {
+	_ = 0 < x // ERROR "invalid operation"
+	_ = x < 0 // ERROR "invalid operation"
+}