[dev.typeparams] cmd/compile/internal/types2: adjustments toward matching compiler error messages

In order to get types2 usable by the compiler, we need to
pass all the compiler tests with respect to error messages.
Sometimes the compiler error messages are better, sometimes
the types2 error messages are better. Where we can, we decide
on a case-by-case basis; but sometimes, for expediency's sake,
we just choose the compiler error message as it may reduce the
amount of tests that we need to update.

This CL introduces a new Config flag: CompilerErrorMessages.
If set, the typechecker emits an error message that matches
the expected errors in the tests most easily. Eventually, we
need to get rid of this flag by either adjusting the typechecker
errors or the test cases; t.b.d. on a case-by-case basis.

This CL also adjust a few error typechecker error messages already.

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

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

@@ -119,6 +119,11 @@ type Config struct {
 	//          Do not use casually!
 	FakeImportC bool
 
+	// If CompilerErrorMessages is set, errors are reported using
+	// cmd/compile error strings to match $GOROOT/test errors.
+	// TODO(gri) Consolidate error messages and remove this flag.
+	CompilerErrorMessages bool
+
 	// If go115UsesCgo is set, the type checker expects the
 	// _cgo_gotypes.go file generated by running cmd/cgo to be
 	// provided as a package source file. Qualified identifiers

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

@@ -687,8 +687,8 @@ func TestPredicatesInfo(t *testing.T) {
 
 		// values
 		{`package v0; var (a, b int; _ = a + b)`, `a + b`, `value`},
-		{`package v1; var _ = &[]int{1}`, `([]int literal)`, `value`},
+		{`package v1; var _ = &[]int{1}`, `[]int{…}`, `value`},
-		{`package v2; var _ = func(){}`, `(func() literal)`, `value`},
+		{`package v2; var _ = func(){}`, `func() {}`, `value`},
 		{`package v4; func f() { _ = f }`, `f`, `value`},
 		{`package v3; var _ *int = nil`, `nil`, `value, nil`},
 		{`package v3; var _ *int = (nil)`, `(nil)`, `value, nil`},
@@ -896,10 +896,10 @@ func TestInitOrderInfo(t *testing.T) {
 			"z = 0", "a, b = f()",
 		}},
 		{`package p7; var (a = func() int { return b }(); b = 1)`, []string{
-			"b = 1", "a = (func() int literal)()",
+			"b = 1", "a = func() int {…}()",
 		}},
 		{`package p8; var (a, b = func() (_, _ int) { return c, c }(); c = 1)`, []string{
-			"c = 1", "a, b = (func() (_, _ int) literal)()",
+			"c = 1", "a, b = func() (_, _ int) {…}()",
 		}},
 		{`package p9; type T struct{}; func (T) m() int { _ = y; return 0 }; var x, y = T.m, 1`, []string{
 			"y = 1", "x = T.m",

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

@@ -63,10 +63,14 @@ func (check *Checker) assignment(x *operand, T Type, context string) {
 	}
 
 	if reason := ""; !x.assignableTo(check, T, &reason) {
-		if reason != "" {
+		if check.conf.CompilerErrorMessages {
-			check.errorf(x, "cannot use %s as %s value in %s: %s", x, T, context, reason)
+			check.errorf(x, "incompatible type: cannot use %s as %s value", x, T)
 		} else {
-			check.errorf(x, "cannot use %s as %s value in %s", x, T, context)
+			if reason != "" {
+				check.errorf(x, "cannot use %s as %s value in %s: %s", x, T, context, reason)
+			} else {
+				check.errorf(x, "cannot use %s as %s value in %s", x, T, context)
+			}
 		}
 		x.mode = invalid
 	}

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

@@ -281,7 +281,7 @@ func (check *Checker) builtin(x *operand, call *syntax.CallExpr, id builtinId) (
 
 		// both argument types must be identical
 		if !check.identical(x.typ, y.typ) {
-			check.invalidArgf(x, "mismatched types %s and %s", x.typ, y.typ)
+			check.invalidOpf(x, "%s (mismatched types %s and %s)", call, x.typ, y.typ)
 			return
 		}
 

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

@@ -792,7 +792,11 @@ func (check *Checker) collectMethods(obj *TypeName) {
 			case *Var:
 				check.errorf(m.pos, "field and method with the same name %s", m.name)
 			case *Func:
-				check.errorf(m.pos, "method %s already declared for %s", m.name, obj)
+				if check.conf.CompilerErrorMessages {
+					check.errorf(m.pos, "%s.%s redeclared in this block", obj.Name(), m.name)
+				} else {
+					check.errorf(m.pos, "method %s already declared for %s", m.name, obj)
+				}
 			default:
 				unreachable()
 			}

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

@@ -736,7 +736,8 @@ func (check *Checker) comparison(x, y *operand, op syntax.Operator) {
 	}
 
 	if err != "" {
-		check.errorf(x, "cannot compare %s %s %s (%s)", x.expr, op, y.expr, err)
+		// TODO(gri) better error message for cases where one can only compare against nil
+		check.invalidOpf(x, "cannot compare %s %s %s (%s)", x.expr, op, y.expr, err)
 		x.mode = invalid
 		return
 	}
@@ -1174,6 +1175,9 @@ func (check *Checker) exprInternal(x *operand, e syntax.Expr, hint Type) exprKin
 		goto Error
 
 	case *syntax.BasicLit:
+		if e.Bad {
+			goto Error // error was reported before
+		}
 		x.setConst(e.Kind, e.Value)
 		if x.mode == invalid {
 			// The parser already establishes syntactic correctness.
@@ -1624,7 +1628,7 @@ func (check *Checker) exprInternal(x *operand, e syntax.Expr, hint Type) exprKin
 			valid = true
 			length = typ.len
 			if x.mode != variable {
-				check.invalidOpf(x, "cannot slice %s (value not addressable)", x)
+				check.invalidOpf(x, "%s (slice of unaddressable value)", x)
 				goto Error
 			}
 			x.typ = &Slice{elem: typ.elem}

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

@@ -50,14 +50,20 @@ func WriteExpr(buf *bytes.Buffer, x syntax.Expr) {
 		buf.WriteString(x.Value)
 
 	case *syntax.FuncLit:
-		buf.WriteByte('(')
 		WriteExpr(buf, x.Type)
-		buf.WriteString(" literal)") // shortened
+		if x.Body != nil && len(x.Body.List) > 0 {
+			buf.WriteString(" {…}") // shortened
+		} else {
+			buf.WriteString(" {}")
+		}
 
 	case *syntax.CompositeLit:
-		buf.WriteByte('(')
 		WriteExpr(buf, x.Type)
-		buf.WriteString(" literal)") // shortened
+		if len(x.ElemList) > 0 {
+			buf.WriteString("{…}") // shortened
+		} else {
+			buf.WriteString("{}")
+		}
 
 	case *syntax.ParenExpr:
 		buf.WriteByte('(')

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

@@ -24,9 +24,9 @@ var testExprs = []testEntry{
 	dup("`bar`"),
 
 	// func and composite literals
-	{"func(){}", "(func() literal)"},
+	{"func(){}", "func() {}"},
-	{"func(x int) complex128 {}", "(func(x int) complex128 literal)"},
+	{"func(x int) complex128 {}", "func(x int) complex128 {}"},
-	{"[]int{1, 2, 3}", "([]int literal)"},
+	{"[]int{1, 2, 3}", "[]int{…}"},
 
 	// non-type expressions
 	dup("(x)"),

src/cmd/compile/internal/types2/testdata/decls3.src

@@ -221,16 +221,16 @@ func _() {
 	_ = S2{}.B
 	_ = S2{}.C
 	_ = S2{}.D /* ERROR "no field or method" */
-	_ = S3{}.S1 /* ERROR "ambiguous selector \(S3 literal\).S1" */
+	_ = S3{}.S1 /* ERROR "ambiguous selector S3\{\}.S1" */
 	_ = S3{}.A
-	_ = S3{}.B /* ERROR "ambiguous selector" \(S3 literal\).B */
+	_ = S3{}.B /* ERROR "ambiguous selector" S3\{\}.B */
 	_ = S3{}.D
 	_ = S3{}.E
 	_ = S4{}.A
 	_ = S4{}.B /* ERROR "no field or method" */
-	_ = S5{}.X /* ERROR "ambiguous selector \(S5 literal\).X" */
+	_ = S5{}.X /* ERROR "ambiguous selector S5\{\}.X" */
 	_ = S5{}.Y
-	_ = S10{}.X /* ERROR "ambiguous selector \(S10 literal\).X" */
+	_ = S10{}.X /* ERROR "ambiguous selector S10\{\}.X" */
 	_ = S10{}.Y
 }
 
@@ -306,4 +306,4 @@ type R22 R21
 type R23 R21
 type R24 R21
 
-var _ = R0{}.X /* ERROR "ambiguous selector \(R0 literal\).X" */
+var _ = R0{}.X /* ERROR "ambiguous selector R0\{\}.X" */

src/cmd/compile/internal/types2/testdata/decls4.src

@@ -190,8 +190,8 @@ type eD struct {
 }
 
 var (
-	_ = eD{}.xf /* ERROR ambiguous selector \(eD literal\).xf */
+	_ = eD{}.xf /* ERROR ambiguous selector eD\{\}.xf */
-	_ = eD{}.xm /* ERROR ambiguous selector \(eD literal\).xm */
+	_ = eD{}.xm /* ERROR ambiguous selector eD\{\}.xm */
 )
 
 var (

src/cmd/compile/internal/types2/testdata/issue28251.src

@@ -60,6 +60,6 @@ type (
         T11 = T
 )
 
-func (T9 /* ERROR invalid receiver \*\*T */ ) m9() {}
+func (T9 /* ERROR invalid receiver type \*\*T */ ) m9() {}
 func _() { (T{}).m9 /* ERROR has no field or method m9 */ () }
 func _() { (&T{}).m9 /* ERROR has no field or method m9 */ () }

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

@@ -384,6 +384,10 @@ func (check *Checker) funcType(sig *Signature, recvPar *syntax.Field, tparams []
 				// as the method."
 				if T.obj.pkg != check.pkg {
 					err = "type not defined in this package"
+					if check.conf.CompilerErrorMessages {
+						check.errorf(recv.pos, "cannot define new methods on non-local type %s", recv.typ)
+						err = ""
+					}
 				} else {
 					switch u := optype(T.Under()).(type) {
 					case *Basic:
@@ -395,11 +399,17 @@ func (check *Checker) funcType(sig *Signature, recvPar *syntax.Field, tparams []
 						err = "pointer or interface type"
 					}
 				}
-			} else {
+			} else if T := t.Basic(); T != nil {
 				err = "basic or unnamed type"
+				if check.conf.CompilerErrorMessages {
+					check.errorf(recv.pos, "cannot define new methods on non-local type %s", recv.typ)
+					err = ""
+				}
+			} else {
+				check.errorf(recv.pos, "invalid receiver type %s", recv.typ)
 			}
 			if err != "" {
-				check.errorf(recv.pos, "invalid receiver %s (%s)", recv.typ, err)
+				check.errorf(recv.pos, "invalid receiver type %s (%s)", recv.typ, err)
 				// ok to continue
 			}
 		}