From 720306359a5d1d2452b8335a6e7b753751f1d466 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Thu, 22 Feb 2024 13:03:52 -0800 Subject: [PATCH] go/types, types2: make error handling more similar This change will make error handling between go/types and types2 more similar which will in turn allow more go/types files to be generated from types2 sources. Specifically: - add Checker.newError to create error_ objects - s/error_.errorf/error_.addf/ - remove error_.String (use error_.msg instead) - replace Checker.report with error_.report - make error_.report as similar as currently possible - adjust dependencies The new code consistently uses newError/addf/report to report all errors. Change-Id: Ibd6fd743a4f7746b4aa6b93fe768814dad9ee9c0 Reviewed-on: https://go-review.googlesource.com/c/go/+/566096 Reviewed-by: Robert Griesemer Reviewed-by: Robert Findley Auto-Submit: Robert Griesemer LUCI-TryBot-Result: Go LUCI --- .../compile/internal/types2/assignments.go | 11 +- src/cmd/compile/internal/types2/call.go | 24 ++- src/cmd/compile/internal/types2/decl.go | 29 ++-- src/cmd/compile/internal/types2/errors.go | 126 ++++++++------ .../compile/internal/types2/errors_test.go | 14 +- src/cmd/compile/internal/types2/infer.go | 16 +- src/cmd/compile/internal/types2/initorder.go | 11 +- src/cmd/compile/internal/types2/labels.go | 7 +- src/cmd/compile/internal/types2/mono.go | 11 +- src/cmd/compile/internal/types2/resolver.go | 16 +- src/cmd/compile/internal/types2/signature.go | 7 +- src/cmd/compile/internal/types2/stmt.go | 36 ++-- src/cmd/compile/internal/types2/struct.go | 7 +- src/cmd/compile/internal/types2/typeset.go | 18 +- src/go/types/assignments.go | 11 +- src/go/types/call.go | 22 +-- src/go/types/errors.go | 164 ++++++++++-------- src/go/types/errors_test.go | 14 +- src/go/types/generate_test.go | 8 +- src/go/types/infer.go | 16 +- src/go/types/stmt.go | 9 +- 21 files changed, 299 insertions(+), 278 deletions(-) diff --git a/src/cmd/compile/internal/types2/assignments.go b/src/cmd/compile/internal/types2/assignments.go index 92c71a30d6..382ce2d1dd 100644 --- a/src/cmd/compile/internal/types2/assignments.go +++ b/src/cmd/compile/internal/types2/assignments.go @@ -357,12 +357,11 @@ func (check *Checker) returnError(at poser, lhs []*Var, rhs []*operand) { } else if r > 0 { at = rhs[r-1] // report at last value } - var err error_ - err.code = WrongResultCount - err.errorf(at, "%s return values", qualifier) - err.errorf(nopos, "have %s", check.typesSummary(operandTypes(rhs), false)) - err.errorf(nopos, "want %s", check.typesSummary(varTypes(lhs), false)) - check.report(&err) + err := check.newError(WrongResultCount) + err.addf(at, "%s return values", qualifier) + err.addf(nopos, "have %s", check.typesSummary(operandTypes(rhs), false)) + err.addf(nopos, "want %s", check.typesSummary(varTypes(lhs), false)) + err.report() } // initVars type-checks assignments of initialization expressions orig_rhs diff --git a/src/cmd/compile/internal/types2/call.go b/src/cmd/compile/internal/types2/call.go index 55400d436c..45879e85fb 100644 --- a/src/cmd/compile/internal/types2/call.go +++ b/src/cmd/compile/internal/types2/call.go @@ -108,12 +108,11 @@ func (check *Checker) funcInst(T *target, pos syntax.Pos, x *operand, inst *synt // Note that NewTuple(params...) below is (*Tuple)(nil) if len(params) == 0, as desired. tparams, params2 := check.renameTParams(pos, sig.TypeParams().list(), NewTuple(params...)) - var err error_ - targs = check.infer(pos, tparams, targs, params2.(*Tuple), args, reverse, &err) + err := check.newError(CannotInferTypeArgs) + targs = check.infer(pos, tparams, targs, params2.(*Tuple), args, reverse, err) if targs == nil { if !err.empty() { - err.code = CannotInferTypeArgs - check.report(&err) + err.report() } x.mode = invalid return nil, nil @@ -528,12 +527,11 @@ func (check *Checker) arguments(call *syntax.CallExpr, sig *Signature, targs []T if sig.params != nil { params = sig.params.vars } - var err error_ - err.code = WrongArgCount - err.errorf(at, "%s arguments in call to %s", qualifier, call.Fun) - err.errorf(nopos, "have %s", check.typesSummary(operandTypes(args), false)) - err.errorf(nopos, "want %s", check.typesSummary(varTypes(params), sig.variadic)) - check.report(&err) + err := check.newError(WrongArgCount) + err.addf(at, "%s arguments in call to %s", qualifier, call.Fun) + err.addf(nopos, "have %s", check.typesSummary(operandTypes(args), false)) + err.addf(nopos, "want %s", check.typesSummary(varTypes(params), sig.variadic)) + err.report() return } @@ -606,15 +604,15 @@ func (check *Checker) arguments(call *syntax.CallExpr, sig *Signature, targs []T // infer missing type arguments of callee and function arguments if len(tparams) > 0 { - var err error_ - targs = check.infer(call.Pos(), tparams, targs, sigParams, args, false, &err) + err := check.newError(CannotInferTypeArgs) + targs = check.infer(call.Pos(), tparams, targs, sigParams, args, false, err) if targs == nil { // TODO(gri) If infer inferred the first targs[:n], consider instantiating // the call signature for better error messages/gopls behavior. // Perhaps instantiate as much as we can, also for arguments. // This will require changes to how infer returns its results. if !err.empty() { - check.errorf(err.pos(), CannotInferTypeArgs, "in call to %s, %s", call.Fun, err.msg(check.qualifier)) + check.errorf(err.pos(), CannotInferTypeArgs, "in call to %s, %s", call.Fun, err.msg()) } return } diff --git a/src/cmd/compile/internal/types2/decl.go b/src/cmd/compile/internal/types2/decl.go index 4408a0b168..fd9a90e1ae 100644 --- a/src/cmd/compile/internal/types2/decl.go +++ b/src/cmd/compile/internal/types2/decl.go @@ -16,7 +16,7 @@ func (err *error_) recordAltDecl(obj Object) { // We use "other" rather than "previous" here because // the first declaration seen may not be textually // earlier in the source. - err.errorf(pos, "other declaration of %s", obj.Name()) + err.addf(pos, "other declaration of %s", obj.Name()) } } @@ -27,11 +27,10 @@ func (check *Checker) declare(scope *Scope, id *syntax.Name, obj Object, pos syn // binding." if obj.Name() != "_" { if alt := scope.Insert(obj); alt != nil { - var err error_ - err.code = DuplicateDecl - err.errorf(obj, "%s redeclared in this block", obj.Name()) + err := check.newError(DuplicateDecl) + err.addf(obj, "%s redeclared in this block", obj.Name()) err.recordAltDecl(alt) - check.report(&err) + err.report() return } obj.setScopePos(pos) @@ -343,15 +342,14 @@ func (check *Checker) cycleError(cycle []Object) { return } - var err error_ - err.code = InvalidDeclCycle + err := check.newError(InvalidDeclCycle) if tname != nil { - err.errorf(obj, "invalid recursive type %s", objName) + err.addf(obj, "invalid recursive type %s", objName) } else { - err.errorf(obj, "invalid cycle in declaration of %s", objName) + err.addf(obj, "invalid cycle in declaration of %s", objName) } for range cycle { - err.errorf(obj, "%s refers to", objName) + err.addf(obj, "%s refers to", objName) i++ if i >= len(cycle) { i = 0 @@ -359,8 +357,8 @@ func (check *Checker) cycleError(cycle []Object) { obj = cycle[i] objName = name(obj) } - err.errorf(obj, "%s", objName) - check.report(&err) + err.addf(obj, "%s", objName) + err.report() } // firstInSrc reports the index of the object with the "smallest" @@ -721,11 +719,10 @@ func (check *Checker) checkFieldUniqueness(base *Named) { // For historical consistency, we report the primary error on the // method, and the alt decl on the field. - var err error_ - err.code = DuplicateFieldAndMethod - err.errorf(alt, "field and method with the same name %s", fld.name) + err := check.newError(DuplicateFieldAndMethod) + err.addf(alt, "field and method with the same name %s", fld.name) err.recordAltDecl(fld) - check.report(&err) + err.report() } } } diff --git a/src/cmd/compile/internal/types2/errors.go b/src/cmd/compile/internal/types2/errors.go index e7d8863c6f..e0ce087e31 100644 --- a/src/cmd/compile/internal/types2/errors.go +++ b/src/cmd/compile/internal/types2/errors.go @@ -29,11 +29,21 @@ func assert(p bool) { } // An error_ represents a type-checking error. -// To report an error_, call Checker.report. +// A new error_ is created with Checker.newError. +// To report an error_, call error_.report. type error_ struct { - desc []errorDesc - code Code - soft bool // TODO(gri) eventually determine this from an error code + check *Checker + desc []errorDesc + code Code + soft bool // TODO(gri) eventually determine this from an error code +} + +// newError returns a new error_ with the given error code. +func (check *Checker) newError(code Code) *error_ { + if code == 0 { + panic("error code must not be 0") + } + return &error_{check: check, code: code} } // An errorDesc describes part of a type-checking error. @@ -54,10 +64,12 @@ func (err *error_) pos() syntax.Pos { return err.desc[0].pos } -func (err *error_) msg(qf Qualifier) string { +// msg returns the formatted error message without the primary error position pos(). +func (err *error_) msg() string { if err.empty() { return "no error" } + var buf strings.Builder for i := range err.desc { p := &err.desc[i] @@ -67,22 +79,18 @@ func (err *error_) msg(qf Qualifier) string { fmt.Fprintf(&buf, "%s: ", p.pos) } } - buf.WriteString(sprintf(qf, false, p.format, p.args...)) + buf.WriteString(err.check.sprintf(p.format, p.args...)) } return buf.String() } -// String is for testing. -func (err *error_) String() string { - if err.empty() { - return "no error" - } - return fmt.Sprintf("%s: %s", err.pos(), err.msg(nil)) -} - -// errorf adds formatted error information to err. +// addf adds formatted error information to err. // It may be called multiple times to provide additional information. -func (err *error_) errorf(at poser, format string, args ...interface{}) { +// The position of the first call to addf determines the position of the reported Error. +// Subsequent calls to addf provide additional information in the form of additional lines +// in the error message (types2) or continuation errors identified by a tab-indented error +// message (go/types). +func (err *error_) addf(at poser, format string, args ...interface{}) { err.desc = append(err.desc, errorDesc{atPos(at), format, args}) } @@ -196,13 +204,6 @@ func (check *Checker) sprintf(format string, args ...interface{}) string { return sprintf(qf, false, format, args...) } -func (check *Checker) report(err *error_) { - if err.empty() { - panic("no error to report") - } - check.err(err.pos(), err.code, err.msg(check.qualifier), err.soft) -} - func (check *Checker) trace(pos syntax.Pos, format string, args ...interface{}) { fmt.Printf("%s:\t%s%s\n", pos, @@ -216,38 +217,48 @@ func (check *Checker) dump(format string, args ...interface{}) { fmt.Println(sprintf(check.qualifier, true, format, args...)) } -func (check *Checker) err(at poser, code Code, msg string, soft bool) { - switch code { - case InvalidSyntaxTree: - msg = "invalid syntax tree: " + msg - case 0: - panic("no error code provided") +// report reports the error err, setting check.firstError if necessary. +func (err *error_) report() { + if err.empty() { + panic("no error to report") } - // Cheap trick: Don't report errors with messages containing - // "invalid operand" or "invalid type" as those tend to be - // follow-on errors which don't add useful information. Only - // exclude them if these strings are not at the beginning, - // and only if we have at least one error already reported. - if check.firstErr != nil && (strings.Index(msg, "invalid operand") > 0 || strings.Index(msg, "invalid type") > 0) { - return + msg := err.msg() + code := err.code + assert(code != 0) + if code == InvalidSyntaxTree { + msg = "invalid syntax tree: " + msg } - pos := atPos(at) - // If we are encountering an error while evaluating an inherited // constant initialization expression, pos is the position of in // the original expression, and not of the currently declared // constant identifier. Use the provided errpos instead. // TODO(gri) We may also want to augment the error message and // refer to the position (pos) in the original expression. + check := err.check + pos := err.pos() if check.errpos.IsKnown() { assert(check.iota != nil) pos = check.errpos } + if check.conf.Trace { + check.trace(pos, "ERROR: %s", msg) + } + + // Cheap trick: Don't report errors with messages containing + // "invalid operand" or "invalid type" as those tend to be + // follow-on errors which don't add useful information. Only + // exclude them if these strings are not at the beginning, + // and only if we have at least one error already reported. + isInvalidErr := strings.Index(msg, "invalid operand") > 0 || strings.Index(msg, "invalid type") > 0 + if check.firstErr != nil && isInvalidErr { + return + } + // If we have a URL for error codes, add a link to the first line. - if code != 0 && check.conf.ErrorURL != "" { + if check.conf.ErrorURL != "" { u := fmt.Sprintf(check.conf.ErrorURL, code) if i := strings.Index(msg, "\n"); i >= 0 { msg = msg[:i] + u + msg[i:] @@ -256,20 +267,27 @@ func (check *Checker) err(at poser, code Code, msg string, soft bool) { } } - err := Error{pos, stripAnnotations(msg), msg, soft, code} - if check.firstErr == nil { - check.firstErr = err + e := Error{ + Pos: pos, + Msg: stripAnnotations(msg), + Full: msg, + Soft: err.soft, + Code: code, } - if check.conf.Trace { - check.trace(pos, "ERROR: %s", msg) + if check.firstErr == nil { + check.firstErr = e } f := check.conf.Error if f == nil { panic(bailout{}) // report only first error } - f(err) + + // TODO(gri) If e contains \t-indented sub-errors, + // for go/types f must be called for each + // of those sub-errors. + f(e) } const ( @@ -282,21 +300,29 @@ type poser interface { } func (check *Checker) error(at poser, code Code, msg string) { - check.err(at, code, msg, false) + err := check.newError(code) + err.addf(at, "%s", msg) + err.report() } func (check *Checker) errorf(at poser, code Code, format string, args ...interface{}) { - check.err(at, code, check.sprintf(format, args...), false) + err := check.newError(code) + err.addf(at, format, args...) + err.report() } func (check *Checker) softErrorf(at poser, code Code, format string, args ...interface{}) { - check.err(at, code, check.sprintf(format, args...), true) + err := check.newError(code) + err.addf(at, format, args...) + err.soft = true + err.report() } func (check *Checker) versionErrorf(at poser, v goVersion, format string, args ...interface{}) { msg := check.sprintf(format, args...) - msg = fmt.Sprintf("%s requires %s or later", msg, v) - check.err(at, UnsupportedFeature, msg, true) + err := check.newError(UnsupportedFeature) + err.addf(at, "%s requires %s or later", msg, v) + err.report() } // atPos reports the left (= start) position of at. diff --git a/src/cmd/compile/internal/types2/errors_test.go b/src/cmd/compile/internal/types2/errors_test.go index ac73ca4650..cfa52472b2 100644 --- a/src/cmd/compile/internal/types2/errors_test.go +++ b/src/cmd/compile/internal/types2/errors_test.go @@ -9,19 +9,19 @@ import "testing" func TestError(t *testing.T) { var err error_ want := "no error" - if got := err.String(); got != want { + if got := err.msg(); got != want { t.Errorf("empty error: got %q, want %q", got, want) } - want = ": foo 42" - err.errorf(nopos, "foo %d", 42) - if got := err.String(); got != want { + want = "foo 42" + err.addf(nopos, "foo %d", 42) + if got := err.msg(); got != want { t.Errorf("simple error: got %q, want %q", got, want) } - want = ": foo 42\n\tbar 43" - err.errorf(nopos, "bar %d", 43) - if got := err.String(); got != want { + want = "foo 42\n\tbar 43" + err.addf(nopos, "bar %d", 43) + if got := err.msg(); got != want { t.Errorf("simple error: got %q, want %q", got, want) } } diff --git a/src/cmd/compile/internal/types2/infer.go b/src/cmd/compile/internal/types2/infer.go index 071d11aafa..7499135733 100644 --- a/src/cmd/compile/internal/types2/infer.go +++ b/src/cmd/compile/internal/types2/infer.go @@ -127,7 +127,7 @@ func (check *Checker) infer(pos syntax.Pos, tparams []*TypeParam, targs []Type, } } if allFailed { - err.errorf(arg, "type %s of %s does not match %s (cannot infer %s)", targ, arg.expr, tpar, typeParamsString(tparams)) + err.addf(arg, "type %s of %s does not match %s (cannot infer %s)", targ, arg.expr, tpar, typeParamsString(tparams)) return } } @@ -140,12 +140,12 @@ func (check *Checker) infer(pos syntax.Pos, tparams []*TypeParam, targs []Type, // the more general CannotInferTypeArgs. if inferred != tpar { if reverse { - err.errorf(arg, "inferred type %s for %s does not match type %s of %s", inferred, tpar, targ, arg.expr) + err.addf(arg, "inferred type %s for %s does not match type %s of %s", inferred, tpar, targ, arg.expr) } else { - err.errorf(arg, "type %s of %s does not match inferred type %s for %s", targ, arg.expr, inferred, tpar) + err.addf(arg, "type %s of %s does not match inferred type %s for %s", targ, arg.expr, inferred, tpar) } } else { - err.errorf(arg, "type %s of %s does not match %s", targ, arg.expr, tpar) + err.addf(arg, "type %s of %s does not match %s", targ, arg.expr, tpar) } } @@ -252,7 +252,7 @@ func (check *Checker) infer(pos syntax.Pos, tparams []*TypeParam, targs []Type, // TODO(gri) Type parameters that appear in the constraint and // for which we have type arguments inferred should // use those type arguments for a better error message. - err.errorf(pos, "%s (type %s) does not satisfy %s", tpar, tx, tpar.Constraint()) + err.addf(pos, "%s (type %s) does not satisfy %s", tpar, tx, tpar.Constraint()) return nil } case single && !core.tilde: @@ -277,7 +277,7 @@ func (check *Checker) infer(pos syntax.Pos, tparams []*TypeParam, targs []Type, constraint := tpar.iface() if m, _ := check.missingMethod(tx, constraint, true, func(x, y Type) bool { return u.unify(x, y, exact) }, &cause); m != nil { // TODO(gri) better error message (see TODO above) - err.errorf(pos, "%s (type %s) does not satisfy %s %s", tpar, tx, tpar.Constraint(), cause) + err.addf(pos, "%s (type %s) does not satisfy %s %s", tpar, tx, tpar.Constraint(), cause) return nil } } @@ -318,7 +318,7 @@ func (check *Checker) infer(pos syntax.Pos, tparams []*TypeParam, targs []Type, } else { m := maxType(max, arg.typ) if m == nil { - err.errorf(arg, "mismatched types %s and %s (cannot infer %s)", max, arg.typ, tpar) + err.addf(arg, "mismatched types %s and %s (cannot infer %s)", max, arg.typ, tpar) return nil } max = m @@ -427,7 +427,7 @@ func (check *Checker) infer(pos syntax.Pos, tparams []*TypeParam, targs []Type, for i, typ := range inferred { if typ == nil || isParameterized(tparams, typ) { obj := tparams[i].obj - err.errorf(pos, "cannot infer %s (%s)", obj.name, obj.pos) + err.addf(pos, "cannot infer %s (%s)", obj.name, obj.pos) return nil } } diff --git a/src/cmd/compile/internal/types2/initorder.go b/src/cmd/compile/internal/types2/initorder.go index 6e041721e8..f864876a70 100644 --- a/src/cmd/compile/internal/types2/initorder.go +++ b/src/cmd/compile/internal/types2/initorder.go @@ -160,17 +160,16 @@ func (check *Checker) reportCycle(cycle []Object) { return } - var err error_ - err.code = InvalidInitCycle - err.errorf(obj, "initialization cycle for %s", obj.Name()) + err := check.newError(InvalidInitCycle) + err.addf(obj, "initialization cycle for %s", obj.Name()) // subtle loop: print cycle[i] for i = 0, n-1, n-2, ... 1 for len(cycle) = n for i := len(cycle) - 1; i >= 0; i-- { - err.errorf(obj, "%s refers to", obj.Name()) + err.addf(obj, "%s refers to", obj.Name()) obj = cycle[i] } // print cycle[0] again to close the cycle - err.errorf(obj, "%s", obj.Name()) - check.report(&err) + err.addf(obj, "%s", obj.Name()) + err.report() } // ---------------------------------------------------------------------------- diff --git a/src/cmd/compile/internal/types2/labels.go b/src/cmd/compile/internal/types2/labels.go index ffb37004ce..37d10d4a51 100644 --- a/src/cmd/compile/internal/types2/labels.go +++ b/src/cmd/compile/internal/types2/labels.go @@ -133,12 +133,11 @@ func (check *Checker) blockBranches(all *Scope, parent *block, lstmt *syntax.Lab if name := s.Label.Value; name != "_" { lbl := NewLabel(s.Label.Pos(), check.pkg, name) if alt := all.Insert(lbl); alt != nil { - var err error_ - err.code = DuplicateLabel + err := check.newError(DuplicateLabel) err.soft = true - err.errorf(lbl.pos, "label %s already declared", name) + err.addf(lbl.pos, "label %s already declared", name) err.recordAltDecl(alt) - check.report(&err) + err.report() // ok to continue } else { b.insert(s) diff --git a/src/cmd/compile/internal/types2/mono.go b/src/cmd/compile/internal/types2/mono.go index dae9230252..72bbc24ba0 100644 --- a/src/cmd/compile/internal/types2/mono.go +++ b/src/cmd/compile/internal/types2/mono.go @@ -137,10 +137,9 @@ func (check *Checker) reportInstanceLoop(v int) { // TODO(mdempsky): Pivot stack so we report the cycle from the top? - var err error_ - err.code = InvalidInstanceCycle + err := check.newError(InvalidInstanceCycle) obj0 := check.mono.vertices[v].obj - err.errorf(obj0, "instantiation cycle:") + err.addf(obj0, "instantiation cycle:") qf := RelativeTo(check.pkg) for _, v := range stack { @@ -151,12 +150,12 @@ func (check *Checker) reportInstanceLoop(v int) { default: panic("unexpected type") case *Named: - err.errorf(edge.pos, "%s implicitly parameterized by %s", obj.Name(), TypeString(edge.typ, qf)) // secondary error, \t indented + err.addf(edge.pos, "%s implicitly parameterized by %s", obj.Name(), TypeString(edge.typ, qf)) // secondary error, \t indented case *TypeParam: - err.errorf(edge.pos, "%s instantiated as %s", obj.Name(), TypeString(edge.typ, qf)) // secondary error, \t indented + err.addf(edge.pos, "%s instantiated as %s", obj.Name(), TypeString(edge.typ, qf)) // secondary error, \t indented } } - check.report(&err) + err.report() } // recordCanon records that tpar is the canonical type parameter diff --git a/src/cmd/compile/internal/types2/resolver.go b/src/cmd/compile/internal/types2/resolver.go index 0cf7c9142e..3bc29ae8d5 100644 --- a/src/cmd/compile/internal/types2/resolver.go +++ b/src/cmd/compile/internal/types2/resolver.go @@ -314,11 +314,10 @@ func (check *Checker) collectObjects() { // the object may be imported into more than one file scope // concurrently. See go.dev/issue/32154.) if alt := fileScope.Lookup(name); alt != nil { - var err error_ - err.code = DuplicateDecl - err.errorf(s.LocalPkgName, "%s redeclared in this block", alt.Name()) + err := check.newError(DuplicateDecl) + err.addf(s.LocalPkgName, "%s redeclared in this block", alt.Name()) err.recordAltDecl(alt) - check.report(&err) + err.report() } else { fileScope.insert(name, obj) check.dotImportMap[dotImportKey{fileScope, name}] = pkgName @@ -473,17 +472,16 @@ func (check *Checker) collectObjects() { for name, obj := range scope.elems { if alt := pkg.scope.Lookup(name); alt != nil { obj = resolve(name, obj) - var err error_ - err.code = DuplicateDecl + err := check.newError(DuplicateDecl) if pkg, ok := obj.(*PkgName); ok { - err.errorf(alt, "%s already declared through import of %s", alt.Name(), pkg.Imported()) + err.addf(alt, "%s already declared through import of %s", alt.Name(), pkg.Imported()) err.recordAltDecl(pkg) } else { - err.errorf(alt, "%s already declared through dot-import of %s", alt.Name(), obj.Pkg()) + err.addf(alt, "%s already declared through dot-import of %s", alt.Name(), obj.Pkg()) // TODO(gri) dot-imported objects don't have a position; recordAltDecl won't print anything err.recordAltDecl(obj) } - check.report(&err) + err.report() } } } diff --git a/src/cmd/compile/internal/types2/signature.go b/src/cmd/compile/internal/types2/signature.go index 72b57bc842..4482de51e8 100644 --- a/src/cmd/compile/internal/types2/signature.go +++ b/src/cmd/compile/internal/types2/signature.go @@ -186,11 +186,10 @@ func (check *Checker) funcType(sig *Signature, recvPar *syntax.Field, tparams [] params, variadic := check.collectParams(scope, ftyp.ParamList, true, scopePos) results, _ := check.collectParams(scope, ftyp.ResultList, false, scopePos) scope.Squash(func(obj, alt Object) { - var err error_ - err.code = DuplicateDecl - err.errorf(obj, "%s redeclared in this block", obj.Name()) + err := check.newError(DuplicateDecl) + err.addf(obj, "%s redeclared in this block", obj.Name()) err.recordAltDecl(alt) - check.report(&err) + err.report() }) if recvPar != nil { diff --git a/src/cmd/compile/internal/types2/stmt.go b/src/cmd/compile/internal/types2/stmt.go index 0ec5829ee4..e79e4cd586 100644 --- a/src/cmd/compile/internal/types2/stmt.go +++ b/src/cmd/compile/internal/types2/stmt.go @@ -260,11 +260,10 @@ L: // (quadratic algorithm, but these lists tend to be very short) for _, vt := range seen[val] { if Identical(v.typ, vt.typ) { - var err error_ - err.code = DuplicateCase - err.errorf(&v, "duplicate case %s in expression switch", &v) - err.errorf(vt.pos, "previous case") - check.report(&err) + err := check.newError(DuplicateCase) + err.addf(&v, "duplicate case %s in expression switch", &v) + err.addf(vt.pos, "previous case") + err.report() continue L } } @@ -307,11 +306,10 @@ L: if T != nil { Ts = TypeString(T, check.qualifier) } - var err error_ - err.code = DuplicateCase - err.errorf(e, "duplicate case %s in type switch", Ts) - err.errorf(other, "previous case") - check.report(&err) + err := check.newError(DuplicateCase) + err.addf(e, "duplicate case %s in type switch", Ts) + err.addf(other, "previous case") + err.report() continue L } } @@ -350,11 +348,10 @@ L: // if T != nil { // Ts = TypeString(T, check.qualifier) // } -// var err error_ -// err.code = _DuplicateCase -// err.errorf(e, "duplicate case %s in type switch", Ts) -// err.errorf(other, "previous case") -// check.report(&err) +// err := check.newError(_DuplicateCase) +// err.addf(e, "duplicate case %s in type switch", Ts) +// err.addf(other, "previous case") +// err.report() // continue L // } // seen[hash] = e @@ -498,11 +495,10 @@ func (check *Checker) stmt(ctxt stmtContext, s syntax.Stmt) { // with the same name as a result parameter is in scope at the place of the return." for _, obj := range res.vars { if alt := check.lookup(obj.name); alt != nil && alt != obj { - var err error_ - err.code = OutOfScopeResult - err.errorf(s, "result parameter %s not in scope at return", obj.name) - err.errorf(alt, "inner declaration of %s", obj) - check.report(&err) + err := check.newError(OutOfScopeResult) + err.addf(s, "result parameter %s not in scope at return", obj.name) + err.addf(alt, "inner declaration of %s", obj) + err.report() // ok to continue } } diff --git a/src/cmd/compile/internal/types2/struct.go b/src/cmd/compile/internal/types2/struct.go index 212e9e17fb..7f6213e772 100644 --- a/src/cmd/compile/internal/types2/struct.go +++ b/src/cmd/compile/internal/types2/struct.go @@ -200,11 +200,10 @@ func embeddedFieldIdent(e syntax.Expr) *syntax.Name { func (check *Checker) declareInSet(oset *objset, pos syntax.Pos, obj Object) bool { if alt := oset.insert(obj); alt != nil { - var err error_ - err.code = DuplicateDecl - err.errorf(pos, "%s redeclared", obj.Name()) + err := check.newError(DuplicateDecl) + err.addf(pos, "%s redeclared", obj.Name()) err.recordAltDecl(alt) - check.report(&err) + err.report() return false } return true diff --git a/src/cmd/compile/internal/types2/typeset.go b/src/cmd/compile/internal/types2/typeset.go index bf07162a21..8913a2145d 100644 --- a/src/cmd/compile/internal/types2/typeset.go +++ b/src/cmd/compile/internal/types2/typeset.go @@ -221,11 +221,10 @@ func computeInterfaceTypeSet(check *Checker, pos syntax.Pos, ityp *Interface) *_ mpos[m] = pos case explicit: if check != nil { - var err error_ - err.code = DuplicateDecl - err.errorf(pos, "duplicate method %s", m.name) - err.errorf(mpos[other.(*Func)], "other declaration of %s", m.name) - check.report(&err) + err := check.newError(DuplicateDecl) + err.addf(pos, "duplicate method %s", m.name) + err.addf(mpos[other.(*Func)], "other declaration of %s", m.name) + err.report() } default: // We have a duplicate method name in an embedded (not explicitly declared) method. @@ -236,11 +235,10 @@ func computeInterfaceTypeSet(check *Checker, pos syntax.Pos, ityp *Interface) *_ if check != nil { check.later(func() { if !check.allowVersion(m.pkg, pos, go1_14) || !Identical(m.typ, other.Type()) { - var err error_ - err.code = DuplicateDecl - err.errorf(pos, "duplicate method %s", m.name) - err.errorf(mpos[other.(*Func)], "other declaration of %s", m.name) - check.report(&err) + err := check.newError(DuplicateDecl) + err.addf(pos, "duplicate method %s", m.name) + err.addf(mpos[other.(*Func)], "other declaration of %s", m.name) + err.report() } }).describef(pos, "duplicate method check for %s", m.name) } diff --git a/src/go/types/assignments.go b/src/go/types/assignments.go index 853598b000..dd28e0618d 100644 --- a/src/go/types/assignments.go +++ b/src/go/types/assignments.go @@ -359,12 +359,11 @@ func (check *Checker) returnError(at positioner, lhs []*Var, rhs []*operand) { } else if r > 0 { at = rhs[r-1] // report at last value } - var err error_ - err.code = WrongResultCount - err.errorf(at, "%s return values", qualifier) - err.errorf(noposn, "have %s", check.typesSummary(operandTypes(rhs), false)) - err.errorf(noposn, "want %s", check.typesSummary(varTypes(lhs), false)) - check.report(&err) + err := check.newError(WrongResultCount) + err.addf(at, "%s return values", qualifier) + err.addf(noposn, "have %s", check.typesSummary(operandTypes(rhs), false)) + err.addf(noposn, "want %s", check.typesSummary(varTypes(lhs), false)) + err.report() } // initVars type-checks assignments of initialization expressions orig_rhs diff --git a/src/go/types/call.go b/src/go/types/call.go index b7775f6c6b..4a70d26964 100644 --- a/src/go/types/call.go +++ b/src/go/types/call.go @@ -111,12 +111,11 @@ func (check *Checker) funcInst(T *target, pos token.Pos, x *operand, ix *typepar // Note that NewTuple(params...) below is (*Tuple)(nil) if len(params) == 0, as desired. tparams, params2 := check.renameTParams(pos, sig.TypeParams().list(), NewTuple(params...)) - var err error_ - targs = check.infer(atPos(pos), tparams, targs, params2.(*Tuple), args, reverse, &err) + err := check.newError(CannotInferTypeArgs) + targs = check.infer(atPos(pos), tparams, targs, params2.(*Tuple), args, reverse, err) if targs == nil { if !err.empty() { - err.code = CannotInferTypeArgs - check.report(&err) + err.report() } x.mode = invalid return nil, nil @@ -529,10 +528,11 @@ func (check *Checker) arguments(call *ast.CallExpr, sig *Signature, targs []Type if sig.params != nil { params = sig.params.vars } - err := newErrorf(at, WrongArgCount, "%s arguments in call to %s", qualifier, call.Fun) - err.errorf(noposn, "have %s", check.typesSummary(operandTypes(args), false)) - err.errorf(noposn, "want %s", check.typesSummary(varTypes(params), sig.variadic)) - check.report(err) + err := check.newError(WrongArgCount) + err.addf(at, "%s arguments in call to %s", qualifier, call.Fun) + err.addf(noposn, "have %s", check.typesSummary(operandTypes(args), false)) + err.addf(noposn, "want %s", check.typesSummary(varTypes(params), sig.variadic)) + err.report() return } @@ -607,15 +607,15 @@ func (check *Checker) arguments(call *ast.CallExpr, sig *Signature, targs []Type // infer missing type arguments of callee and function arguments if len(tparams) > 0 { - var err error_ - targs = check.infer(call, tparams, targs, sigParams, args, false, &err) + err := check.newError(CannotInferTypeArgs) + targs = check.infer(call, tparams, targs, sigParams, args, false, err) if targs == nil { // TODO(gri) If infer inferred the first targs[:n], consider instantiating // the call signature for better error messages/gopls behavior. // Perhaps instantiate as much as we can, also for arguments. // This will require changes to how infer returns its results. if !err.empty() { - check.errorf(err.posn(), CannotInferTypeArgs, "in call to %s, %s", call.Fun, err.msg(check.fset, check.qualifier)) + check.errorf(err.posn(), CannotInferTypeArgs, "in call to %s, %s", call.Fun, err.msg()) } return } diff --git a/src/go/types/errors.go b/src/go/types/errors.go index 0e4b8a8c44..433dba30e7 100644 --- a/src/go/types/errors.go +++ b/src/go/types/errors.go @@ -30,11 +30,21 @@ func assert(p bool) { } // An error_ represents a type-checking error. -// To report an error_, call Checker.report. +// A new error_ is created with Checker.newError. +// To report an error_, call error_.report. type error_ struct { - desc []errorDesc - code Code - soft bool // TODO(gri) eventually determine this from an error code + check *Checker + desc []errorDesc + code Code + soft bool // TODO(gri) eventually determine this from an error code +} + +// newError returns a new error_ with the given error code. +func (check *Checker) newError(code Code) *error_ { + if code == 0 { + panic("error code must not be 0") + } + return &error_{check: check, code: code} } // An errorDesc describes part of a type-checking error. @@ -55,35 +65,33 @@ func (err *error_) posn() positioner { return err.desc[0].posn } -func (err *error_) msg(fset *token.FileSet, qf Qualifier) string { +// msg returns the formatted error message without the primary error position pos(). +func (err *error_) msg() string { if err.empty() { return "no error" } + var buf strings.Builder for i := range err.desc { p := &err.desc[i] if i > 0 { fmt.Fprint(&buf, "\n\t") if p.posn.Pos().IsValid() { - fmt.Fprintf(&buf, "%s: ", fset.Position(p.posn.Pos())) + fmt.Fprintf(&buf, "%s: ", err.check.fset.Position(p.posn.Pos())) } } - buf.WriteString(sprintf(fset, qf, false, p.format, p.args...)) + buf.WriteString(err.check.sprintf(p.format, p.args...)) } return buf.String() } -// String is for testing. -func (err *error_) String() string { - if err.empty() { - return "no error" - } - return fmt.Sprintf("%d: %s", err.posn().Pos(), err.msg(nil, nil)) -} - -// errorf adds formatted error information to err. +// addf adds formatted error information to err. // It may be called multiple times to provide additional information. -func (err *error_) errorf(at positioner, format string, args ...interface{}) { +// The position of the first call to addf determines the position of the reported Error. +// Subsequent calls to addf provide additional information in the form of additional lines +// in the error message (types2) or continuation errors identified by a tab-indented error +// message (go/types). +func (err *error_) addf(at positioner, format string, args ...interface{}) { err.desc = append(err.desc, errorDesc{at, format, args}) } @@ -209,24 +217,49 @@ func (check *Checker) dump(format string, args ...any) { fmt.Println(sprintf(check.fset, check.qualifier, true, format, args...)) } -// Report records the error pointed to by errp, setting check.firstError if -// necessary. -func (check *Checker) report(errp *error_) { - if errp.empty() { +// report reports the error err, setting check.firstError if necessary. +func (err *error_) report() { + if err.empty() { panic("empty error details") } - msg := errp.msg(check.fset, check.qualifier) - switch errp.code { - case InvalidSyntaxTree: - msg = "invalid AST: " + msg - case 0: - panic("no error code provided") + msg := err.msg() + code := err.code + assert(code != 0) + if code == InvalidSyntaxTree { + msg = "invalid syntax tree: " + msg + } + + // If we are encountering an error while evaluating an inherited + // constant initialization expression, pos is the position of in + // the original expression, and not of the currently declared + // constant identifier. Use the provided errpos instead. + // TODO(gri) We may also want to augment the error message and + // refer to the position (pos) in the original expression. + check := err.check + posn := err.posn() + if check.errpos != nil && check.errpos.Pos().IsValid() { + assert(check.iota != nil) + posn = check.errpos + } + + if check.conf._Trace { + check.trace(posn.Pos(), "ERROR: %s", msg) + } + + // Cheap trick: Don't report errors with messages containing + // "invalid operand" or "invalid type" as those tend to be + // follow-on errors which don't add useful information. Only + // exclude them if these strings are not at the beginning, + // and only if we have at least one error already reported. + isInvalidErr := strings.Index(msg, "invalid operand") > 0 || strings.Index(msg, "invalid type") > 0 + if check.firstErr != nil && isInvalidErr { + return } // If we have a URL for error codes, add a link to the first line. - if errp.code != 0 && check.conf._ErrorURL != "" { - u := fmt.Sprintf(check.conf._ErrorURL, errp.code) + if check.conf._ErrorURL != "" { + u := fmt.Sprintf(check.conf._ErrorURL, code) if i := strings.Index(msg, "\n"); i >= 0 { msg = msg[:i] + u + msg[i:] } else { @@ -234,28 +267,17 @@ func (check *Checker) report(errp *error_) { } } - span := spanOf(errp.desc[0].posn) + span := spanOf(err.desc[0].posn) e := Error{ Fset: check.fset, Pos: span.pos, - Msg: msg, - Soft: errp.soft, - go116code: errp.code, + Msg: stripAnnotations(msg), + Soft: err.soft, + go116code: code, go116start: span.start, go116end: span.end, } - // Cheap trick: Don't report errors with messages containing - // "invalid operand" or "invalid type" as those tend to be - // follow-on errors which don't add useful information. Only - // exclude them if these strings are not at the beginning, - // and only if we have at least one error already reported. - isInvalidErr := strings.Index(e.Msg, "invalid operand") > 0 || strings.Index(e.Msg, "invalid type") > 0 - if check.firstErr != nil && isInvalidErr { - return - } - - e.Msg = stripAnnotations(e.Msg) if check.errpos != nil { // If we have an internal error and the errpos override is set, use it to // augment our error positioning. @@ -266,23 +288,20 @@ func (check *Checker) report(errp *error_) { e.go116start = span.start e.go116end = span.end } - err := e if check.firstErr == nil { - check.firstErr = err - } - - if check.conf._Trace { - pos := e.Pos - msg := e.Msg - check.trace(pos, "ERROR: %s", msg) + check.firstErr = e } f := check.conf.Error if f == nil { panic(bailout{}) // report only first error } - f(err) + + // TODO(gri) If e contains \t-indented sub-errors, + // for go/types f must be called for each + // of those sub-errors. + f(e) } const ( @@ -290,39 +309,36 @@ const ( invalidOp = "invalid operation: " ) -// newErrorf creates a new error_ for later reporting with check.report. -func newErrorf(at positioner, code Code, format string, args ...any) *error_ { - return &error_{ - desc: []errorDesc{{at, format, args}}, - code: code, - } +// The positioner interface is used to extract the position of type-checker +// errors. +type positioner interface { + Pos() token.Pos } func (check *Checker) error(at positioner, code Code, msg string) { - check.report(newErrorf(at, code, "%s", msg)) + err := check.newError(code) + err.addf(at, "%s", msg) + err.report() } -func (check *Checker) errorf(at positioner, code Code, format string, args ...any) { - check.report(newErrorf(at, code, format, args...)) +func (check *Checker) errorf(at positioner, code Code, format string, args ...interface{}) { + err := check.newError(code) + err.addf(at, format, args...) + err.report() } -func (check *Checker) softErrorf(at positioner, code Code, format string, args ...any) { - err := newErrorf(at, code, format, args...) +func (check *Checker) softErrorf(at positioner, code Code, format string, args ...interface{}) { + err := check.newError(code) + err.addf(at, format, args...) err.soft = true - check.report(err) + err.report() } func (check *Checker) versionErrorf(at positioner, v goVersion, format string, args ...interface{}) { msg := check.sprintf(format, args...) - var err *error_ - err = newErrorf(at, UnsupportedFeature, "%s requires %s or later", msg, v) - check.report(err) -} - -// The positioner interface is used to extract the position of type-checker -// errors. -type positioner interface { - Pos() token.Pos + err := check.newError(UnsupportedFeature) + err.addf(at, "%s requires %s or later", msg, v) + err.report() } // posSpan holds a position range along with a highlighted position within that diff --git a/src/go/types/errors_test.go b/src/go/types/errors_test.go index 0b81730adf..5c47ef51e8 100644 --- a/src/go/types/errors_test.go +++ b/src/go/types/errors_test.go @@ -11,19 +11,19 @@ import ( func TestError(t *testing.T) { var err error_ want := "no error" - if got := err.String(); got != want { + if got := err.msg(); got != want { t.Errorf("empty error: got %q, want %q", got, want) } - want = "0: foo 42" - err.errorf(noposn, "foo %d", 42) - if got := err.String(); got != want { + want = "foo 42" + err.addf(noposn, "foo %d", 42) + if got := err.msg(); got != want { t.Errorf("simple error: got %q, want %q", got, want) } - want = "0: foo 42\n\tbar 43" - err.errorf(noposn, "bar %d", 43) - if got := err.String(); got != want { + want = "foo 42\n\tbar 43" + err.addf(noposn, "bar %d", 43) + if got := err.msg(); got != want { t.Errorf("simple error: got %q, want %q", got, want) } } diff --git a/src/go/types/generate_test.go b/src/go/types/generate_test.go index d1b69c383c..268f389b24 100644 --- a/src/go/types/generate_test.go +++ b/src/go/types/generate_test.go @@ -353,8 +353,8 @@ func fixInferSig(f *ast.File) { n.Args[0] = arg return false } - case "errorf": - // rewrite check.errorf(pos, ...) to check.errorf(posn, ...) + case "addf": + // rewrite err.addf(pos, ...) to err.addf(posn, ...) if isIdent(n.Args[0], "pos") { pos := n.Args[0].Pos() arg := newIdent(pos, "posn") @@ -400,7 +400,7 @@ func fixAtPosCall(f *ast.File) { }) } -// fixErrErrorfCall updates calls of the form err.errorf(obj, ...) to err.errorf(obj.Pos(), ...). +// fixErrErrorfCall updates calls of the form err.addf(obj, ...) to err.addf(obj.Pos(), ...). func fixErrErrorfCall(f *ast.File) { ast.Inspect(f, func(n ast.Node) bool { switch n := n.(type) { @@ -409,7 +409,7 @@ func fixErrErrorfCall(f *ast.File) { if isIdent(selx.X, "err") { switch selx.Sel.Name { case "errorf": - // rewrite err.errorf(obj, ... ) to err.errorf(obj.Pos(), ... ) + // rewrite err.addf(obj, ... ) to err.addf(obj.Pos(), ... ) if ident, _ := n.Args[0].(*ast.Ident); ident != nil && ident.Name == "obj" { pos := n.Args[0].Pos() fun := &ast.SelectorExpr{X: ident, Sel: newIdent(pos, "Pos")} diff --git a/src/go/types/infer.go b/src/go/types/infer.go index 8261ae7eb2..20da145aee 100644 --- a/src/go/types/infer.go +++ b/src/go/types/infer.go @@ -129,7 +129,7 @@ func (check *Checker) infer(posn positioner, tparams []*TypeParam, targs []Type, } } if allFailed { - err.errorf(arg, "type %s of %s does not match %s (cannot infer %s)", targ, arg.expr, tpar, typeParamsString(tparams)) + err.addf(arg, "type %s of %s does not match %s (cannot infer %s)", targ, arg.expr, tpar, typeParamsString(tparams)) return } } @@ -142,12 +142,12 @@ func (check *Checker) infer(posn positioner, tparams []*TypeParam, targs []Type, // the more general CannotInferTypeArgs. if inferred != tpar { if reverse { - err.errorf(arg, "inferred type %s for %s does not match type %s of %s", inferred, tpar, targ, arg.expr) + err.addf(arg, "inferred type %s for %s does not match type %s of %s", inferred, tpar, targ, arg.expr) } else { - err.errorf(arg, "type %s of %s does not match inferred type %s for %s", targ, arg.expr, inferred, tpar) + err.addf(arg, "type %s of %s does not match inferred type %s for %s", targ, arg.expr, inferred, tpar) } } else { - err.errorf(arg, "type %s of %s does not match %s", targ, arg.expr, tpar) + err.addf(arg, "type %s of %s does not match %s", targ, arg.expr, tpar) } } @@ -254,7 +254,7 @@ func (check *Checker) infer(posn positioner, tparams []*TypeParam, targs []Type, // TODO(gri) Type parameters that appear in the constraint and // for which we have type arguments inferred should // use those type arguments for a better error message. - err.errorf(posn, "%s (type %s) does not satisfy %s", tpar, tx, tpar.Constraint()) + err.addf(posn, "%s (type %s) does not satisfy %s", tpar, tx, tpar.Constraint()) return nil } case single && !core.tilde: @@ -279,7 +279,7 @@ func (check *Checker) infer(posn positioner, tparams []*TypeParam, targs []Type, constraint := tpar.iface() if m, _ := check.missingMethod(tx, constraint, true, func(x, y Type) bool { return u.unify(x, y, exact) }, &cause); m != nil { // TODO(gri) better error message (see TODO above) - err.errorf(posn, "%s (type %s) does not satisfy %s %s", tpar, tx, tpar.Constraint(), cause) + err.addf(posn, "%s (type %s) does not satisfy %s %s", tpar, tx, tpar.Constraint(), cause) return nil } } @@ -320,7 +320,7 @@ func (check *Checker) infer(posn positioner, tparams []*TypeParam, targs []Type, } else { m := maxType(max, arg.typ) if m == nil { - err.errorf(arg, "mismatched types %s and %s (cannot infer %s)", max, arg.typ, tpar) + err.addf(arg, "mismatched types %s and %s (cannot infer %s)", max, arg.typ, tpar) return nil } max = m @@ -429,7 +429,7 @@ func (check *Checker) infer(posn positioner, tparams []*TypeParam, targs []Type, for i, typ := range inferred { if typ == nil || isParameterized(tparams, typ) { obj := tparams[i].obj - err.errorf(posn, "cannot infer %s (%s)", obj.name, obj.pos) + err.addf(posn, "cannot infer %s (%s)", obj.name, obj.pos) return nil } } diff --git a/src/go/types/stmt.go b/src/go/types/stmt.go index ca528368b5..2bfe5890da 100644 --- a/src/go/types/stmt.go +++ b/src/go/types/stmt.go @@ -341,11 +341,10 @@ L: // if T != nil { // Ts = TypeString(T, check.qualifier) // } -// var err error_ -// err.code = DuplicateCase -// err.errorf(e, "duplicate case %s in type switch", Ts) -// err.errorf(other, "previous case") -// check.report(&err) +// err := check.newError(_DuplicateCase) +// err.addf(e, "duplicate case %s in type switch", Ts) +// err.addf(other, "previous case") +// err.report() // continue L // } // seen[hash] = e -- 2.50.0