diff --git a/gopls/internal/lsp/cache/check.go b/gopls/internal/lsp/cache/check.go index 6852c741f08..50ce8955014 100644 --- a/gopls/internal/lsp/cache/check.go +++ b/gopls/internal/lsp/cache/check.go @@ -1457,8 +1457,10 @@ func typeCheckImpl(ctx context.Context, b *typeCheckBatch, inputs typeCheckInput diags, err := typeErrorDiagnostics(inputs.moduleMode, inputs.linkTarget, pkg, e) if err != nil { // If we fail here and there are no parse errors, it means we are hiding - // a valid type-checking error from the user. This must be a bug. - if len(pkg.parseErrors) == 0 { + // a valid type-checking error from the user. This must be a bug, with + // one exception: relocated primary errors may fail processing, because + // they reference locations outside of the package. + if len(pkg.parseErrors) == 0 && !e.relocated { bug.Reportf("failed to compute position for type error %v: %v", e, err) } continue @@ -1788,6 +1790,7 @@ func missingPkgError(from PackageID, pkgPath string, moduleMode bool) error { } type extendedError struct { + relocated bool // if set, this is a relocation of a primary error to a secondary location primary types.Error secondaries []types.Error } @@ -1840,7 +1843,7 @@ func expandErrors(errs []types.Error, supportsRelatedInformation bool) []extende // Copy over the secondary errors, noting the location of the // current error we're cloning. - clonedError := extendedError{primary: relocatedSecondary, secondaries: []types.Error{original.primary}} + clonedError := extendedError{relocated: true, primary: relocatedSecondary, secondaries: []types.Error{original.primary}} for j, secondary := range original.secondaries { if i == j { secondary.Msg += " (this error)" @@ -1849,7 +1852,6 @@ func expandErrors(errs []types.Error, supportsRelatedInformation bool) []extende } result = append(result, clonedError) } - } return result } diff --git a/gopls/internal/regtest/marker/marker_test.go b/gopls/internal/regtest/marker/marker_test.go index 41c8e4697cb..557c2228d79 100644 --- a/gopls/internal/regtest/marker/marker_test.go +++ b/gopls/internal/regtest/marker/marker_test.go @@ -8,11 +8,13 @@ import ( "os" "testing" + "golang.org/x/tools/gopls/internal/bug" . "golang.org/x/tools/gopls/internal/lsp/regtest" "golang.org/x/tools/internal/testenv" ) func TestMain(m *testing.M) { + bug.PanicOnBugs = true testenv.ExitIfSmallMachine() os.Exit(m.Run()) } diff --git a/gopls/internal/regtest/marker/testdata/references/issue60676.txt b/gopls/internal/regtest/marker/testdata/references/issue60676.txt index 9116d77100a..cacf6fd4cff 100644 --- a/gopls/internal/regtest/marker/testdata/references/issue60676.txt +++ b/gopls/internal/regtest/marker/testdata/references/issue60676.txt @@ -5,6 +5,9 @@ shared by types from multiple packages. See golang/go#60676. Note that the marker test runner awaits the initial workspace load, so export data should be populated at the time references are requested. +-- flags -- +-min_go=go1.18 + -- go.mod -- module mod.test @@ -32,8 +35,11 @@ type EI interface { N() //@loc(NDef, "N") } +type T[P any] struct{ f P } + type Error error + -- b/b.go -- package b @@ -43,6 +49,8 @@ type B a.A type BI a.AI +type T a.T[int] // must not panic + -- c/c.go -- package c diff --git a/internal/gcimporter/iexport.go b/internal/gcimporter/iexport.go index da2fa7a1bb9..6103dd7102b 100644 --- a/internal/gcimporter/iexport.go +++ b/internal/gcimporter/iexport.go @@ -46,7 +46,7 @@ func IExportShallow(fset *token.FileSet, pkg *types.Package, reportf ReportFunc) // TODO(adonovan): use byte slices throughout, avoiding copying. const bundle, shallow = false, true var out bytes.Buffer - err := iexportCommon(&out, fset, bundle, shallow, iexportVersion, []*types.Package{pkg}, reportf) + err := iexportCommon(&out, fset, bundle, shallow, iexportVersion, []*types.Package{pkg}) return out.Bytes(), err } @@ -86,16 +86,16 @@ const bundleVersion = 0 // so that calls to IImportData can override with a provided package path. func IExportData(out io.Writer, fset *token.FileSet, pkg *types.Package) error { const bundle, shallow = false, false - return iexportCommon(out, fset, bundle, shallow, iexportVersion, []*types.Package{pkg}, nil) + return iexportCommon(out, fset, bundle, shallow, iexportVersion, []*types.Package{pkg}) } // IExportBundle writes an indexed export bundle for pkgs to out. func IExportBundle(out io.Writer, fset *token.FileSet, pkgs []*types.Package) error { const bundle, shallow = true, false - return iexportCommon(out, fset, bundle, shallow, iexportVersion, pkgs, nil) + return iexportCommon(out, fset, bundle, shallow, iexportVersion, pkgs) } -func iexportCommon(out io.Writer, fset *token.FileSet, bundle, shallow bool, version int, pkgs []*types.Package, reportf ReportFunc) (err error) { +func iexportCommon(out io.Writer, fset *token.FileSet, bundle, shallow bool, version int, pkgs []*types.Package) (err error) { if !debug { defer func() { if e := recover(); e != nil { @@ -113,7 +113,6 @@ func iexportCommon(out io.Writer, fset *token.FileSet, bundle, shallow bool, ver fset: fset, version: version, shallow: shallow, - reportf: reportf, allPkgs: map[*types.Package]bool{}, stringIndex: map[string]uint64{}, declIndex: map[types.Object]uint64{}, @@ -330,7 +329,6 @@ type iexporter struct { shallow bool // don't put types from other packages in the index objEncoder *objectpath.Encoder // encodes objects from other packages in shallow mode; lazily allocated - reportf ReportFunc // if non-nil, used to report bugs localpkg *types.Package // (nil in bundle mode) // allPkgs tracks all packages that have been referenced by @@ -917,22 +915,25 @@ func (w *exportWriter) objectPath(obj types.Object) { objectPath, err := w.p.objectpathEncoder().For(obj) if err != nil { // Fall back to the empty string, which will cause the importer to create a - // new object. + // new object, which matches earlier behavior. Creating a new object is + // sufficient for many purposes (such as type checking), but causes certain + // references algorithms to fail (golang/go#60819). However, we didn't + // notice this problem during months of gopls@v0.12.0 testing. // - // This is incorrect in shallow mode (golang/go#60819), but matches - // the previous behavior. This code is defensive, as it is hard to - // prove that the objectpath algorithm will succeed in all cases, and - // creating a new object sort of works. - // (we didn't notice the bug during months of gopls@v0.12.0 testing) + // TODO(golang/go#61674): this workaround is insufficient, as in the case + // where the field forwarded from an instantiated type that may not appear + // in the export data of the original package: // - // However, report a bug so that we can eventually have confidence - // that export/import is producing a correct package. + // // package a + // type A[P any] struct{ F P } // - // TODO: remove reportf once we have such confidence. + // // package b + // type B a.A[int] + // + // We need to update references algorithms not to depend on this + // de-duplication, at which point we may want to simply remove the + // workaround here. w.string("") - if w.p.reportf != nil { - w.p.reportf("unable to encode object %q in package %q: %v", obj.Name(), obj.Pkg().Path(), err) - } return } w.string(string(objectPath)) diff --git a/internal/gcimporter/iexport_test.go b/internal/gcimporter/iexport_test.go index fd4c200f1db..7f77a796077 100644 --- a/internal/gcimporter/iexport_test.go +++ b/internal/gcimporter/iexport_test.go @@ -61,7 +61,7 @@ func readExportFile(filename string) ([]byte, error) { func iexport(fset *token.FileSet, version int, pkg *types.Package) ([]byte, error) { var buf bytes.Buffer const bundle, shallow = false, false - if err := gcimporter.IExportCommon(&buf, fset, bundle, shallow, version, []*types.Package{pkg}, nil); err != nil { + if err := gcimporter.IExportCommon(&buf, fset, bundle, shallow, version, []*types.Package{pkg}); err != nil { return nil, err } return buf.Bytes(), nil