diff --git a/gopls/internal/regtest/marker/testdata/diagnostics/issue60605.txt b/gopls/internal/regtest/marker/testdata/diagnostics/issue60605.txt new file mode 100644 index 00000000000..f80857dcb99 --- /dev/null +++ b/gopls/internal/regtest/marker/testdata/diagnostics/issue60605.txt @@ -0,0 +1,12 @@ +This test verifies that we can export constants with unknown kind. +Previously, the exporter would panic while attempting to convert such constants +to their target type (float64, in this case). + +-- go.mod -- +module mod.txt/p + +go 1.20 +-- p.go -- +package p + +const EPSILON float64 = 1e- //@diag(re"1e-()", re"exponent has no digits") diff --git a/internal/gcimporter/gcimporter_test.go b/internal/gcimporter/gcimporter_test.go index 3d17e114d0d..e13cfd9219a 100644 --- a/internal/gcimporter/gcimporter_test.go +++ b/internal/gcimporter/gcimporter_test.go @@ -786,57 +786,72 @@ func TestIssue57015(t *testing.T) { } // This is a regression test for a failure to export a package -// containing a specific type error. +// containing type errors. // -// Though the issue and test are specific, they may be representatives -// of class of exporter bugs on ill-typed code that we have yet to -// flush out. +// Though the issues and tests are specific, they may be representatives of a +// class of exporter bugs on ill-typed code that we have yet to flush out. // // TODO(adonovan): systematize our search for similar problems using -// fuzz testing, and drive this test from a table of test cases -// discovered by fuzzing. -func TestIssue57729(t *testing.T) { - // The lack of a receiver causes Recv.Type=Invalid. - // (The type checker then treats Foo as a package-level - // function, inserting it into the package scope.) - // The exporter needs to apply the same treatment. - const src = `package p; func () Foo() {}` - - // Parse the ill-typed input. - fset := token.NewFileSet() - f, err := goparser.ParseFile(fset, "p.go", src, 0) - if err != nil { - t.Fatalf("parse: %v", err) - } +// fuzz testing. +func TestExportInvalid(t *testing.T) { - // Type check it, expecting errors. - config := &types.Config{ - Error: func(err error) { t.Log(err) }, // don't abort at first error - } - pkg1, _ := config.Check("p", fset, []*ast.File{f}, nil) + tests := []struct { + name string + src string + objName string + }{ + // The lack of a receiver causes Recv.Type=Invalid. + // (The type checker then treats Foo as a package-level + // function, inserting it into the package scope.) + // The exporter needs to apply the same treatment. + {"issue 57729", `package p; func () Foo() {}`, "Foo"}, - // Export it. - // (Shallowness isn't important here.) - data, err := IExportShallow(fset, pkg1) - if err != nil { - t.Fatalf("export: %v", err) // any failure to export is a bug + // It must be possible to export a constant with unknown kind, even if its + // type is known. + {"issue 60605", `package p; const EPSILON float64 = 1e-`, "EPSILON"}, } - // Re-import it. - imports := make(map[string]*types.Package) - insert := func(pkg1 *types.Package, name string) { panic("unexpected insert") } - pkg2, err := IImportShallow(fset, GetPackageFromMap(imports), data, "p", insert) - if err != nil { - t.Fatalf("import: %v", err) // any failure of IExport+IImport is a bug. - } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + // Parse the ill-typed input. + fset := token.NewFileSet() + + f, err := goparser.ParseFile(fset, "p.go", test.src, 0) + if f == nil { + // Some test cases may have parse errors, but we must always have a + // file. + t.Fatalf("ParseFile returned nil file. Err: %v", err) + } + + // Type check it, expecting errors. + config := &types.Config{ + Error: func(err error) { t.Log(err) }, // don't abort at first error + } + pkg1, _ := config.Check("p", fset, []*ast.File{f}, nil) + + // Export it. + // (Shallowness isn't important here.) + data, err := IExportShallow(fset, pkg1) + if err != nil { + t.Fatalf("export: %v", err) // any failure to export is a bug + } - // Check that Lookup("Foo") still returns something. - // We can't assert the type hasn't change: it has, - // from a method of Invalid to a standalone function. - hasObj1 := pkg1.Scope().Lookup("Foo") != nil - hasObj2 := pkg2.Scope().Lookup("Foo") != nil - if hasObj1 != hasObj2 { - t.Errorf("export+import changed Lookup('Foo')!=nil: was %t, became %t", hasObj1, hasObj2) + // Re-import it. + imports := make(map[string]*types.Package) + insert := func(pkg1 *types.Package, name string) { panic("unexpected insert") } + pkg2, err := IImportShallow(fset, GetPackageFromMap(imports), data, "p", insert) + if err != nil { + t.Fatalf("import: %v", err) // any failure of IExport+IImport is a bug. + } + + // Check that the expected object is present in both packages. + // We can't assert the type hasn't changed: it may have, in some cases. + hasObj1 := pkg1.Scope().Lookup(test.objName) != nil + hasObj2 := pkg2.Scope().Lookup(test.objName) != nil + if hasObj1 != hasObj2 { + t.Errorf("export+import changed Lookup(%q)!=nil: was %t, became %t", test.objName, hasObj1, hasObj2) + } + }) } } diff --git a/internal/gcimporter/iexport.go b/internal/gcimporter/iexport.go index 9930d8c36a7..3fc7989c083 100644 --- a/internal/gcimporter/iexport.go +++ b/internal/gcimporter/iexport.go @@ -913,6 +913,17 @@ func (w *exportWriter) value(typ types.Type, v constant.Value) { w.int64(int64(v.Kind())) } + if v.Kind() == constant.Unknown { + // golang/go#60605: treat unknown constant values as if they have invalid type + // + // This loses some fidelity over the package type-checked from source, but that + // is acceptable. + // + // TODO(rfindley): we should switch on the recorded constant kind rather + // than the constant type + return + } + switch b := typ.Underlying().(*types.Basic); b.Info() & types.IsConstType { case types.IsBoolean: w.bool(constant.BoolVal(v))