Skip to content

Commit

Permalink
go/types, types2: eagerly check that constraints are not type params
Browse files Browse the repository at this point in the history
As a result of the change to the underlying of a type parameter to be
its constraint interface, we had couple inaccuracies that combined to
cause an infinite recursion when type checking the invalid type
parameter list [A A].
 - We deferred tpar.iface() using check.later twice: once in
   newTypeParam, and then again at the end of collectTypeParams.
 - We deferred the check that type parameter constraints are not type
   parameters, even though this is unnecessary: the constraint type is
   known.

With these inaccuracies, tpar.iface() was executing before our guard
against using type parameters as constraints, causing an infinite
recursion through under().

Fix this by eagerly checking whether the constraint is a type
parameter, and marking it invalid if so. Also remove the unnecessary
calls to tpar.iface() at the end of collectTypeParams, as this will
already have been scheduled by newTypeParam.

Fixes golang#50321

Change-Id: I4eecbecf21656615867cb94be65b520e9e795bd1
Reviewed-on: https://go-review.googlesource.com/c/go/+/374294
Reviewed-by: Robert Griesemer <gri@golang.org>
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
  • Loading branch information
findleyr authored and jproberts committed Jun 21, 2022
1 parent b7bebd3 commit 067eb20
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 32 deletions.
19 changes: 5 additions & 14 deletions src/cmd/compile/internal/types2/decl.go
Original file line number Diff line number Diff line change
Expand Up @@ -657,33 +657,24 @@ func (check *Checker) collectTypeParams(dst **TypeParamList, list []*syntax.Fiel
// Keep track of bounds for later validation.
var bound Type
var bounds []Type
var posers []poser
for i, f := range list {
// Optimization: Re-use the previous type bound if it hasn't changed.
// This also preserves the grouped output of type parameter lists
// when printing type strings.
if i == 0 || f.Type != list[i-1].Type {
bound = check.bound(f.Type)
bounds = append(bounds, bound)
posers = append(posers, f.Type)
}
tparams[i].bound = bound
}

check.later(func() {
for i, bound := range bounds {
if isTypeParam(bound) {
// We may be able to allow this since it is now well-defined what
// the underlying type and thus type set of a type parameter is.
// But we may need some additional form of cycle detection within
// type parameter lists.
check.error(posers[i], "cannot use a type parameter as constraint")
check.error(f.Type, "cannot use a type parameter as constraint")
bound = Typ[Invalid]
}
bounds = append(bounds, bound)
}
for _, tpar := range tparams {
tpar.iface() // compute type set
}
})
tparams[i].bound = bound
}
}

func (check *Checker) bound(x syntax.Expr) Type {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// Copyright 2021 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 p

func Ln[A A /* ERROR cannot use a type parameter as constraint */ ](p A) {
}
31 changes: 13 additions & 18 deletions src/go/types/decl.go
Original file line number Diff line number Diff line change
Expand Up @@ -708,34 +708,29 @@ func (check *Checker) collectTypeParams(dst **TypeParamList, list *ast.FieldList

index := 0
var bounds []Type
var posns []positioner // bound positions
for _, f := range list.List {
// TODO(rfindley) we should be able to rely on f.Type != nil at this point
var bound Type
// NOTE: we may be able to assert that f.Type != nil here, but this is not
// an invariant of the AST, so we are cautious.
if f.Type != nil {
bound := check.bound(f.Type)
bounds = append(bounds, bound)
posns = append(posns, f.Type)
for i := range f.Names {
tparams[index+i].bound = bound
}
}
index += len(f.Names)
}

check.later(func() {
for i, bound := range bounds {
bound = check.bound(f.Type)
if isTypeParam(bound) {
// We may be able to allow this since it is now well-defined what
// the underlying type and thus type set of a type parameter is.
// But we may need some additional form of cycle detection within
// type parameter lists.
check.error(posns[i], _MisplacedTypeParam, "cannot use a type parameter as constraint")
check.error(f.Type, _MisplacedTypeParam, "cannot use a type parameter as constraint")
bound = Typ[Invalid]
}
} else {
bound = Typ[Invalid]
}
for _, tpar := range tparams {
tpar.iface() // compute type set
bounds = append(bounds, bound)
for i := range f.Names {
tparams[index+i].bound = bound
}
})
index += len(f.Names)
}
}

func (check *Checker) bound(x ast.Expr) Type {
Expand Down
8 changes: 8 additions & 0 deletions src/go/types/testdata/fixedbugs/issue50321.go2
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// Copyright 2021 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 p

func Ln[A A /* ERROR cannot use a type parameter as constraint */ ](p A) {
}

0 comments on commit 067eb20

Please sign in to comment.