]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: fix importers to deal with recursion through type constraints
authorDan Scales <danscales@google.com>
Thu, 17 Feb 2022 04:11:52 +0000 (20:11 -0800)
committerDan Scales <danscales@google.com>
Fri, 18 Feb 2022 16:56:42 +0000 (16:56 +0000)
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 <mdempsky@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

src/cmd/compile/internal/importer/iimport.go
src/cmd/compile/internal/typecheck/iimport.go
src/go/internal/gcimporter/iimport.go
test/typeparam/issue51219.dir/a.go [new file with mode: 0644]
test/typeparam/issue51219.dir/b.go [new file with mode: 0644]
test/typeparam/issue51219.dir/main.go [new file with mode: 0644]
test/typeparam/issue51219.go [new file with mode: 0644]
test/typeparam/issue51219.out [new file with mode: 0644]

index a827987a48701bf968b63cb569fa5f384dab5d08..bed4fbb0160d72918793106b5c56ef7b17a82b1d 100644 (file)
@@ -180,6 +180,14 @@ func ImportData(imports map[string]*types2.Package, data, path string) (pkg *typ
                p.doDecl(localpkg, name)
        }
 
+       // SetConstraint can't be called if the constraint type is not yet complete.
+       // When type params are created in the 'P' case of (*importReader).obj(),
+       // the associated constraint type may not be complete due to recursion.
+       // Therefore, we defer calling SetConstraint there, and call it here instead
+       // after all types are complete.
+       for _, d := range p.later {
+               d.t.SetConstraint(d.constraint)
+       }
        // record all referenced packages as imports
        list := append(([]*types2.Package)(nil), pkgList[1:]...)
        sort.Sort(byPath(list))
@@ -191,6 +199,11 @@ func ImportData(imports map[string]*types2.Package, data, path string) (pkg *typ
        return localpkg, nil
 }
 
