go/types, types2: simplify Checker.typeAssertion, use same code in both type checkers

- Remove the xtyp argument from the Checker.typeAssertion parameter
  list; it was confusing and not needed. Adjusted call sites.

- Simplify logic in Checker.typeAssertion.

- Use the same code in both types2 and go/types, specifically
  use the same error positions.

- Adjust error messages as needed.

This removes another subtle discrepancy between types2 and go/types.
The go/types error messages don't have the have/want appendix for
the affected error messages yet because we don't use case folding
in lookups yet.

Change-Id: Id39f5c473da36c9baad60082f85cf1f34dc26c50
Reviewed-on: https://go-review.googlesource.com/c/go/+/383014
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>

diff --git a/src/cmd/compile/internal/types2/expr.go b/src/cmd/compile/internal/types2/expr.go
index f1696bbe51085179b7240f38d14be9429994d945..4fdabe754e472452ebab63a58ea9267a0fff9218 100644 (file)
--- a/src/cmd/compile/internal/types2/expr.go
+++ b/src/cmd/compile/internal/types2/expr.go
@@ -1578,8 +1578,7 @@ func (check *Checker) exprInternal(x *operand, e syntax.Expr, hint Type) exprKin
                        check.errorf(x, invalidOp+"cannot use type assertion on type parameter value %s", x)
                        goto Error
                }
