go/types, types2: consistently use error_ type for sub-errors (cleanup)

Also, rename reportAltDecl/recordAltDecl to addAltDecl and
move function into errors.go.

Change-Id: Ie5210d1989f1e51fc5fec483dfa6dba8c4212b59
Reviewed-on: https://go-review.googlesource.com/c/go/+/567616
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

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

@@ -11,15 +11,6 @@ import (
 	. "internal/types/errors"
 )
 
-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.
-		err.addf(pos, "other declaration of %s", obj.Name())
-	}
-}
-
 func (check *Checker) declare(scope *Scope, id *syntax.Name, obj Object, pos syntax.Pos) {
 	// spec: "The blank identifier, represented by the underscore
 	// character _, may be used in a declaration like any other
@@ -29,7 +20,7 @@ func (check *Checker) declare(scope *Scope, id *syntax.Name, obj Object, pos syn
 		if alt := scope.Insert(obj); alt != nil {
 			err := check.newError(DuplicateDecl)
 			err.addf(obj, "%s redeclared in this block", obj.Name())
-			err.recordAltDecl(alt)
+			err.addAltDecl(alt)
 			err.report()
 			return
 		}
@@ -743,7 +734,7 @@ func (check *Checker) checkFieldUniqueness(base *Named) {
 					// method, and the alt decl on the field.
 					err := check.newError(DuplicateFieldAndMethod)
 					err.addf(alt, "field and method with the same name %s", fld.name)
-					err.recordAltDecl(fld)
+					err.addAltDecl(fld)
 					err.report()
 				}
 			}

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

@@ -60,6 +60,16 @@ func (err *error_) addf(at poser, format string, args ...interface{}) {
 	err.desc = append(err.desc, errorDesc{atPos(at), err.check.sprintf(format, args...)})
 }
 
+// addAltDecl is a specialized form of addf reporting another declaration of obj.
+func (err *error_) addAltDecl(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.
+		err.addf(obj, "other declaration of %s", obj.Name())
+	}
+}
+
 func (err *error_) empty() bool {
 	return err.desc == nil
 }
@@ -136,6 +146,9 @@ func (err *error_) report() {
 	} else {
 		check.handleError(0, err.pos(), err.code, err.msg(), err.soft)
 	}
+
+	// make sure the error is not reported twice
+	err.desc = nil
 }
 
 // handleError should only be called by error_.report.

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

