From 87bc85a846d5dc2d8fe7dbda900d6066ab98f1a5 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Mon, 30 Nov 2020 21:28:32 -0800 Subject: [PATCH] [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 Reviewed-by: Robert Findley --- src/cmd/compile/internal/types2/api.go | 5 +++++ src/cmd/compile/internal/types2/api_test.go | 8 ++++---- src/cmd/compile/internal/types2/assignments.go | 10 +++++++--- src/cmd/compile/internal/types2/builtins.go | 2 +- src/cmd/compile/internal/types2/decl.go | 6 +++++- src/cmd/compile/internal/types2/expr.go | 8 ++++++-- src/cmd/compile/internal/types2/exprstring.go | 14 ++++++++++---- src/cmd/compile/internal/types2/exprstring_test.go | 6 +++--- .../compile/internal/types2/testdata/decls3.src | 10 +++++----- .../compile/internal/types2/testdata/decls4.src | 4 ++-- .../internal/types2/testdata/issue28251.src | 2 +- src/cmd/compile/internal/types2/typexpr.go | 14 ++++++++++++-- 12 files changed, 61 insertions(+), 28 deletions(-) diff --git a/src/cmd/compile/internal/types2/api.go b/src/cmd/compile/internal/types2/api.go index eff90d4cdf..a40665ee17 100644 --- a/src/cmd/compile/internal/types2/api.go +++ b/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 diff --git a/src/cmd/compile/internal/types2/api_test.go b/src/cmd/compile/internal/types2/api_test.go index 403df3f941..58d7df2f1d 100644 --- a/src/cmd/compile/internal/types2/api_test.go +++ b/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 v2; var _ = func(){}`, `(func() literal)`, `value`}, + {`package v1; var _ = &[]int{1}`, `[]int{…}`, `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", diff --git a/src/cmd/compile/internal/types2/assignments.go b/src/cmd/compile/internal/types2/assignments.go index 93d3255686..3178c38ade 100644 --- a/src/cmd/compile/internal/types2/assignments.go +++ b/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 != "" { - check.errorf(x, "cannot use %s as %s value in %s: %s", x, T, context, reason) + if check.conf.CompilerErrorMessages { + 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 } diff --git a/src/cmd/compile/internal/types2/builtins.go b/src/cmd/compile/internal/types2/builtins.go index 6ad84f4354..43da6a1529 100644 --- a/src/cmd/compile/internal/types2/builtins.go +++ b/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 } diff --git a/src/cmd/compile/internal/types2/decl.go b/src/cmd/compile/internal/types2/decl.go index ff37d85c6f..c7bfd3fd7b 100644 --- a/src/cmd/compile/internal/types2/decl.go +++ b/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() } diff --git a/src/cmd/compile/internal/types2/expr.go b/src/cmd/compile/internal/types2/expr.go index cb92143f93..3c9540783a 100644 --- a/src/cmd/compile/internal/types2/expr.go +++ b/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} diff --git a/src/cmd/compile/internal/types2/exprstring.go b/src/cmd/compile/internal/types2/exprstring.go index e187d050e6..0ec5d1338f 100644 --- a/src/cmd/compile/internal/types2/exprstring.go +++ b/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('(') diff --git a/src/cmd/compile/internal/types2/exprstring_test.go b/src/cmd/compile/internal/types2/exprstring_test.go index d7b9d5b2ef..efb7c308b7 100644 --- a/src/cmd/compile/internal/types2/exprstring_test.go +++ b/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(x int) complex128 {}", "(func(x int) complex128 literal)"}, - {"[]int{1, 2, 3}", "([]int literal)"}, + {"func(){}", "func() {}"}, + {"func(x int) complex128 {}", "func(x int) complex128 {}"}, + {"[]int{1, 2, 3}", "[]int{…}"}, // non-type expressions dup("(x)"), diff --git a/src/cmd/compile/internal/types2/testdata/decls3.src b/src/cmd/compile/internal/types2/testdata/decls3.src index 745175c710..d7a0c444da 100644 --- a/src/cmd/compile/internal/types2/testdata/decls3.src +++ b/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" */ \ No newline at end of file +var _ = R0{}.X /* ERROR "ambiguous selector R0\{\}.X" */ \ No newline at end of file diff --git a/src/cmd/compile/internal/types2/testdata/decls4.src b/src/cmd/compile/internal/types2/testdata/decls4.src index 140bbfd31f..eb08421bee 100644 --- a/src/cmd/compile/internal/types2/testdata/decls4.src +++ b/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{}.xm /* ERROR ambiguous selector \(eD literal\).xm */ + _ = eD{}.xf /* ERROR ambiguous selector eD\{\}.xf */ + _ = eD{}.xm /* ERROR ambiguous selector eD\{\}.xm */ ) var ( diff --git a/src/cmd/compile/internal/types2/testdata/issue28251.src b/src/cmd/compile/internal/types2/testdata/issue28251.src index cd79e0e8b5..ef5e61df47 100644 --- a/src/cmd/compile/internal/types2/testdata/issue28251.src +++ b/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 */ () } diff --git a/src/cmd/compile/internal/types2/typexpr.go b/src/cmd/compile/internal/types2/typexpr.go index 1adf967859..0c27e5e04b 100644 --- a/src/cmd/compile/internal/types2/typexpr.go +++ b/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 } } -- 2.50.0