+type setConstraintArgs struct {
+       t          *types2.TypeParam
+       constraint types2.Type
+}
+
 type iimporter struct {
        exportVersion int64
        ipath         string
@@ -206,6 +219,9 @@ type iimporter struct {
        tparamIndex map[ident]*types2.TypeParam
 
        interfaceList []*types2.Interface
+
+       // Arguments for calls to SetConstraint that are deferred due to recursive types
+       later []setConstraintArgs
 }
 
 func (p *iimporter) doDecl(pkg *types2.Package, name string) {
@@ -401,7 +417,11 @@ func (r *importReader) obj(name string) {
                        }
                        iface.MarkImplicit()
                }
-               t.SetConstraint(constraint)
+               // The constraint type may not be complete, if we
+               // are in the middle of a type recursion involving type
+               // constraints. So, we defer SetConstraint until we have
+               // completely set up all types in ImportData.
+               r.p.later = append(r.p.later, setConstraintArgs{t: t, constraint: constraint})
 
        case 'V':
                typ := r.typ()
index bc34d3933a96835718903b4c43aaa2c6b64ced85..ef91f550a5a1fe3460516a5ec70c69649e012966 100644 (file)
@@ -354,15 +354,18 @@ func (r *importReader) doDecl(sym *types.Sym) *ir.Name {
                // declaration before recursing.
                n := importtype(pos, sym)
                t := n.Type()
+
+               // Because of recursion, we need to defer width calculations and
+               // instantiations on intermediate types until the top-level type is
+               // fully constructed. Note that we can have recursion via type
+               // constraints.
+               types.DeferCheckSize()
+               deferDoInst()
                if tag == 'U' {
                        rparams := r.typeList()
                        t.SetRParams(rparams)
                }
 
-               // We also need to defer width calculations until
-               // after the underlying type has been assigned.
-               types.DeferCheckSize()
-               deferDoInst()
                underlying := r.typ()
                t.SetUnderlying(underlying)
 
index 8ec4c5413ba4bbaf3c38a787b6697fc80c657511..bff1c09cc9714dd017356a91257d726cded21129 100644 (file)
@@ -181,6 +181,15 @@ func iImportData(fset *token.FileSet, imports map[string]*types.Package, dataRea
                p.doDecl(localpkg, name)
        }
 
+       // SetConstraint can't be called if the constraint type is not yet complete.
+       // When type params are created in the 'P' case of (*importReader).obj(),
+       // the associated constraint type may not be complete due to recursion.
+       // Therefore, we defer calling SetConstraint there, and call it here instead
+       // after all types are complete.
+       for _, d := range p.later {
+               d.t.SetConstraint(d.constraint)
+       }
+
        for _, typ := range p.interfaceList {
                typ.Complete()
        }
@@ -195,6 +204,11 @@ func iImportData(fset *token.FileSet, imports map[string]*types.Package, dataRea
        return localpkg, nil
 }
 
+type setConstraintArgs struct {
+       t          *types.TypeParam
+       constraint types.Type
+}
+
 type iimporter struct {
        exportVersion int64
        ipath         string
@@ -211,6 +225,9 @@ type iimporter struct {
 
        fake          fakeFileSet
        interfaceList []*types.Interface
+
+       // Arguments for calls to SetConstraint that are deferred due to recursive types
+       later []setConstraintArgs
 }
 
 func (p *iimporter) doDecl(pkg *types.Package, name string) {
@@ -391,7 +408,11 @@ func (r *importReader) obj(name string) {
                        }
                        iface.MarkImplicit()
                }
-               t.SetConstraint(constraint)
+               // The constraint type may not be complete, if we
+               // are in the middle of a type recursion involving type
+               // constraints. So, we defer SetConstraint until we have
+               // completely set up all types in ImportData.
+               r.p.later = append(r.p.later, setConstraintArgs{t: t, constraint: constraint})
 
        case 'V':
                typ := r.typ()
diff --git a/test/typeparam/issue51219.dir/a.go b/test/typeparam/issue51219.dir/a.go
new file mode 100644 (file)
index 0000000..3ed4322
--- /dev/null
@@ -0,0 +1,59 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package a
+
+// Type I is the first basic test for the issue, which relates to a type that is recursive
+// via a type constraint.  (In this test, I -> IConstraint -> MyStruct -> I.)
+type JsonRaw []byte
+
+type MyStruct struct {
+       x *I[JsonRaw]
+}
+
+type IConstraint interface {
+       JsonRaw | MyStruct
+}
+
+type I[T IConstraint] struct {
+}
+
+// The following types form an even more complex recursion (through two type
+// constraints), and model the actual types in the issue (#51219) more closely.
+// However, they don't reveal any new issue. But it seems useful to leave this
+// complex set of types in a test in case it might be broken by future changes.
+
+type Message struct {
+       Interaction *Interaction[JsonRaw] `json:"interaction,omitempty"`
+}
+
+type ResolvedDataConstraint interface {
+       User | Message
+}
+
+type Snowflake uint64
+
+type ResolvedData[T ResolvedDataConstraint] map[Snowflake]T
+
+type User struct {
+}
+
+type Resolved struct {
+       Users ResolvedData[User] `json:"users,omitempty"`
+}
+
+type resolvedInteractionWithOptions struct {
+       Resolved Resolved `json:"resolved,omitempty"`
+}
+
+type UserCommandInteractionData struct {
+       resolvedInteractionWithOptions
+}
+
+type InteractionDataConstraint interface {
+       JsonRaw | UserCommandInteractionData
+}
+
+type Interaction[DataT InteractionDataConstraint] struct {
+}
diff --git a/test/typeparam/issue51219.dir/b.go b/test/typeparam/issue51219.dir/b.go
new file mode 100644 (file)
index 0000000..c159072
--- /dev/null
@@ -0,0 +1,11 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package b
+
+import "a"
+
+type InteractionRequest[T a.InteractionDataConstraint] struct {
+       a.Interaction[T]
+}
diff --git a/test/typeparam/issue51219.dir/main.go b/test/typeparam/issue51219.dir/main.go
new file mode 100644 (file)
index 0000000..c5cffd1
--- /dev/null
@@ -0,0 +1,18 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+import (
+       "a"
+       "b"
+       "fmt"
+)
+
+func main() {
+       var x a.I[a.JsonRaw]
+       var y b.InteractionRequest[a.JsonRaw]
+
+       fmt.Printf("%v %v\n", x, y)
+}
diff --git a/test/typeparam/issue51219.go b/test/typeparam/issue51219.go
new file mode 100644 (file)
index 0000000..642f4bf
--- /dev/null
@@ -0,0 +1,7 @@
+// rundir -G=3
+
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package ignored
diff --git a/test/typeparam/issue51219.out b/test/typeparam/issue51219.out
new file mode 100644 (file)
index 0000000..99c5b9a
--- /dev/null
@@ -0,0 +1 @@
+{} {{}}