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 <rfindley@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>

diff --git a/src/cmd/compile/internal/types2/expr.go b/src/cmd/compile/internal/types2/expr.go
index 40d6e5da695228dd7fbc613624bae6e202cc2eb9..9a0348e0250a4fbd9441d47c292bfc805083b132 100644 (file)
--- a/src/cmd/compile/internal/types2/expr.go
+++ b/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

diff --git a/src/go/types/expr.go b/src/go/types/expr.go
index da9cd67826f6db59326833e157e68229ec87dd80..e09b461d8c6b9c41abcf05bb9bf1fe84129223e7 100644 (file)
--- a/src/go/types/expr.go
+++ b/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

diff --git a/src/internal/types/errors/codes.go b/src/internal/types/errors/codes.go
index 7a0c0e16b83929108382fc3567d4c9e35cc189e2..acddcbb9c50deca0acf6130571b50ccf72cc1f31 100644 (file)
--- a/src/internal/types/errors/codes.go
+++ b/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

diff --git a/src/internal/types/testdata/fixedbugs/issue57160.go b/src/internal/types/testdata/fixedbugs/issue57160.go
new file mode 100644 (file)
index 0000000..446d019
--- /dev/null
+++ b/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"
+}