From 63c7614fd05052009c1eee061964241e4d73c6a1 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Thu, 10 Mar 2022 21:50:54 -0800 Subject: [PATCH] go/types, types2: factor out isInterface(x) && !isTypeParam(x) (cleanup) Fixes #51581. Change-Id: I3232428edd7dd86f2930af950fb5841f7394c4e5 Reviewed-on: https://go-review.googlesource.com/c/go/+/391834 Trust: Robert Griesemer Reviewed-by: Robert Findley --- src/cmd/compile/internal/types2/assignments.go | 2 +- src/cmd/compile/internal/types2/conversions.go | 2 +- src/cmd/compile/internal/types2/expr.go | 13 ++++++++++--- src/cmd/compile/internal/types2/predicates.go | 5 +++++ src/go/types/assignments.go | 2 +- src/go/types/conversions.go | 2 +- src/go/types/expr.go | 13 ++++++++++--- src/go/types/predicates.go | 5 +++++ 8 files changed, 34 insertions(+), 10 deletions(-) diff --git a/src/cmd/compile/internal/types2/assignments.go b/src/cmd/compile/internal/types2/assignments.go index d88b03748f..49f4e2d2ab 100644 --- a/src/cmd/compile/internal/types2/assignments.go +++ b/src/cmd/compile/internal/types2/assignments.go @@ -44,7 +44,7 @@ func (check *Checker) assignment(x *operand, T Type, context string) { x.mode = invalid return } - } else if T == nil || IsInterface(T) && !isTypeParam(T) { + } else if T == nil || isNonTypeParamInterface(T) { target = Default(x.typ) } newType, val, code := check.implicitTypeAndValue(x, target) diff --git a/src/cmd/compile/internal/types2/conversions.go b/src/cmd/compile/internal/types2/conversions.go index 08b3cbff29..a86645a547 100644 --- a/src/cmd/compile/internal/types2/conversions.go +++ b/src/cmd/compile/internal/types2/conversions.go @@ -105,7 +105,7 @@ func (check *Checker) conversion(x *operand, T Type) { // (See also the TODO below.) if x.typ == Typ[UntypedNil] { // ok - } else if IsInterface(T) && !isTypeParam(T) || constArg && !isConstType(T) { + } else if isNonTypeParamInterface(T) || constArg && !isConstType(T) { final = Default(x.typ) } else if x.mode == constant_ && isInteger(x.typ) && allString(T) { final = x.typ diff --git a/src/cmd/compile/internal/types2/expr.go b/src/cmd/compile/internal/types2/expr.go index 05cf1d0b33..e59f0b74ac 100644 --- a/src/cmd/compile/internal/types2/expr.go +++ b/src/cmd/compile/internal/types2/expr.go @@ -1101,7 +1101,7 @@ func (check *Checker) binary(x *operand, e syntax.Expr, lhs, rhs syntax.Expr, op // TODO(gri) make canMix more efficient - called for each binary operation canMix := func(x, y *operand) bool { - if IsInterface(x.typ) && !isTypeParam(x.typ) || IsInterface(y.typ) && !isTypeParam(y.typ) { + if isNonTypeParamInterface(x.typ) || isNonTypeParamInterface(y.typ) { return true } if allBoolean(x.typ) != allBoolean(y.typ) { @@ -1491,6 +1491,10 @@ func (check *Checker) exprInternal(x *operand, e syntax.Expr, hint Type) exprKin check.error(e, "illegal cycle in type declaration") goto Error } + // If the map key type is an interface (but not a type parameter), + // the type of a constant key must be considered when checking for + // duplicates. + keyIsInterface := isNonTypeParamInterface(utyp.key) visited := make(map[interface{}][]Type, len(e.ElemList)) for _, e := range e.ElemList { kv, _ := e.(*syntax.KeyValueExpr) @@ -1505,9 +1509,8 @@ func (check *Checker) exprInternal(x *operand, e syntax.Expr, hint Type) exprKin } if x.mode == constant_ { duplicate := false - // if the key is of interface type, the type is also significant when checking for duplicates xkey := keyVal(x.val) - if IsInterface(utyp.key) { + if keyIsInterface { for _, vtyp := range visited[xkey] { if Identical(vtyp, x.typ) { duplicate = true @@ -1714,6 +1717,10 @@ Error: } func keyVal(x constant.Value) interface{} { + // TODO(gri) This function must map 1, 1.0, and 1.0 + 0i to the same (integer) value. + // Same for 1.1 and 1.1 + 0i. + // Otherwise we won't get duplicate key errors for certain type parameter + // key types. See issue #51610. switch x.Kind() { case constant.Bool: return constant.BoolVal(x) diff --git a/src/cmd/compile/internal/types2/predicates.go b/src/cmd/compile/internal/types2/predicates.go index ba259341f6..6bce26137e 100644 --- a/src/cmd/compile/internal/types2/predicates.go +++ b/src/cmd/compile/internal/types2/predicates.go @@ -85,6 +85,11 @@ func IsInterface(t Type) bool { return ok } +// isNonTypeParamInterface reports whether t is an interface type but not a type parameter. +func isNonTypeParamInterface(t Type) bool { + return !isTypeParam(t) && IsInterface(t) +} + // isTypeParam reports whether t is a type parameter. func isTypeParam(t Type) bool { _, ok := t.(*TypeParam) diff --git a/src/go/types/assignments.go b/src/go/types/assignments.go index f5e22c2f67..101e868d82 100644 --- a/src/go/types/assignments.go +++ b/src/go/types/assignments.go @@ -38,7 +38,7 @@ func (check *Checker) assignment(x *operand, T Type, context string) { // bool, rune, int, float64, complex128 or string respectively, depending // on whether the value is a boolean, rune, integer, floating-point, // complex, or string constant." - if T == nil || IsInterface(T) && !isTypeParam(T) { + if T == nil || isNonTypeParamInterface(T) { if T == nil && x.typ == Typ[UntypedNil] { check.errorf(x, _UntypedNil, "use of untyped nil in %s", context) x.mode = invalid diff --git a/src/go/types/conversions.go b/src/go/types/conversions.go index c5a69cddf4..65691cf455 100644 --- a/src/go/types/conversions.go +++ b/src/go/types/conversions.go @@ -101,7 +101,7 @@ func (check *Checker) conversion(x *operand, T Type) { // - Keep untyped nil for untyped nil arguments. // - For constant integer to string conversions, keep the argument type. // (See also the TODO below.) - if IsInterface(T) && !isTypeParam(T) || constArg && !isConstType(T) || x.isNil() { + if isNonTypeParamInterface(T) || constArg && !isConstType(T) || x.isNil() { final = Default(x.typ) // default type of untyped nil is untyped nil } else if x.mode == constant_ && isInteger(x.typ) && allString(T) { final = x.typ diff --git a/src/go/types/expr.go b/src/go/types/expr.go index e24bd60dc3..596bcef9c1 100644 --- a/src/go/types/expr.go +++ b/src/go/types/expr.go @@ -1081,7 +1081,7 @@ func (check *Checker) binary(x *operand, e ast.Expr, lhs, rhs ast.Expr, op token // TODO(gri) make canMix more efficient - called for each binary operation canMix := func(x, y *operand) bool { - if IsInterface(x.typ) && !isTypeParam(x.typ) || IsInterface(y.typ) && !isTypeParam(y.typ) { + if isNonTypeParamInterface(x.typ) || isNonTypeParamInterface(y.typ) { return true } if allBoolean(x.typ) != allBoolean(y.typ) { @@ -1468,6 +1468,10 @@ func (check *Checker) exprInternal(x *operand, e ast.Expr, hint Type) exprKind { check.error(e, _InvalidTypeCycle, "illegal cycle in type declaration") goto Error } + // If the map key type is an interface (but not a type parameter), + // the type of a constant key must be considered when checking for + // duplicates. + keyIsInterface := isNonTypeParamInterface(utyp.key) visited := make(map[any][]Type, len(e.Elts)) for _, e := range e.Elts { kv, _ := e.(*ast.KeyValueExpr) @@ -1482,9 +1486,8 @@ func (check *Checker) exprInternal(x *operand, e ast.Expr, hint Type) exprKind { } if x.mode == constant_ { duplicate := false - // if the key is of interface type, the type is also significant when checking for duplicates xkey := keyVal(x.val) - if IsInterface(utyp.key) { + if keyIsInterface { for _, vtyp := range visited[xkey] { if Identical(vtyp, x.typ) { duplicate = true @@ -1657,6 +1660,10 @@ Error: } func keyVal(x constant.Value) any { + // TODO(gri) This function must map 1, 1.0, and 1.0 + 0i to the same (integer) value. + // Same for 1.1 and 1.1 + 0i. + // Otherwise we won't get duplicate key errors for certain type parameter + // key types. See issue #51610. switch x.Kind() { case constant.Bool: return constant.BoolVal(x) diff --git a/src/go/types/predicates.go b/src/go/types/predicates.go index 0360f27ee6..51d056f355 100644 --- a/src/go/types/predicates.go +++ b/src/go/types/predicates.go @@ -87,6 +87,11 @@ func IsInterface(t Type) bool { return ok } +// isNonTypeParamInterface reports whether t is an interface type but not a type parameter. +func isNonTypeParamInterface(t Type) bool { + return !isTypeParam(t) && IsInterface(t) +} + // isTypeParam reports whether t is a type parameter. func isTypeParam(t Type) bool { _, ok := t.(*TypeParam)