Skip to content

Commit d35ed09

Browse files
committed
cmd/compile: fix importers to deal with recursion through type constraints
The code for issue #51219 reveals bugs in the types1 and types2 importers that can occur for recursive types that are recursive through the type constraint. The crash in the issue is caused by the types1 bug, which leads to the production of a type1 type which is incomplete and improperly has the HasTParam flag set. The bug in the types1 importer is that we were not deferring type instantiations when reading the type parameters, but we need to do that exactly to correctly handle recursion through the type constraint. So, the fix is to move the start of the deferrals (in the 'U' section of doDecl in typecheck/iimport.go) above the code that reads the type params. Once that bug is fixed, the test still crashes due to a related types2 importer issues. The problem is that t.SetConstraint(c) requires c to be fully constructed (have its underlying type set). Since that may not be done yet in the 'U' case in (*importReader).obj() in importer/iimport.go, we need to defer the call to SetConstraint() in that case, until we are done importing all the types. I added a test case with recursion through a type constraint that causes a problem that is fixed by the types1 importer change, though the error is not the same as in the issue. I added more types in the test case (which try to imitate the issue types more closely) the types2 bug, but wasn't able to cause it yet with the smaller test case. Fixes #51219 Change-Id: I85d860c98c09dddc37f76ce87a78a6015ec6fd20 Reviewed-on: https://go-review.googlesource.com/c/go/+/386335 Reviewed-by: Matthew Dempsky <[email protected]> Reviewed-by: Robert Findley <[email protected]> Trust: Dan Scales <[email protected]> Run-TryBot: Dan Scales <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
1 parent eaf0405 commit d35ed09

File tree

8 files changed

+146
-6
lines changed

8 files changed

+146
-6
lines changed

src/cmd/compile/internal/importer/iimport.go

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,14 @@ func ImportData(imports map[string]*types2.Package, data, path string) (pkg *typ
180180
p.doDecl(localpkg, name)
181181
}
182182

183+
// SetConstraint can't be called if the constraint type is not yet complete.
184+
// When type params are created in the 'P' case of (*importReader).obj(),
185+
// the associated constraint type may not be complete due to recursion.
186+
// Therefore, we defer calling SetConstraint there, and call it here instead
187+
// after all types are complete.
188+
for _, d := range p.later {
189+
d.t.SetConstraint(d.constraint)
190+
}
183191
// record all referenced packages as imports
184192
list := append(([]*types2.Package)(nil), pkgList[1:]...)
185193
sort.Sort(byPath(list))
@@ -191,6 +199,11 @@ func ImportData(imports map[string]*types2.Package, data, path string) (pkg *typ
191199
return localpkg, nil
192200
}
193201