@@ -136,7 +136,7 @@ func (check *Checker) blockBranches(all *Scope, parent *block, lstmt *syntax.Lab
 					err := check.newError(DuplicateLabel)
 					err.soft = true
 					err.addf(lbl.pos, "label %s already declared", name)
-					err.recordAltDecl(alt)
+					err.addAltDecl(alt)
 					err.report()
 					// ok to continue
 				} else {

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

@@ -316,7 +316,7 @@ func (check *Checker) collectObjects() {
 							if alt := fileScope.Lookup(name); alt != nil {
 								err := check.newError(DuplicateDecl)
 								err.addf(s.LocalPkgName, "%s redeclared in this block", alt.Name())
-								err.recordAltDecl(alt)
+								err.addAltDecl(alt)
 								err.report()
 							} else {
 								fileScope.insert(name, obj)
@@ -474,11 +474,11 @@ func (check *Checker) collectObjects() {
 				err := check.newError(DuplicateDecl)
 				if pkg, ok := obj.(*PkgName); ok {
 					err.addf(alt, "%s already declared through import of %s", alt.Name(), pkg.Imported())
-					err.recordAltDecl(pkg)
+					err.addAltDecl(pkg)
 				} else {
 					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
+					// TODO(gri) dot-imported objects don't have a position; addAltDecl won't print anything
-					err.recordAltDecl(obj)
+					err.addAltDecl(obj)
 				}
 				err.report()
 			}

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

@@ -188,7 +188,7 @@ func (check *Checker) funcType(sig *Signature, recvPar *syntax.Field, tparams []
 	scope.Squash(func(obj, alt Object) {
 		err := check.newError(DuplicateDecl)
 		err.addf(obj, "%s redeclared in this block", obj.Name())
-		err.recordAltDecl(alt)
+		err.addAltDecl(alt)
 		err.report()
 	})
 

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

@@ -202,7 +202,7 @@ func (check *Checker) declareInSet(oset *objset, pos syntax.Pos, obj Object) boo
 	if alt := oset.insert(obj); alt != nil {
 		err := check.newError(DuplicateDecl)
 		err.addf(pos, "%s redeclared", obj.Name())
-		err.recordAltDecl(alt)
+		err.addAltDecl(alt)
 		err.report()
 		return false
 	}

src/go/types/decl.go

@@ -12,15 +12,6 @@ import (
 	. "internal/types/errors"
 )
 
-func (check *Checker) reportAltDecl(obj Object) {
-	if pos := obj.Pos(); pos.IsValid() {
-		// We use "other" rather than "previous" here because
-		// the first declaration seen may not be textually
-		// earlier in the source.
-		check.errorf(obj, DuplicateDecl, "\tother declaration of %s", obj.Name()) // secondary error, \t indented
-	}
-}
-
 func (check *Checker) declare(scope *Scope, id *ast.Ident, obj Object, pos token.Pos) {
 	// spec: "The blank identifier, represented by the underscore
 	// character _, may be used in a declaration like any other
@@ -28,8 +19,10 @@ func (check *Checker) declare(scope *Scope, id *ast.Ident, obj Object, pos token
 	// binding."
 	if obj.Name() != "_" {
 		if alt := scope.Insert(obj); alt != nil {
-			check.errorf(obj, DuplicateDecl, "%s redeclared in this block", obj.Name())
+			err := check.newError(DuplicateDecl)
-			check.reportAltDecl(alt)
+			err.addf(obj, "%s redeclared in this block", obj.Name())
+			err.addAltDecl(alt)
+			err.report()
 			return
 		}
 		obj.setScopePos(pos)
@@ -336,14 +329,15 @@ func (check *Checker) cycleError(cycle []Object, start int) {
 		return
 	}
 
+	err := check.newError(InvalidDeclCycle)
 	if tname != nil {
-		check.errorf(obj, InvalidDeclCycle, "invalid recursive type %s", objName)
+		err.addf(obj, "invalid recursive type %s", objName)
 	} else {
-		check.errorf(obj, InvalidDeclCycle, "invalid cycle in declaration of %s", objName)
+		err.addf(obj, "invalid cycle in declaration of %s", objName)
 	}
 	i := start
 	for range cycle {
-		check.errorf(obj, InvalidDeclCycle, "\t%s refers to", objName) // secondary error, \t indented
+		err.addf(obj, "%s refers to", objName)
 		i++
 		if i >= len(cycle) {
 			i = 0
@@ -351,7 +345,8 @@ func (check *Checker) cycleError(cycle []Object, start int) {
 		obj = cycle[i]
 		objName = name(obj)
 	}
-	check.errorf(obj, InvalidDeclCycle, "\t%s", objName)
+	err.addf(obj, "%s", objName)
+	err.report()
 }
 
 // firstInSrc reports the index of the object with the "smallest"
@@ -829,8 +824,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.
-					check.errorf(alt, DuplicateFieldAndMethod, "field and method with the same name %s", fld.name)
+					err := check.newError(DuplicateFieldAndMethod)
-					check.reportAltDecl(fld)
+					err.addf(alt, "field and method with the same name %s", fld.name)
+					err.addAltDecl(fld)
+					err.report()
 				}
 			}
 		}

src/go/types/errors.go

@@ -61,6 +61,16 @@ func (err *error_) addf(at positioner, format string, args ...interface{}) {
 	err.desc = append(err.desc, errorDesc{at, err.check.sprintf(format, args...)})
 }
 
+// addAltDecl is a specialized form of addf reporting another declaration of obj.
+func (err *error_) addAltDecl(obj Object) {
+	if pos := obj.Pos(); pos.IsValid() {
+		// We use "other" rather than "previous" here because
+		// the first declaration seen may not be textually
+		// earlier in the source.
+		err.addf(obj, "other declaration of %s", obj.Name())
+	}
+}
+
 func (err *error_) empty() bool {
 	return err.desc == nil
 }
@@ -137,6 +147,9 @@ func (err *error_) report() {
 	} else {
 		check.handleError(0, err.posn(), err.code, err.msg(), err.soft)
 	}
+
+	// make sure the error is not reported twice
+	err.desc = nil
 }
 
 // handleError should only be called by error_.report.

src/go/types/labels.go

@@ -138,8 +138,11 @@ func (check *Checker) blockBranches(all *Scope, parent *block, lstmt *ast.Labele
 			if name := s.Label.Name; name != "_" {
 				lbl := NewLabel(s.Label.Pos(), check.pkg, name)
 				if alt := all.Insert(lbl); alt != nil {
-					check.softErrorf(lbl, DuplicateLabel, "label %s already declared", name)
+					err := check.newError(DuplicateLabel)
-					check.reportAltDecl(alt)
+					err.soft = true
+					err.addf(lbl, "label %s already declared", name)
+					err.addAltDecl(alt)
+					err.report()
 					// ok to continue
 				} else {
 					b.insert(s)

src/go/types/resolver.go

@@ -327,8 +327,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 {
-								check.errorf(d.spec.Name, DuplicateDecl, "%s redeclared in this block", alt.Name())
+								err := check.newError(DuplicateDecl)
-								check.reportAltDecl(alt)
+								err.addf(d.spec.Name, "%s redeclared in this block", alt.Name())
+								err.addAltDecl(alt)
+								err.report()
 							} else {
 								fileScope.insert(name, obj)
 								check.dotImportMap[dotImportKey{fileScope, name}] = pkgName
@@ -458,14 +460,16 @@ func (check *Checker) collectObjects() {
 		for name, obj := range scope.elems {
 			if alt := pkg.scope.Lookup(name); alt != nil {
 				obj = resolve(name, obj)
+				err := check.newError(DuplicateDecl)
 				if pkg, ok := obj.(*PkgName); ok {
-					check.errorf(alt, DuplicateDecl, "%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())
-					check.reportAltDecl(pkg)
+					err.addAltDecl(pkg)
 				} else {
-					check.errorf(alt, DuplicateDecl, "%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; reportAltDecl won't print anything
+					// TODO(gri) dot-imported objects don't have a position; addAltDecl won't print anything
-					check.reportAltDecl(obj)
+					err.addAltDecl(obj)
 				}
+				err.report()
 			}
 		}
 	}

src/go/types/signature.go

@@ -193,8 +193,10 @@ func (check *Checker) funcType(sig *Signature, recvPar *ast.FieldList, ftyp *ast
 	params, variadic := check.collectParams(scope, ftyp.Params, true, scopePos)
 	results, _ := check.collectParams(scope, ftyp.Results, false, scopePos)
 	scope.squash(func(obj, alt Object) {
-		check.errorf(obj, DuplicateDecl, "%s redeclared in this block", obj.Name())
+		err := check.newError(DuplicateDecl)
-		check.reportAltDecl(alt)
+		err.addf(obj, "%s redeclared in this block", obj.Name())
+		err.addAltDecl(alt)
+		err.report()
 	})
 
 	if recvPar != nil {

src/go/types/stmt.go

@@ -257,8 +257,10 @@ L:
 			// (quadratic algorithm, but these lists tend to be very short)
 			for _, vt := range seen[val] {
 				if Identical(v.typ, vt.typ) {
-					check.errorf(&v, DuplicateCase, "duplicate case %s in expression switch", &v)
+					err := check.newError(DuplicateCase)
-					check.error(atPos(vt.pos), DuplicateCase, "\tprevious case") // secondary error, \t indented
+					err.addf(&v, "duplicate case %s in expression switch", &v)
+					err.addf(atPos(vt.pos), "previous case")
+					err.report()
 					continue L
 				}
 			}
@@ -301,8 +303,10 @@ L:
 				if T != nil {
 					Ts = TypeString(T, check.qualifier)
 				}
-				check.errorf(e, DuplicateCase, "duplicate case %s in type switch", Ts)
+				err := check.newError(DuplicateCase)
-				check.error(other, DuplicateCase, "\tprevious case") // secondary error, \t indented
+				err.addf(e, "duplicate case %s in type switch", Ts)
+				err.addf(other, "previous case")
+				err.report()
 				continue L
 			}
 		}
@@ -510,8 +514,10 @@ func (check *Checker) stmt(ctxt stmtContext, s ast.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, OutOfScopeResult, "result parameter %s not in scope at return", obj.name)
+					err := check.newError(OutOfScopeResult)
-					check.errorf(alt, OutOfScopeResult, "\tinner declaration of %s", obj)
+					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
 				}
 			}

src/go/types/struct.go

@@ -199,8 +199,10 @@ func embeddedFieldIdent(e ast.Expr) *ast.Ident {
 
 func (check *Checker) declareInSet(oset *objset, pos token.Pos, obj Object) bool {
 	if alt := oset.insert(obj); alt != nil {
-		check.errorf(atPos(pos), DuplicateDecl, "%s redeclared", obj.Name())
+		err := check.newError(DuplicateDecl)
-		check.reportAltDecl(alt)
+		err.addf(atPos(pos), "%s redeclared", obj.Name())
+		err.addAltDecl(alt)
+		err.report()
 		return false
 	}
 	return true