From 787708a6ff66092678cd4312358e90a5085eac89 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Fri, 12 Nov 2021 08:59:49 -0800 Subject: [PATCH] cmd/compile/internal/types2: remove tparamIsIface flag and corresponding dead code Added/clarified some comments. Change-Id: Ib08d3343ff08c23cc8880a27a0148d1ff077a80f Reviewed-on: https://go-review.googlesource.com/c/go/+/363654 Trust: Robert Griesemer Run-TryBot: Robert Griesemer TryBot-Result: Go Bot Reviewed-by: Robert Findley --- src/cmd/compile/internal/types2/builtins.go | 27 +----- src/cmd/compile/internal/types2/decl.go | 4 + src/cmd/compile/internal/types2/expr.go | 15 +--- src/cmd/compile/internal/types2/index.go | 89 +------------------ src/cmd/compile/internal/types2/predicates.go | 18 +--- src/cmd/compile/internal/types2/sizes.go | 7 +- src/cmd/compile/internal/types2/struct.go | 5 +- src/cmd/compile/internal/types2/type.go | 4 +- src/cmd/compile/internal/types2/typeparam.go | 17 +--- 9 files changed, 24 insertions(+), 162 deletions(-) diff --git a/src/cmd/compile/internal/types2/builtins.go b/src/cmd/compile/internal/types2/builtins.go index c4b897e80f..5b4ffd0dad 100644 --- a/src/cmd/compile/internal/types2/builtins.go +++ b/src/cmd/compile/internal/types2/builtins.go @@ -179,28 +179,10 @@ func (check *Checker) builtin(x *operand, call *syntax.CallExpr, id builtinId) ( } case *Interface: - if tparamIsIface && isTypeParam(x.typ) { - if t.typeSet().underIs(func(t Type) bool { - switch t := arrayPtrDeref(t).(type) { - case *Basic: - if isString(t) && id == _Len { - return true - } - case *Array, *Slice, *Chan: - return true - case *Map: - if id == _Len { - return true - } - } - return false - }) { - mode = value - } + if !isTypeParam(x.typ) { + break } - case *TypeParam: - assert(!tparamIsIface) - if t.underIs(func(t Type) bool { + if t.typeSet().underIs(func(t Type) bool { switch t := arrayPtrDeref(t).(type) { case *Basic: if isString(t) && id == _Len { @@ -820,9 +802,6 @@ func hasVarSize(t Type) bool { } case *Interface: return isTypeParam(t) - case *TypeParam: - assert(!tparamIsIface) - return true case *Named, *Union: unreachable() } diff --git a/src/cmd/compile/internal/types2/decl.go b/src/cmd/compile/internal/types2/decl.go index d58fac5dbb..739fc163de 100644 --- a/src/cmd/compile/internal/types2/decl.go +++ b/src/cmd/compile/internal/types2/decl.go @@ -673,6 +673,10 @@ func (check *Checker) collectTypeParams(dst **TypeParamList, list []*syntax.Fiel 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") } } diff --git a/src/cmd/compile/internal/types2/expr.go b/src/cmd/compile/internal/types2/expr.go index 6faa54475b..b700716b0c 100644 --- a/src/cmd/compile/internal/types2/expr.go +++ b/src/cmd/compile/internal/types2/expr.go @@ -659,7 +659,7 @@ func (check *Checker) convertUntyped(x *operand, target Type) { newType, val, code := check.implicitTypeAndValue(x, target) if code != 0 { t := target - if !tparamIsIface || !isTypeParam(target) { + if !isTypeParam(target) { t = safeUnderlying(target) } check.invalidConversion(code, x, t) @@ -741,19 +741,8 @@ func (check *Checker) implicitTypeAndValue(x *operand, target Type) (Type, const default: return nil, nil, _InvalidUntypedConversion } - case *TypeParam: - assert(!tparamIsIface) - if !u.underIs(func(u Type) bool { - if u == nil { - return false - } - t, _, _ := check.implicitTypeAndValue(x, u) - return t != nil - }) { - return nil, nil, _InvalidUntypedConversion - } case *Interface: - if tparamIsIface && isTypeParam(target) { + if isTypeParam(target) { if !u.typeSet().underIs(func(u Type) bool { if u == nil { return false diff --git a/src/cmd/compile/internal/types2/index.go b/src/cmd/compile/internal/types2/index.go index 97d153dfe4..648c7abe6f 100644 --- a/src/cmd/compile/internal/types2/index.go +++ b/src/cmd/compile/internal/types2/index.go @@ -100,97 +100,14 @@ func (check *Checker) indexExpr(x *operand, e *syntax.IndexExpr) (isFuncInst boo return false case *Interface: - // Note: The body of this 'if' statement is the same as the body - // of the case for type parameters below. If we keep both - // these branches we should factor out the code. - if tparamIsIface && isTypeParam(x.typ) { - // TODO(gri) report detailed failure cause for better error messages - var key, elem Type // key != nil: we must have all maps - mode := variable // non-maps result mode - // TODO(gri) factor out closure and use it for non-typeparam cases as well - if typ.typeSet().underIs(func(u Type) bool { - l := int64(-1) // valid if >= 0 - var k, e Type // k is only set for maps - switch t := u.(type) { - case *Basic: - if isString(t) { - e = universeByte - mode = value - } - case *Array: - l = t.len - e = t.elem - if x.mode != variable { - mode = value - } - case *Pointer: - if t, _ := under(t.base).(*Array); t != nil { - l = t.len - e = t.elem - } - case *Slice: - e = t.elem - case *Map: - k = t.key - e = t.elem - } - if e == nil { - return false - } - if elem == nil { - // first type - length = l - key, elem = k, e - return true - } - // all map keys must be identical (incl. all nil) - // (that is, we cannot mix maps with other types) - if !Identical(key, k) { - return false - } - // all element types must be identical - if !Identical(elem, e) { - return false - } - // track the minimal length for arrays, if any - if l >= 0 && l < length { - length = l - } - return true - }) { - // For maps, the index expression must be assignable to the map key type. - if key != nil { - index := check.singleIndex(e) - if index == nil { - x.mode = invalid - return false - } - var k operand - check.expr(&k, index) - check.assignment(&k, key, "map index") - // ok to continue even if indexing failed - map element type is known - x.mode = mapindex - x.typ = elem - x.expr = e - return false - } - - // no maps - valid = true - x.mode = mode - x.typ = elem - } + if !isTypeParam(x.typ) { + break } - case *TypeParam: - // Note: The body of this case is the same as the body of the 'if' - // statement in the interface case above. If we keep both - // these branches we should factor out the code. // TODO(gri) report detailed failure cause for better error messages - assert(!tparamIsIface) var key, elem Type // key != nil: we must have all maps mode := variable // non-maps result mode // TODO(gri) factor out closure and use it for non-typeparam cases as well - if typ.underIs(func(u Type) bool { + if typ.typeSet().underIs(func(u Type) bool { l := int64(-1) // valid if >= 0 var k, e Type // k is only set for maps switch t := u.(type) { diff --git a/src/cmd/compile/internal/types2/predicates.go b/src/cmd/compile/internal/types2/predicates.go index 62db3861ed..8ba534ce77 100644 --- a/src/cmd/compile/internal/types2/predicates.go +++ b/src/cmd/compile/internal/types2/predicates.go @@ -131,13 +131,7 @@ func comparable(T Type, seen map[Type]bool) bool { case *Array: return comparable(t.elem, seen) case *Interface: - if tparamIsIface && isTypeParam(T) { - return t.IsComparable() - } - return true - case *TypeParam: - assert(!tparamIsIface) - return t.iface().IsComparable() + return !isTypeParam(T) || t.IsComparable() } return false } @@ -150,15 +144,7 @@ func hasNil(t Type) bool { case *Slice, *Pointer, *Signature, *Map, *Chan: return true case *Interface: - if tparamIsIface && isTypeParam(t) { - return u.typeSet().underIs(func(u Type) bool { - return u != nil && hasNil(u) - }) - } - return true - case *TypeParam: - assert(!tparamIsIface) - return u.underIs(func(u Type) bool { + return !isTypeParam(t) || u.typeSet().underIs(func(u Type) bool { return u != nil && hasNil(u) }) } diff --git a/src/cmd/compile/internal/types2/sizes.go b/src/cmd/compile/internal/types2/sizes.go index b23cec435d..6f981964be 100644 --- a/src/cmd/compile/internal/types2/sizes.go +++ b/src/cmd/compile/internal/types2/sizes.go @@ -67,7 +67,9 @@ func (s *StdSizes) Alignof(T Type) int64 { case *Slice, *Interface: // Multiword data structures are effectively structs // in which each element has size WordSize. - assert(!tparamIsIface || !isTypeParam(T)) + // Type parameters lead to variable sizes/alignments; + // StdSizes.Alignof won't be called for them. + assert(!isTypeParam(T)) return s.WordSize case *Basic: // Strings are like slices and interfaces. @@ -152,6 +154,9 @@ func (s *StdSizes) Sizeof(T Type) int64 { offsets := s.Offsetsof(t.fields) return offsets[n-1] + s.Sizeof(t.fields[n-1].typ) case *Interface: + // Type parameters lead to variable sizes/alignments; + // StdSizes.Sizeof won't be called for them. + assert(!isTypeParam(T)) return s.WordSize * 2 case *TypeParam, *Union: unreachable() diff --git a/src/cmd/compile/internal/types2/struct.go b/src/cmd/compile/internal/types2/struct.go index 3e271039d1..31a3b1af5b 100644 --- a/src/cmd/compile/internal/types2/struct.go +++ b/src/cmd/compile/internal/types2/struct.go @@ -156,11 +156,8 @@ func (check *Checker) structType(styp *Struct, e *syntax.StructType) { } case *Pointer: check.error(embeddedPos, "embedded field type cannot be a pointer") - case *TypeParam: - assert(!tparamIsIface) - check.error(embeddedPos, "embedded field type cannot be a (pointer to a) type parameter") case *Interface: - if tparamIsIface && isTypeParam(t) { + if isTypeParam(t) { check.error(embeddedPos, "embedded field type cannot be a (pointer to a) type parameter") break } diff --git a/src/cmd/compile/internal/types2/type.go b/src/cmd/compile/internal/types2/type.go index 3fea8d1776..7fcb196c5a 100644 --- a/src/cmd/compile/internal/types2/type.go +++ b/src/cmd/compile/internal/types2/type.go @@ -25,9 +25,7 @@ func under(t Type) Type { case *Named: return t.under() case *TypeParam: - if tparamIsIface { - return t.iface() - } + return t.iface() } return t } diff --git a/src/cmd/compile/internal/types2/typeparam.go b/src/cmd/compile/internal/types2/typeparam.go index 5499d975a1..8dd04ff408 100644 --- a/src/cmd/compile/internal/types2/typeparam.go +++ b/src/cmd/compile/internal/types2/typeparam.go @@ -6,12 +6,6 @@ package types2 import "sync/atomic" -// If set, the underlying type of a type parameter is -// is the underlying type of its type constraint, i.e., -// an interface. With that, a type parameter satisfies -// isInterface. -const tparamIsIface = true - // Note: This is a uint32 rather than a uint64 because the // respective 64 bit atomic instructions are not available // on all platforms. @@ -76,10 +70,7 @@ func (t *TypeParam) SetConstraint(bound Type) { } func (t *TypeParam) Underlying() Type { - if tparamIsIface { - return t.iface() - } - return t + return t.iface() } func (t *TypeParam) String() string { return TypeString(t, nil) } @@ -102,15 +93,11 @@ func (t *TypeParam) iface() *Interface { return &emptyInterface } case *Interface: - if tparamIsIface && isTypeParam(bound) { + if isTypeParam(bound) { // error is reported in Checker.collectTypeParams return &emptyInterface } ityp = u - case *TypeParam: - assert(!tparamIsIface) - // error is reported in Checker.collectTypeParams - return &emptyInterface } // If we don't have an interface, wrap constraint into an implicit interface. -- 2.50.0