202+
type setConstraintArgs struct {
203+
t *types2.TypeParam
204+
constraint types2.Type
205+
}
206+
194207
type iimporter struct {
195208
exportVersion int64
196209
ipath string
@@ -206,6 +219,9 @@ type iimporter struct {
206219
tparamIndex map[ident]*types2.TypeParam
207220

208221
interfaceList []*types2.Interface
222+
223+
// Arguments for calls to SetConstraint that are deferred due to recursive types
224+
later []setConstraintArgs
209225
}
210226

211227
func (p *iimporter) doDecl(pkg *types2.Package, name string) {
@@ -401,7 +417,11 @@ func (r *importReader) obj(name string) {
401417
}
402418
iface.MarkImplicit()
403419
}
404-
t.SetConstraint(constraint)
420+
// The constraint type may not be complete, if we
421+
// are in the middle of a type recursion involving type
422+
// constraints. So, we defer SetConstraint until we have
423+
// completely set up all types in ImportData.
424+
r.p.later = append(r.p.later, setConstraintArgs{t: t, constraint: constraint})
405425

406426
case 'V':
407427
typ := r.typ()

src/cmd/compile/internal/typecheck/iimport.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -354,15 +354,18 @@ func (r *importReader) doDecl(sym *types.Sym) *ir.Name {
354354
// declaration before recursing.
355355
n := importtype(pos, sym)
356356
t := n.Type()
357+
358+
// Because of recursion, we need to defer width calculations and
359+
// instantiations on intermediate types until the top-level type is
360+
// fully constructed. Note that we can have recursion via type
361+
// constraints.
362+
types.DeferCheckSize()
363+
deferDoInst()
357364
if tag == 'U' {
358365
rparams := r.typeList()
359366
t.SetRParams(rparams)
360367
}
361368

362-
// We also need to defer width calculations until
363-
// after the underlying type has been assigned.
364-
types.DeferCheckSize()
365-
deferDoInst()
366369
underlying := r.typ()
367370
t.SetUnderlying(underlying)
368371

src/go/internal/gcimporter/iimport.go

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,15 @@ func iImportData(fset *token.FileSet, imports map[string]*types.Package, dataRea
181181
p.doDecl(localpkg, name)
182182
}
183183

184+
// SetConstraint can't be called if the constraint type is not yet complete.
185+
// When type params are created in the 'P' case of (*importReader).obj(),
186+
// the associated constraint type may not be complete due to recursion.
187+
// Therefore, we defer calling SetConstraint there, and call it here instead
188+
// after all types are complete.
189+
for _, d := range p.later {
190+
d.t.SetConstraint(d.constraint)
191+
}
192+
184193
for _, typ := range p.interfaceList {
185194
typ.Complete()
186195
}
@@ -195,6 +204,11 @@ func iImportData(fset *token.FileSet, imports map[string]*types.Package, dataRea
195204
return localpkg, nil
196205
}
197206

207+
type setConstraintArgs struct {
208+
t *types.TypeParam
209+
constraint types.Type
210+
}
211+
198212
type iimporter struct {
199213
exportVersion int64
200214
ipath string
@@ -211,6 +225,9 @@ type iimporter struct {
211225

212226
fake fakeFileSet
213227
interfaceList []*types.Interface
228+
229+
// Arguments for calls to SetConstraint that are deferred due to recursive types
230+
later []setConstraintArgs
214231
}
215232

216233
func (p *iimporter) doDecl(pkg *types.Package, name string) {
@@ -391,7 +408,11 @@ func (r *importReader) obj(name string) {
391408
}
392409
iface.MarkImplicit()
393410
}
394-
t.SetConstraint(constraint)
411+
// The constraint type may not be complete, if we
412+
// are in the middle of a type recursion involving type
413+
// constraints. So, we defer SetConstraint until we have
414+
// completely set up all types in ImportData.
415+
r.p.later = append(r.p.later, setConstraintArgs{t: t, constraint: constraint})
395416

396417
case 'V':
397418
typ := r.typ()

test/typeparam/issue51219.dir/a.go

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// Copyright 2022 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package a
6+
7+
// Type I is the first basic test for the issue, which relates to a type that is recursive
8+
// via a type constraint. (In this test, I -> IConstraint -> MyStruct -> I.)
9+
type JsonRaw []byte
10+
11+
type MyStruct struct {
12+
x *I[JsonRaw]
13+
}
14+
15+
type IConstraint interface {
16+
JsonRaw | MyStruct
17+
}
18+
19+
type I[T IConstraint] struct {
20+
}
21+
22+
// The following types form an even more complex recursion (through two type
23+
// constraints), and model the actual types in the issue (#51219) more closely.
24+
// However, they don't reveal any new issue. But it seems useful to leave this
25+
// complex set of types in a test in case it might be broken by future changes.
26+
27+
type Message struct {
28+
Interaction *Interaction[JsonRaw] `json:"interaction,omitempty"`
29+
}
30+
31+
type ResolvedDataConstraint interface {
32+
User | Message
33+
}
34+
35+
type Snowflake uint64
36+
37+
type ResolvedData[T ResolvedDataConstraint] map[Snowflake]T
38+
39+
type User struct {
40+
}
41+
42+
type Resolved struct {
43+
Users ResolvedData[User] `json:"users,omitempty"`
44+
}
45+
46+
type resolvedInteractionWithOptions struct {
47+
Resolved Resolved `json:"resolved,omitempty"`
48+
}
49+
50+
type UserCommandInteractionData struct {
51+
resolvedInteractionWithOptions
52+
}
53+
54+
type InteractionDataConstraint interface {
55+
JsonRaw | UserCommandInteractionData
56+
}
57+
58+
type Interaction[DataT InteractionDataConstraint] struct {
59+
}

test/typeparam/issue51219.dir/b.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// Copyright 2022 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package b
6+
7+
import "a"
8+
9+
type InteractionRequest[T a.InteractionDataConstraint] struct {
10+
a.Interaction[T]
11+
}

test/typeparam/issue51219.dir/main.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Copyright 2022 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package main
6+
7+
import (
8+
"a"
9+
"b"
10+
"fmt"
11+
)
12+
13+
func main() {
14+
var x a.I[a.JsonRaw]
15+
var y b.InteractionRequest[a.JsonRaw]
16+
17+
fmt.Printf("%v %v\n", x, y)
18+
}

test/typeparam/issue51219.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
// rundir -G=3
2+
3+
// Copyright 2022 The Go Authors. All rights reserved.
4+
// Use of this source code is governed by a BSD-style
5+
// license that can be found in the LICENSE file.
6+
7+
package ignored

test/typeparam/issue51219.out

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{} {{}}

0 commit comments

Comments
 (0)