internal/gcimporter: treat unknown constant values the same as invalid

Fixes a crash resulting from trying to convert an unknown constant
value.

Fixes golang/go#60605

Change-Id: If6b831b8fe2f9690b9f89e191b329eb7660f5e14
Reviewed-on: https://go-review.googlesource.com/c/tools/+/501209
TryBot-Bypass: Robert Findley <[email protected]>
gopls-CI: kokoro <[email protected]>
Run-TryBot: Robert Findley <[email protected]>
Reviewed-by: Robert Griesemer <[email protected]>

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")

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)
+			}
+		})
 	}
 }
 

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))