-               xtyp, _ := under(x.typ).(*Interface)
-               if xtyp == nil {
+               if _, ok := under(x.typ).(*Interface); !ok {
                        check.errorf(x, invalidOp+"%s is not an interface", x)
                        goto Error
                }
@@ -1592,7 +1591,7 @@ func (check *Checker) exprInternal(x *operand, e syntax.Expr, hint Type) exprKin
                if T == Typ[Invalid] {
                        goto Error
                }
-               check.typeAssertion(e, x, xtyp, T, false)
+               check.typeAssertion(e, x, T, false)
                x.mode = commaok
                x.typ = T
 
@@ -1733,28 +1732,21 @@ func keyVal(x constant.Value) interface{} {
        return x
 }
 
-// typeAssertion checks that x.(T) is legal; xtyp must be the type of x.
-func (check *Checker) typeAssertion(e syntax.Expr, x *operand, xtyp *Interface, T Type, typeSwitch bool) {
-       method, wrongType := check.assertableTo(xtyp, T)
+// typeAssertion checks x.(T). The type of x must be an interface.
+func (check *Checker) typeAssertion(e syntax.Expr, x *operand, T Type, typeSwitch bool) {
+       method, alt := check.assertableTo(under(x.typ).(*Interface), T)
        if method == nil {
-               return
+               return // success
        }
 
-       var err error_
-       var msg string
-       if typeSwitch {
-               err.errorf(e.Pos(), "impossible type switch case: %s", e)
-               msg = check.sprintf("%s cannot have dynamic type %s %s", x, T,
-                       check.missingMethodReason(T, x.typ, method, wrongType))
-
-       } else {
-               err.errorf(e.Pos(), "impossible type assertion: %s", e)
-               msg = check.sprintf("%s does not implement %s %s", T, x.typ,
-                       check.missingMethodReason(T, x.typ, method, wrongType))
+       cause := check.missingMethodReason(T, x.typ, method, alt)
 
+       if typeSwitch {
+               check.errorf(e, "impossible type switch case: %s\n\t%s cannot have dynamic type %s %s", e, x, T, cause)
+               return
        }
-       err.errorf(nopos, msg)
-       check.report(&err)
+
+       check.errorf(e, "impossible type assertion: %s\n\t%s does not implement %s %s", e, T, x.typ, cause)
 }
 
 // expr typechecks expression e and initializes x with the expression value.

diff --git a/src/cmd/compile/internal/types2/stmt.go b/src/cmd/compile/internal/types2/stmt.go
index 633ee31551396c5b7ced4f913f6882ffd87f196a..03da98af34fd6c9cfed4d4db6eeb579bf246fa96 100644 (file)
--- a/src/cmd/compile/internal/types2/stmt.go
+++ b/src/cmd/compile/internal/types2/stmt.go
@@ -274,7 +274,8 @@ func (check *Checker) isNil(e syntax.Expr) bool {
        return false
 }
 
-func (check *Checker) caseTypes(x *operand, xtyp *Interface, types []syntax.Expr, seen map[Type]syntax.Expr) (T Type) {
+// If the type switch expression is invalid, x is nil.
+func (check *Checker) caseTypes(x *operand, types []syntax.Expr, seen map[Type]syntax.Expr) (T Type) {
        var dummy operand
 L:
        for _, e := range types {
@@ -305,8 +306,8 @@ L:
                        }
                }
                seen[T] = e
-               if T != nil && xtyp != nil {
-                       check.typeAssertion(e, x, xtyp, T, true)
+               if x != nil && T != nil {
+                       check.typeAssertion(e, x, T, true)
                }
        }
        return
@@ -733,12 +734,13 @@ func (check *Checker) typeSwitchStmt(inner stmtContext, s *syntax.SwitchStmt, gu
        }
 
        // TODO(gri) we may want to permit type switches on type parameter values at some point
-       var xtyp *Interface
+       var sx *operand // switch expression against which cases are compared against; nil if invalid
        if isTypeParam(x.typ) {
                check.errorf(&x, "cannot use type switch on type parameter value %s", &x)
        } else {
-               xtyp, _ = under(x.typ).(*Interface)
-               if xtyp == nil {
+               if _, ok := under(x.typ).(*Interface); ok {
+                       sx = &x
+               } else {
                        check.errorf(&x, "%s is not an interface", &x)
                }
        }
@@ -758,7 +760,7 @@ func (check *Checker) typeSwitchStmt(inner stmtContext, s *syntax.SwitchStmt, gu
                }
                // Check each type in this type switch case.
                cases := unpackExpr(clause.Cases)
-               T := check.caseTypes(&x, xtyp, cases, seen)
+               T := check.caseTypes(sx, cases, seen)
                check.openScopeUntil(clause, end, "case")
                // If lhs exists, declare a corresponding variable in the case-local scope.
                if lhs != nil {

diff --git a/src/cmd/compile/internal/types2/testdata/check/issues.src b/src/cmd/compile/internal/types2/testdata/check/issues.src
index 4c49147922623ce040800c689a10b416f1c7f9df..3b27e0358547405e0adebd64b5a5c97caa3c6704 100644 (file)
--- a/src/cmd/compile/internal/types2/testdata/check/issues.src
+++ b/src/cmd/compile/internal/types2/testdata/check/issues.src
@@ -132,12 +132,12 @@ func issue10260() {
 
        var x I1
        x = T1 /* ERROR cannot use .*: missing method foo \(foo has pointer receiver\) */ {}
-       _ = x. /* ERROR impossible type assertion: x.\(T1\)\n\tT1 does not implement I1 \(method foo has pointer receiver\) */ (T1)
+       _ = x /* ERROR impossible type assertion: x\.\(T1\)\n\tT1 does not implement I1 \(method foo has pointer receiver\) */ .(T1)
 
        T1{}.foo /* ERROR cannot call pointer method foo on T1 */ ()
        x.Foo /* ERROR "x.Foo undefined \(type I1 has no field or method Foo, but does have foo\)" */ ()
 
-       _ = i2. /* ERROR impossible type assertion: i2\.\(\*T1\)\n\t\*T1 does not implement I2 \(wrong type for method foo\)\n\t\thave foo\(\)\n\t\twant foo\(x int\) */ (*T1)
+       _ = i2 /* ERROR impossible type assertion: i2\.\(\*T1\)\n\t\*T1 does not implement I2 \(wrong type for method foo\)\n\t\thave foo\(\)\n\t\twant foo\(x int\) */ .(*T1)
 
        i1 = i0 /* ERROR cannot use .* missing method foo */
        i1 = t0 /* ERROR cannot use .* missing method foo */

diff --git a/src/cmd/compile/internal/types2/testdata/fixedbugs/issue49005.go b/src/cmd/compile/internal/types2/testdata/fixedbugs/issue49005.go
index f152e7f55ce27547ded9b31d157f0d0f7fa0e537..7083dc9eef569b0d574d833a6a28c05d334df500 100644 (file)
--- a/src/cmd/compile/internal/types2/testdata/fixedbugs/issue49005.go
+++ b/src/cmd/compile/internal/types2/testdata/fixedbugs/issue49005.go
@@ -2,9 +2,6 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// This file is tested when running "go test -run Manual"
-// without source arguments. Use for one-off debugging.
-
 package p
 
 type T1 interface{ M() }
@@ -23,7 +20,7 @@ type T2 interface{ M() }
 
 func F2() T2
 
-var _ = F2(). /* ERROR impossible type assertion: F2\(\).\(\*X2\)\n\t\*X2 does not implement T2 \(missing method M\) */ (*X2)
+var _ = F2 /* ERROR impossible type assertion: F2\(\)\.\(\*X2\)\n\t\*X2 does not implement T2 \(missing method M\) */ ().(*X2)
 
 type X2 struct{}
 

diff --git a/src/cmd/compile/internal/types2/testdata/fixedbugs/issue50816.go2 b/src/cmd/compile/internal/types2/testdata/fixedbugs/issue50816.go2
index b2bcb4524819b4ab7e25dc5c40cce1d21dca79c0..e7e31d91922ee4ef5ef23cf8739894e8e2559fe2 100644 (file)
--- a/src/cmd/compile/internal/types2/testdata/fixedbugs/issue50816.go2
+++ b/src/cmd/compile/internal/types2/testdata/fixedbugs/issue50816.go2
@@ -18,6 +18,6 @@ func (T2) foo() string { return "" }
 
 func _() {
        var i I
-       _ = i./* ERROR impossible type assertion: i.\(T1\)\n\tT1 does not implement I \(missing method Foo\)\n\t\thave foo\(\)\n\t\twant Foo\(\) */ (T1)
-       _ = i./* ERROR impossible type assertion: i.\(T2\)\n\tT2 does not implement I \(missing method Foo\)\n\t\thave foo\(\) string\n\t\twant Foo\(\) */ (T2)
+       _ = i /* ERROR impossible type assertion: i\.\(T1\)\n\tT1 does not implement I \(missing method Foo\)\n\t\thave foo\(\)\n\t\twant Foo\(\) */ .(T1)
+       _ = i /* ERROR impossible type assertion: i\.\(T2\)\n\tT2 does not implement I \(missing method Foo\)\n\t\thave foo\(\) string\n\t\twant Foo\(\) */ .(T2)
 }

diff --git a/src/go/types/expr.go b/src/go/types/expr.go
index 88a8901b07e38ccee1c9e502e1751c4b82074007..0d21a592f959e7564e374e7a2de0f3a509d6891f 100644 (file)
--- a/src/go/types/expr.go
+++ b/src/go/types/expr.go
@@ -1556,8 +1556,7 @@ func (check *Checker) exprInternal(x *operand, e ast.Expr, hint Type) exprKind {
                        check.invalidOp(x, _InvalidAssert, "cannot use type assertion on type parameter value %s", x)
                        goto Error
                }
-               xtyp, _ := under(x.typ).(*Interface)
-               if xtyp == nil {
+               if _, ok := under(x.typ).(*Interface); !ok {
                        check.invalidOp(x, _InvalidAssert, "%s is not an interface", x)
                        goto Error
                }
@@ -1572,7 +1571,7 @@ func (check *Checker) exprInternal(x *operand, e ast.Expr, hint Type) exprKind {
                if T == Typ[Invalid] {
                        goto Error
                }
-               check.typeAssertion(x, x, xtyp, T)
+               check.typeAssertion(e, x, T, false)
                x.mode = commaok
                x.typ = T
 
@@ -1676,23 +1675,21 @@ func keyVal(x constant.Value) any {
        return x
 }
 
-// typeAssertion checks that x.(T) is legal; xtyp must be the type of x.
-func (check *Checker) typeAssertion(at positioner, x *operand, xtyp *Interface, T Type) {
-       method, wrongType := check.assertableTo(xtyp, T)
+// typeAssertion checks x.(T). The type of x must be an interface.
+func (check *Checker) typeAssertion(e ast.Expr, x *operand, T Type, typeSwitch bool) {
+       method, alt := check.assertableTo(under(x.typ).(*Interface), T)
        if method == nil {
-               return
+               return // success
        }
-       var msg string
-       if wrongType != nil {
-               if Identical(method.typ, wrongType.typ) {
-                       msg = fmt.Sprintf("missing method %s (%s has pointer receiver)", method.name, method.name)
-               } else {
-                       msg = fmt.Sprintf("wrong type for method %s (have %s, want %s)", method.name, wrongType.typ, method.typ)
-               }
-       } else {
-               msg = "missing method " + method.name
+
+       cause := check.missingMethodReason(T, x.typ, method, alt)
+
+       if typeSwitch {
+               check.errorf(e, _ImpossibleAssert, "impossible type switch case: %s\n\t%s cannot have dynamic type %s %s", e, x, T, cause)
+               return
        }
-       check.errorf(at, _ImpossibleAssert, "%s cannot have dynamic type %s (%s)", x, T, msg)
+
+       check.errorf(e, _ImpossibleAssert, "impossible type assertion: %s\n\t%s does not implement %s %s", e, T, x.typ, cause)
 }
 
 // expr typechecks expression e and initializes x with the expression value.

diff --git a/src/go/types/stmt.go b/src/go/types/stmt.go
index 5ceae08daa20ec9ad5a5f2b75bac96f386e68fe0..b32eb18bef493d28a8e3a51ef980de6390a43c7e 100644 (file)
--- a/src/go/types/stmt.go
+++ b/src/go/types/stmt.go
@@ -281,7 +281,8 @@ func (check *Checker) isNil(e ast.Expr) bool {
        return false
 }
 
-func (check *Checker) caseTypes(x *operand, xtyp *Interface, types []ast.Expr, seen map[Type]ast.Expr) (T Type) {
+// If the type switch expression is invalid, x is nil.
+func (check *Checker) caseTypes(x *operand, types []ast.Expr, seen map[Type]ast.Expr) (T Type) {
        var dummy operand
 L:
        for _, e := range types {
@@ -310,8 +311,8 @@ L:
                        }
                }
                seen[T] = e
-               if T != nil && xtyp != nil {
-                       check.typeAssertion(e, x, xtyp, T)
+               if x != nil && T != nil {
+                       check.typeAssertion(e, x, T, true)
                }
        }
        return
@@ -684,12 +685,13 @@ func (check *Checker) stmt(ctxt stmtContext, s ast.Stmt) {
                        return
                }
                // TODO(gri) we may want to permit type switches on type parameter values at some point
-               var xtyp *Interface
+               var sx *operand // switch expression against which cases are compared against; nil if invalid
                if isTypeParam(x.typ) {
                        check.errorf(&x, _InvalidTypeSwitch, "cannot use type switch on type parameter value %s", &x)
                } else {
-                       xtyp, _ = under(x.typ).(*Interface)
-                       if xtyp == nil {
+                       if _, ok := under(x.typ).(*Interface); ok {
+                               sx = &x
+                       } else {
                                check.errorf(&x, _InvalidTypeSwitch, "%s is not an interface", &x)
                        }
                }
@@ -705,7 +707,7 @@ func (check *Checker) stmt(ctxt stmtContext, s ast.Stmt) {
                                continue
                        }
                        // Check each type in this type switch case.
-                       T := check.caseTypes(&x, xtyp, clause.List, seen)
+                       T := check.caseTypes(sx, clause.List, seen)
                        check.openScope(clause, "case")
                        // If lhs exists, declare a corresponding variable in the case-local scope.
                        if lhs != nil {

diff --git a/src/go/types/testdata/check/issues.src b/src/go/types/testdata/check/issues.src
index 0b77b0e854012b7655081a98fa849e98b1409538..ce27ac3cfb10aae1a48af3e81a257890917b66fd 100644 (file)
--- a/src/go/types/testdata/check/issues.src
+++ b/src/go/types/testdata/check/issues.src
@@ -132,12 +132,12 @@ func issue10260() {
 
        var x I1
        x = T1 /* ERROR cannot use .*: missing method foo \(foo has pointer receiver\) */ {}
-       _ = x /* ERROR .* cannot have dynamic type T1 \(missing method foo \(foo has pointer receiver\)\) */ .(T1)
+       _ = x /* ERROR impossible type assertion: x\.\(T1\)\n\tT1 does not implement I1 \(method foo has pointer receiver\) */ .(T1)
 
        T1{}.foo /* ERROR cannot call pointer method foo on T1 */ ()
        x.Foo /* ERROR "x.Foo undefined \(type I1 has no field or method Foo, but does have foo\)" */ ()
 
-       _ = i2 /* ERROR i2 .* cannot have dynamic type \*T1 \(wrong type for method foo \(have func\(\), want func\(x int\)\)\) */ .(*T1)
+       _ = i2 /* ERROR impossible type assertion: i2\.\(\*T1\)\n\t\*T1 does not implement I2 \(wrong type for method foo\)\n\t\thave foo\(\)\n\t\twant foo\(x int\) */ .(*T1)
 
        i1 = i0 /* ERROR cannot use .* missing method foo */
        i1 = t0 /* ERROR cannot use .* missing method foo */

diff --git a/src/go/types/testdata/fixedbugs/issue49005.go b/src/go/types/testdata/fixedbugs/issue49005.go
new file mode 100644 (file)
index 0000000..7083dc9
--- /dev/null
+++ b/src/go/types/testdata/fixedbugs/issue49005.go
@@ -0,0 +1,31 @@
+// Copyright 2021 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
+
+type T1 interface{ M() }
+
+func F1() T1
+
+var _ = F1().(*X1 /* ERROR undeclared name: X1 */)
+
+func _() {
+       switch F1().(type) {
+       case *X1 /* ERROR undeclared name: X1 */ :
+       }
+}
+
+type T2 interface{ M() }
+
+func F2() T2
+
+var _ = F2 /* ERROR impossible type assertion: F2\(\)\.\(\*X2\)\n\t\*X2 does not implement T2 \(missing method M\) */ ().(*X2)
+
+type X2 struct{}
+
+func _() {
+       switch F2().(type) {
+       case * /* ERROR impossible type switch case: \*X2\n\tF2\(\) \(value of type T2\) cannot have dynamic type \*X2 \(missing method M\) */ X2:
+       }
+}

diff --git a/src/go/types/testdata/fixedbugs/issue50816.go2 b/src/go/types/testdata/fixedbugs/issue50816.go2
index a5eecc551bafd014f87d8cc4563d9918b4dd0f42..025a338184dd20d8e8061f94a4011000a5e06754 100644 (file)
--- a/src/go/types/testdata/fixedbugs/issue50816.go2
+++ b/src/go/types/testdata/fixedbugs/issue50816.go2
@@ -18,6 +18,6 @@ func (T2) foo() string { return "" }
 
 func _() {
        var i I
-       _ = i/* ERROR i \(variable of type I\) cannot have dynamic type T1 \(missing method Foo\) */.(T1)
-       _ = i/* ERROR i \(variable of type I\) cannot have dynamic type T2 \(missing method Foo\) */.(T2)
+       _ = i /* ERROR impossible type assertion: i\.\(T1\)\n\tT1 does not implement I \(missing method Foo\) */ .(T1)
+       _ = i /* ERROR impossible type assertion: i\.\(T2\)\n\tT2 does not implement I \(missing method Foo\) */ .(T2)
 }