From: Rob Findley Date: Mon, 13 Jul 2020 02:36:34 +0000 (-0400) Subject: go/types: factor out usage of implicit type X-Git-Tag: go1.16beta1~1166 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=42e09dc1ba1e820af44b2cbd4db0d60abb5559a2;p=gostls13.git go/types: factor out usage of implicit type There was some duplication of logic interpreting the implicit type of an operand in assignableTo and convertUntyped. Factor out this logic to a new 'implicitType' function, which returns the implicit type of an untyped operand when used in a context where a target type is expected. I believe this resolves some comments about code duplication. There is other similar code in assignable, assignableTo, and convertUntypes, but I found it to to be sufficiently semantically distinct to not warrant factoring out. Change-Id: I199298a2e58fcf05344318fca0226b460c57867d Reviewed-on: https://go-review.googlesource.com/c/go/+/242084 Run-TryBot: Robert Findley TryBot-Result: Gobot Gobot Reviewed-by: Robert Griesemer --- diff --git a/src/go/types/api_test.go b/src/go/types/api_test.go index 6c129cd01b..75cebc9826 100644 --- a/src/go/types/api_test.go +++ b/src/go/types/api_test.go @@ -1243,11 +1243,9 @@ func TestConvertibleTo(t *testing.T) { {newDefined(new(Struct)), new(Struct), true}, {newDefined(Typ[Int]), new(Struct), false}, {Typ[UntypedInt], Typ[Int], true}, - // TODO (rFindley): the below behavior is undefined as non-constant untyped - // string values are not permitted by the spec. But we should consider - // changing this case to return 'true', to have more reasonable behavior in - // cases where the API is used for constant expressions. - {Typ[UntypedString], Typ[String], false}, + // Untyped string values are not permitted by the spec, so the below + // behavior is undefined. + {Typ[UntypedString], Typ[String], true}, } { if got := ConvertibleTo(test.v, test.t); got != test.want { t.Errorf("ConvertibleTo(%v, %v) = %t, want %t", test.v, test.t, got, test.want) @@ -1266,13 +1264,11 @@ func TestAssignableTo(t *testing.T) { {newDefined(new(Struct)), new(Struct), true}, {Typ[UntypedBool], Typ[Bool], true}, {Typ[UntypedString], Typ[Bool], false}, - // TODO (rFindley): the below behavior is undefined as AssignableTo is - // intended for non-constant values (and neither UntypedString or - // UntypedInt assignments arise during normal type checking). But as - // described in TestConvertibleTo above, we should consider changing this - // behavior. - {Typ[UntypedString], Typ[String], false}, - {Typ[UntypedInt], Typ[Int], false}, + // Neither untyped string nor untyped numeric assignments arise during + // normal type checking, so the below behavior is technically undefined by + // the spec. + {Typ[UntypedString], Typ[String], true}, + {Typ[UntypedInt], Typ[Int], true}, } { if got := AssignableTo(test.v, test.t); got != test.want { t.Errorf("AssignableTo(%v, %v) = %t, want %t", test.v, test.t, got, test.want) diff --git a/src/go/types/assignments.go b/src/go/types/assignments.go index 9697e504cd..4e8ec278fc 100644 --- a/src/go/types/assignments.go +++ b/src/go/types/assignments.go @@ -34,8 +34,8 @@ func (check *Checker) assignment(x *operand, T Type, context string) { // spec: "If an untyped constant is assigned to a variable of interface // type or the blank identifier, the constant is first converted to type // bool, rune, int, float64, complex128 or string respectively, depending - // on whether the value is a boolean, rune, integer, floating-point, complex, - // or string constant." + // on whether the value is a boolean, rune, integer, floating-point, + // complex, or string constant." if T == nil || IsInterface(T) { if T == nil && x.typ == Typ[UntypedNil] { check.errorf(x.pos(), "use of untyped nil in %s", context) diff --git a/src/go/types/expr.go b/src/go/types/expr.go index 8503a521f6..94d98f0fbb 100644 --- a/src/go/types/expr.go +++ b/src/go/types/expr.go @@ -506,8 +506,6 @@ func (check *Checker) canConvertUntyped(x *operand, target Type) error { if x.mode == invalid || isTyped(x.typ) || target == Typ[Invalid] { return nil } - // TODO(gri) Sloppy code - clean up. This function is central - // to assignment and expression checking. if isUntyped(target) { // both x and target are untyped @@ -519,80 +517,91 @@ func (check *Checker) canConvertUntyped(x *operand, target Type) error { check.updateExprType(x.expr, target, false) } } else if xkind != tkind { - goto Error + return check.newErrorf(x.pos(), "cannot convert %s to %s", x, target) } return nil } - // typed target + if t, ok := target.Underlying().(*Basic); ok && x.mode == constant_ { + if err := check.isRepresentable(x, t); err != nil { + return err + } + // Expression value may have been rounded - update if needed. + check.updateExprVal(x.expr, x.val) + } else { + newTarget := check.implicitType(x, target) + if newTarget == nil { + return check.newErrorf(x.pos(), "cannot convert %s to %s", x, target) + } + target = newTarget + } + x.typ = target + // Even though implicitType can return UntypedNil, this value is final: the + // predeclared identifier nil has no type. + check.updateExprType(x.expr, target, true) + return nil +} + +// implicitType returns the implicit type of x when used in a context where the +// target type is expected. If no such implicit conversion is possible, it +// returns nil. +func (check *Checker) implicitType(x *operand, target Type) Type { + assert(isUntyped(x.typ)) switch t := target.Underlying().(type) { case *Basic: - if x.mode == constant_ { - if err := check.isRepresentable(x, t); err != nil { - return err + assert(x.mode != constant_) + // Non-constant untyped values may appear as the + // result of comparisons (untyped bool), intermediate + // (delayed-checked) rhs operands of shifts, and as + // the value nil. + switch x.typ.(*Basic).kind { + case UntypedBool: + if !isBoolean(target) { + return nil } - // expression value may have been rounded - update if needed - check.updateExprVal(x.expr, x.val) - } else { - // Non-constant untyped values may appear as the - // result of comparisons (untyped bool), intermediate - // (delayed-checked) rhs operands of shifts, and as - // the value nil. - switch x.typ.(*Basic).kind { - case UntypedBool: - if !isBoolean(target) { - goto Error - } - case UntypedInt, UntypedRune, UntypedFloat, UntypedComplex: - if !isNumeric(target) { - goto Error - } - case UntypedString: - // Non-constant untyped string values are not - // permitted by the spec and should not occur. - unreachable() - case UntypedNil: - // Unsafe.Pointer is a basic type that includes nil. - if !hasNil(target) { - goto Error - } - default: - goto Error + case UntypedInt, UntypedRune, UntypedFloat, UntypedComplex: + if !isNumeric(target) { + return nil + } + case UntypedString: + // Non-constant untyped string values are not permitted by the spec and + // should not occur during normal typechecking passes, but this path is + // reachable via the AssignableTo API. + if !isString(target) { + return nil } + case UntypedNil: + // Unsafe.Pointer is a basic type that includes nil. + if !hasNil(target) { + return nil + } + default: + return nil } case *Interface: - // Update operand types to the default type rather then - // the target (interface) type: values must have concrete - // dynamic types. If the value is nil, keep it untyped - // (this is important for tools such as go vet which need - // the dynamic type for argument checking of say, print + // Values must have concrete dynamic types. If the value is nil, + // keep it untyped (this is important for tools such as go vet which + // need the dynamic type for argument checking of say, print // functions) if x.isNil() { - target = Typ[UntypedNil] - } else { - // cannot assign untyped values to non-empty interfaces - check.completeInterface(t) - if !t.Empty() { - goto Error - } - target = Default(x.typ) + return Typ[UntypedNil] + } + // cannot assign untyped values to non-empty interfaces + check.completeInterface(t) + if !t.Empty() { + return nil } + return Default(x.typ) case *Pointer, *Signature, *Slice, *Map, *Chan: if !x.isNil() { - goto Error + return nil } - // keep nil untyped - see comment for interfaces, above - target = Typ[UntypedNil] + // Keep nil untyped - see comment for interfaces, above. + return Typ[UntypedNil] default: - goto Error + return nil } - - x.typ = target - check.updateExprType(x.expr, target, true) // UntypedNils are final - return nil - -Error: - return check.newErrorf(x.pos(), "cannot convert %s to %s", x, target) + return target } func (check *Checker) comparison(x, y *operand, op token.Token) { diff --git a/src/go/types/operand.go b/src/go/types/operand.go index 80d11e2f21..6fbfe09627 100644 --- a/src/go/types/operand.go +++ b/src/go/types/operand.go @@ -205,15 +205,11 @@ func (x *operand) isNil() bool { return x.mode == value && x.typ == Typ[UntypedNil] } -// TODO(gri) The functions operand.assignableTo, checker.convertUntyped, -// checker.representable, and checker.assignment are -// overlapping in functionality. Need to simplify and clean up. - -// assignableTo reports whether x is assignable to a variable of type T. -// If the result is false and a non-nil reason is provided, it may be set -// to a more detailed explanation of the failure (result != ""). -// The check parameter may be nil if assignableTo is invoked through -// an exported API call, i.e., when all methods have been type-checked. +// assignableTo reports whether x is assignable to a variable of type T. If the +// result is false and a non-nil reason is provided, it may be set to a more +// detailed explanation of the failure (result != ""). The check parameter may +// be nil if assignableTo is invoked through an exported API call, i.e., when +// all methods have been type-checked. func (x *operand) assignableTo(check *Checker, T Type, reason *string) bool { if x.mode == invalid || T == Typ[Invalid] { return true // avoid spurious errors @@ -229,34 +225,17 @@ func (x *operand) assignableTo(check *Checker, T Type, reason *string) bool { Vu := V.Underlying() Tu := T.Underlying() - // x is an untyped value representable by a value of type T - // TODO(gri) This is borrowing from checker.convertUntyped and - // checker.representable. Need to clean up. + // x is an untyped value representable by a value of type T. if isUntyped(Vu) { - switch t := Tu.(type) { - case *Basic: - if x.isNil() && t.kind == UnsafePointer { - return true - } - if x.mode == constant_ { - return representableConst(x.val, check, t, nil) - } - // The result of a comparison is an untyped boolean, - // but may not be a constant. - if Vb, _ := Vu.(*Basic); Vb != nil { - return Vb.kind == UntypedBool && isBoolean(Tu) - } - case *Interface: - check.completeInterface(t) - return x.isNil() || t.Empty() - case *Pointer, *Signature, *Slice, *Map, *Chan: - return x.isNil() + if t, ok := Tu.(*Basic); ok && x.mode == constant_ { + return representableConst(x.val, check, t, nil) } + return check.implicitType(x, Tu) != nil } // Vu is typed - // x's type V and T have identical underlying types - // and at least one of V or T is not a named type + // x's type V and T have identical underlying types and at least one of V or + // T is not a named type. if check.identical(Vu, Tu) && (!isNamed(V) || !isNamed(T)) { return true }