diff --git a/src/cmd/compile/internal/noder/irgen.go b/src/cmd/compile/internal/noder/irgen.go index 06b234c31d..2de8c3fa60 100644 --- a/src/cmd/compile/internal/noder/irgen.go +++ b/src/cmd/compile/internal/noder/irgen.go @@ -41,14 +41,6 @@ func check2(noders []*noder) { CompilerErrorMessages: true, // use error strings matching existing compiler errors Error: func(err error) { terr := err.(types2.Error) - if len(terr.Msg) > 0 && terr.Msg[0] == '\t' { - // types2 reports error clarifications via separate - // error messages which are indented with a tab. - // Ignore them to satisfy tools and tests that expect - // only one error in such cases. - // TODO(gri) Need to adjust error reporting in types2. - return - } base.ErrorfAt(m.makeXPos(terr.Pos), "%s", terr.Msg) }, Importer: &gcimports{ diff --git a/src/cmd/compile/internal/types2/check_test.go b/src/cmd/compile/internal/types2/check_test.go index 9c1d278520..fc6f46b4b8 100644 --- a/src/cmd/compile/internal/types2/check_test.go +++ b/src/cmd/compile/internal/types2/check_test.go @@ -146,11 +146,7 @@ func checkFiles(t *testing.T, sources []string, goVersion string, colDelta uint, t.Error(err) return } - // Ignore secondary error messages starting with "\t"; - // they are clarifying messages for a primary error. - if !strings.Contains(err.Error(), ": \t") { - errlist = append(errlist, err) - } + errlist = append(errlist, err) } conf.Check(pkgName, files, nil) diff --git a/src/cmd/compile/internal/types2/decl.go b/src/cmd/compile/internal/types2/decl.go index f0a037adb0..3528669bf9 100644 --- a/src/cmd/compile/internal/types2/decl.go +++ b/src/cmd/compile/internal/types2/decl.go @@ -11,12 +11,12 @@ import ( "go/constant" ) -func (check *Checker) reportAltDecl(obj Object) { +func (err *error_) recordAltDecl(obj Object) { if pos := obj.Pos(); pos.IsKnown() { // We use "other" rather than "previous" here because // the first declaration seen may not be textually // earlier in the source. - check.errorf(pos, "\tother declaration of %s", obj.Name()) // secondary error, \t indented + err.errorf(pos, "other declaration of %s", obj.Name()) } } @@ -27,8 +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 { - check.errorf(obj.Pos(), "%s redeclared in this block", obj.Name()) - check.reportAltDecl(alt) + var err error_ + err.errorf(obj, "%s redeclared in this block", obj.Name()) + err.recordAltDecl(alt) + check.report(&err) return } obj.setScopePos(pos) @@ -364,20 +366,22 @@ func (check *Checker) cycleError(cycle []Object) { // cycle? That would be more consistent with other error messages. i := firstInSrc(cycle) obj := cycle[i] + var err error_ if check.conf.CompilerErrorMessages { - check.errorf(obj.Pos(), "invalid recursive type %s", obj.Name()) + err.errorf(obj, "invalid recursive type %s", obj.Name()) } else { - check.errorf(obj.Pos(), "illegal cycle in declaration of %s", obj.Name()) + err.errorf(obj, "illegal cycle in declaration of %s", obj.Name()) } for range cycle { - check.errorf(obj.Pos(), "\t%s refers to", obj.Name()) // secondary error, \t indented + err.errorf(obj, "%s refers to", obj.Name()) i++ if i >= len(cycle) { i = 0 } obj = cycle[i] } - check.errorf(obj.Pos(), "\t%s", obj.Name()) + err.errorf(obj, "%s", obj.Name()) + check.report(&err) } // TODO(gri) This functionality should probably be with the Pos implementation. @@ -787,19 +791,21 @@ func (check *Checker) collectMethods(obj *TypeName) { // to it must be unique." assert(m.name != "_") if alt := mset.insert(m); alt != nil { + var err error_ switch alt.(type) { case *Var: - check.errorf(m.pos, "field and method with the same name %s", m.name) + err.errorf(m.pos, "field and method with the same name %s", m.name) case *Func: if check.conf.CompilerErrorMessages { - check.errorf(m.pos, "%s.%s redeclared in this block", obj.Name(), m.name) + err.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) + err.errorf(m.pos, "method %s already declared for %s", m.name, obj) } default: unreachable() } - check.reportAltDecl(alt) + err.recordAltDecl(alt) + check.report(&err) continue } diff --git a/src/cmd/compile/internal/types2/errors.go b/src/cmd/compile/internal/types2/errors.go index 62b1d39d83..01df50c8e3 100644 --- a/src/cmd/compile/internal/types2/errors.go +++ b/src/cmd/compile/internal/types2/errors.go @@ -29,6 +29,83 @@ func unreachable() { panic("unreachable") } +// An error_ represents a type-checking error. +// To report an error_, call Checker.report. +type error_ struct { + desc []errorDesc + soft bool // TODO(gri) eventually determine this from an error code +} + +// An errorDesc describes part of a type-checking error. +type errorDesc struct { + pos syntax.Pos + format string + args []interface{} +} + +func (err *error_) empty() bool { + return err.desc == nil +} + +func (err *error_) pos() syntax.Pos { + if err.empty() { + return nopos + } + return err.desc[0].pos +} + +func (err *error_) msg(qf Qualifier) string { + if err.empty() { + return "no error" + } + var buf bytes.Buffer + for i := range err.desc { + p := &err.desc[i] + if i > 0 { + fmt.Fprintf(&buf, "\n\t%s: ", p.pos) + } + buf.WriteString(sprintf(qf, 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. +// It may be called multiple times to provide additional information. +func (err *error_) errorf(at poser, format string, args ...interface{}) { + err.desc = append(err.desc, errorDesc{posFor(at), format, args}) +} + +func sprintf(qf Qualifier, format string, args ...interface{}) string { + for i, arg := range args { + switch a := arg.(type) { + case nil: + arg = "" + case operand: + panic("internal error: should always pass *operand") + case *operand: + arg = operandString(a, qf) + case syntax.Pos: + arg = a.String() + case syntax.Expr: + arg = syntax.String(a) + case Object: + arg = ObjectString(a, qf) + case Type: + arg = TypeString(a, qf) + } + args[i] = arg + } + return fmt.Sprintf(format, args...) +} + func (check *Checker) qualifier(pkg *Package) string { // Qualify the package unless it's the package being type-checked. if pkg != check.pkg { @@ -42,26 +119,14 @@ func (check *Checker) qualifier(pkg *Package) string { } func (check *Checker) sprintf(format string, args ...interface{}) string { - for i, arg := range args { - switch a := arg.(type) { - case nil: - arg = "" - case operand: - panic("internal error: should always pass *operand") - case *operand: - arg = operandString(a, check.qualifier) - case syntax.Pos: - arg = a.String() - case syntax.Expr: - arg = syntax.String(a) - case Object: - arg = ObjectString(a, check.qualifier) - case Type: - arg = TypeString(a, check.qualifier) - } - args[i] = arg + return sprintf(check.qualifier, format, args...) +} + +func (check *Checker) report(err *error_) { + if err.empty() { + panic("internal error: reporting no error") } - return fmt.Sprintf(format, args...) + check.err(err.pos(), err.msg(check.qualifier), err.soft) } func (check *Checker) trace(pos syntax.Pos, format string, args ...interface{}) { diff --git a/src/cmd/compile/internal/types2/errors_test.go b/src/cmd/compile/internal/types2/errors_test.go index cb21ff1ad3..e1f0e83fc9 100644 --- a/src/cmd/compile/internal/types2/errors_test.go +++ b/src/cmd/compile/internal/types2/errors_test.go @@ -6,6 +6,26 @@ package types2 import "testing" +func TestError(t *testing.T) { + var err error_ + want := "no error" + if got := err.String(); 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 { + t.Errorf("simple error: got %q, want %q", got, want) + } + + want = ": foo 42\n\t: bar 43" + err.errorf(nopos, "bar %d", 43) + if got := err.String(); got != want { + t.Errorf("simple error: got %q, want %q", got, want) + } +} + func TestStripAnnotations(t *testing.T) { for _, test := range []struct { in, want string diff --git a/src/cmd/compile/internal/types2/initorder.go b/src/cmd/compile/internal/types2/initorder.go index a9cabecdf2..4081627666 100644 --- a/src/cmd/compile/internal/types2/initorder.go +++ b/src/cmd/compile/internal/types2/initorder.go @@ -151,18 +151,20 @@ func findPath(objMap map[Object]*declInfo, from, to Object, seen map[Object]bool // reportCycle reports an error for the given cycle. func (check *Checker) reportCycle(cycle []Object) { obj := cycle[0] + var err error_ if check.conf.CompilerErrorMessages { - check.errorf(obj, "initialization loop for %s", obj.Name()) + err.errorf(obj, "initialization loop for %s", obj.Name()) } else { - check.errorf(obj, "initialization cycle for %s", obj.Name()) + err.errorf(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-- { - check.errorf(obj, "\t%s refers to", obj.Name()) // secondary error, \t indented + err.errorf(obj, "%s refers to", obj.Name()) obj = cycle[i] } // print cycle[0] again to close the cycle - check.errorf(obj, "\t%s", obj.Name()) + err.errorf(obj, "%s", obj.Name()) + check.report(&err) } // ---------------------------------------------------------------------------- diff --git a/src/cmd/compile/internal/types2/labels.go b/src/cmd/compile/internal/types2/labels.go index b20b454dea..cbbd65aa9a 100644 --- a/src/cmd/compile/internal/types2/labels.go +++ b/src/cmd/compile/internal/types2/labels.go @@ -128,8 +128,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 { - check.softErrorf(lbl.pos, "label %s already declared", name) - check.reportAltDecl(alt) + var err error_ + err.soft = true + err.errorf(lbl.pos, "label %s already declared", name) + err.recordAltDecl(alt) + check.report(&err) // ok to continue } else { b.insert(s) diff --git a/src/cmd/compile/internal/types2/resolver.go b/src/cmd/compile/internal/types2/resolver.go index 44fa51a8e5..fe551525c6 100644 --- a/src/cmd/compile/internal/types2/resolver.go +++ b/src/cmd/compile/internal/types2/resolver.go @@ -305,8 +305,10 @@ func (check *Checker) collectObjects() { // the object may be imported into more than one file scope // concurrently. See issue #32154.) if alt := fileScope.Insert(obj); alt != nil { - check.errorf(s.LocalPkgName, "%s redeclared in this block", obj.Name()) - check.reportAltDecl(alt) + var err error_ + err.errorf(s.LocalPkgName, "%s redeclared in this block", obj.Name()) + err.recordAltDecl(alt) + check.report(&err) } else { check.dotImportMap[dotImportKey{fileScope, obj}] = pkgName } @@ -456,14 +458,16 @@ func (check *Checker) collectObjects() { for _, scope := range fileScopes { for _, obj := range scope.elems { if alt := pkg.scope.Lookup(obj.Name()); alt != nil { + var err error_ if pkg, ok := obj.(*PkgName); ok { - check.errorf(alt, "%s already declared through import of %s", alt.Name(), pkg.Imported()) - check.reportAltDecl(pkg) + err.errorf(alt, "%s already declared through import of %s", alt.Name(), pkg.Imported()) + err.recordAltDecl(pkg) } else { - check.errorf(alt, "%s already declared through dot-import of %s", alt.Name(), obj.Pkg()) - // TODO(gri) dot-imported objects don't have a position; reportAltDecl won't print anything - check.reportAltDecl(obj) + err.errorf(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) } } } diff --git a/src/cmd/compile/internal/types2/stmt.go b/src/cmd/compile/internal/types2/stmt.go index 490cd0fc19..21244f6065 100644 --- a/src/cmd/compile/internal/types2/stmt.go +++ b/src/cmd/compile/internal/types2/stmt.go @@ -253,8 +253,10 @@ L: // (quadratic algorithm, but these lists tend to be very short) for _, vt := range seen[val] { if check.identical(v.typ, vt.typ) { - check.errorf(&v, "duplicate case %s in expression switch", &v) - check.error(vt.pos, "\tprevious case") // secondary error, \t indented + var err error_ + err.errorf(&v, "duplicate case %s in expression switch", &v) + err.errorf(vt.pos, "previous case") + check.report(&err) continue L } } @@ -282,8 +284,10 @@ L: if T != nil { Ts = T.String() } - check.errorf(e, "duplicate case %s in type switch", Ts) - check.error(pos, "\tprevious case") // secondary error, \t indented + var err error_ + err.errorf(e, "duplicate case %s in type switch", Ts) + err.errorf(pos, "previous case") + check.report(&err) continue L } } @@ -430,8 +434,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 { - check.errorf(s, "result parameter %s not in scope at return", obj.name) - check.errorf(alt, "\tinner declaration of %s", obj) + var err error_ + err.errorf(s, "result parameter %s not in scope at return", obj.name) + err.errorf(alt, "inner declaration of %s", obj) + check.report(&err) // ok to continue } } diff --git a/src/cmd/compile/internal/types2/typexpr.go b/src/cmd/compile/internal/types2/typexpr.go index 02f9b2804d..177fcf4215 100644 --- a/src/cmd/compile/internal/types2/typexpr.go +++ b/src/cmd/compile/internal/types2/typexpr.go @@ -352,8 +352,10 @@ func (check *Checker) funcType(sig *Signature, recvPar *syntax.Field, tparams [] params, variadic := check.collectParams(scope, ftyp.ParamList, nil, true) results, _ := check.collectParams(scope, ftyp.ResultList, nil, false) scope.Squash(func(obj, alt Object) { - check.errorf(obj, "%s redeclared in this block", obj.Name()) - check.reportAltDecl(alt) + var err error_ + err.errorf(obj, "%s redeclared in this block", obj.Name()) + err.recordAltDecl(alt) + check.report(&err) }) if recvPar != nil { @@ -796,8 +798,10 @@ func (check *Checker) collectParams(scope *Scope, list []*syntax.Field, type0 sy func (check *Checker) declareInSet(oset *objset, pos syntax.Pos, obj Object) bool { if alt := oset.insert(obj); alt != nil { - check.errorf(pos, "%s redeclared", obj.Name()) - check.reportAltDecl(alt) + var err error_ + err.errorf(pos, "%s redeclared", obj.Name()) + err.recordAltDecl(alt) + check.report(&err) return false } return true @@ -940,8 +944,10 @@ func (check *Checker) completeInterface(pos syntax.Pos, ityp *Interface) { methods = append(methods, m) mpos[m] = pos case explicit: - check.errorf(pos, "duplicate method %s", m.name) - check.errorf(mpos[other.(*Func)], "\tother declaration of %s", m.name) // secondary error, \t indented + var err error_ + err.errorf(pos, "duplicate method %s", m.name) + err.errorf(mpos[other.(*Func)], "other declaration of %s", m.name) + check.report(&err) default: // We have a duplicate method name in an embedded (not explicitly declared) method. // Check method signatures after all types are computed (issue #33656). @@ -950,8 +956,10 @@ func (check *Checker) completeInterface(pos syntax.Pos, ityp *Interface) { // error message. check.atEnd(func() { if !check.allowVersion(m.pkg, 1, 14) || !check.identical(m.typ, other.Type()) { - check.errorf(pos, "duplicate method %s", m.name) - check.errorf(mpos[other.(*Func)], "\tother declaration of %s", m.name) // secondary error, \t indented + var err error_ + err.errorf(pos, "duplicate method %s", m.name) + err.errorf(mpos[other.(*Func)], "other declaration of %s", m.name) + check.report(&err) } }) }