Skip to content

Commit d1a388b

Browse files
committed
internal/gcimporter: avoid a crash when exporting a struct with no pkg
This is an arbitrary fix to avoid a crash while exporting an empty struct with no ambient package. Unfortunately, the struct encoding assumes that there is an implied package for structs, which at least in invalid code may not be the case. Fixes golang/go#60891 Change-Id: I8acd591ac15cdc1770d615fdca57dcb7bb9b7651 Reviewed-on: https://go-review.googlesource.com/c/tools/+/504556 TryBot-Result: Gopher Robot <[email protected]> Run-TryBot: Robert Findley <[email protected]> gopls-CI: kokoro <[email protected]> Reviewed-by: Alan Donovan <[email protected]>
1 parent a4ed05f commit d1a388b

File tree

2 files changed

+21
-1
lines changed

2 files changed

+21
-1
lines changed

internal/gcimporter/gcimporter_test.go

+3
Original file line numberDiff line numberDiff line change
@@ -809,6 +809,9 @@ func TestExportInvalid(t *testing.T) {
809809
// It must be possible to export a constant with unknown kind, even if its
810810
// type is known.
811811
{"issue 60605", `package p; const EPSILON float64 = 1e-`, "EPSILON"},
812+
813+
// We must not crash when exporting a struct with unknown package.
814+
{"issue 60891", `package p; type I[P any] int; const C I[struct{}] = 42`, "C"},
812815
}
813816

814817
for _, test := range tests {

internal/gcimporter/iexport.go

+18-1
Original file line numberDiff line numberDiff line change
@@ -673,6 +673,9 @@ func (w *exportWriter) qualifiedType(obj *types.TypeName) {
673673
w.pkg(obj.Pkg())
674674
}
675675

676+
// TODO(rfindley): what does 'pkg' even mean here? It would be better to pass
677+
// it in explicitly into signatures and structs that may use it for
678+
// constructing fields.
676679
func (w *exportWriter) typ(t types.Type, pkg *types.Package) {
677680
w.data.uint64(w.p.typOff(t, pkg))
678681
}
@@ -773,7 +776,21 @@ func (w *exportWriter) doTyp(t types.Type, pkg *types.Package) {
773776
if n > 0 {
774777
w.setPkg(t.Field(0).Pkg(), true) // qualifying package for field objects
775778
} else {
776-
w.setPkg(pkg, true)
779+
// TODO(rfindley): improve this very hacky logic.
780+
//
781+
// The importer expects a package to be set for all struct types, even
782+
// those with no fields. A better encoding might be to set NumFields
783+
// before pkg. setPkg panics with a nil package, which may be possible
784+
// to reach with invalid packages (and perhaps valid packages, too?), so
785+
// (arbitrarily) set the localpkg if available.
786+
switch {
787+
case pkg != nil:
788+
w.setPkg(pkg, true)
789+
case w.p.shallow:
790+
w.setPkg(w.p.localpkg, true)
791+
default:
792+
panic(internalErrorf("no package to set for empty struct"))
793+
}
777794
}
778795
w.uint64(uint64(n))
779796
for i := 0; i < n; i++ {

0 commit comments

Comments
 (0)