]> Cypherpunks repositories - gostls13.git/commitdiff
go/types: factor out usage of implicit type
authorRob Findley <rfindley@google.com>
Mon, 13 Jul 2020 02:36:34 +0000 (22:36 -0400)
committerRobert Findley <rfindley@google.com>
Fri, 28 Aug 2020 16:45:21 +0000 (16:45 +0000)
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 <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
src/go/types/api_test.go
src/go/types/assignments.go
src/go/types/expr.go
src/go/types/operand.go

index 6c129cd01b6c7fd7e53d179620420150926be051..75cebc98261124891abf04af07bf933f868ad6c8 100644 (file)
@@ -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)
index 9697e504cdc8301c1f020e3b7c0af97383425df7..4e8ec278fcb91e9084f0d3b2e551223dcc0cf93a 100644 (file)
@@ -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)
index 8503a521f674c6f70d385c517e0d55b8a9284540..94d98f0fbb36ebd3a5b76b9f59935e5d05c41c02 100644 (file)
@@ -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) {
index 80d11e2f2153019a225b126dd0c12cbf9aafd612..6fbfe09627a002e737320efeb03918e6c366cc51 100644 (file)
@@ -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
        }