From: Robert Griesemer Date: Mon, 12 Dec 2022 21:36:44 +0000 (-0800) Subject: go/types, types2: report type mismatch error when conversion is impossible X-Git-Tag: go1.20rc2~1^2~33 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=5ba98b975638323acf733438a619e9190dfa8afa;p=gostls13.git go/types, types2: report type mismatch error when conversion is impossible Rather than reporting an impossible conversion error when mixing an untyped value with a pointer type in an operation, report a type mismatch error. This fixes a regression in error quality compared to pre-1.18. For the fix, clean up the implementation of canMix, add documentation, and give it a better name. Adjust test case for corresponding error code bacause we now get a better error message (and error code) for the old error code example. Fixes #57160. Change-Id: Ib96ce7cbc44db6905fa2f1c90a3769af609e101b Reviewed-on: https://go-review.googlesource.com/c/go/+/457055 Reviewed-by: Robert Findley Run-TryBot: Robert Griesemer TryBot-Result: Gopher Robot Reviewed-by: Robert Griesemer --- diff --git a/src/cmd/compile/internal/types2/expr.go b/src/cmd/compile/internal/types2/expr.go index 40d6e5da69..9a0348e025 100644 --- a/src/cmd/compile/internal/types2/expr.go +++ b/src/cmd/compile/internal/types2/expr.go @@ -1103,26 +1103,50 @@ func (check *Checker) binary(x *operand, e syntax.Expr, lhs, rhs syntax.Expr, op return } - // TODO(gri) make canMix more efficient - called for each binary operation - canMix := func(x, y *operand) bool { + // mayConvert reports whether the operands x and y may + // possibly have matching types after converting one + // untyped operand to the type of the other. + // If mayConvert returns true, we try to convert the + // operands to each other's types, and if that fails + // we report a conversion failure. + // If mayConvert returns false, we continue without an + // attempt at conversion, and if the operand types are + // not compatible, we report a type mismatch error. + mayConvert := func(x, y *operand) bool { + // If both operands are typed, there's no need for an implicit conversion. + if isTyped(x.typ) && isTyped(y.typ) { + return false + } + // An untyped operand may convert to its default type when paired with an empty interface + // TODO(gri) This should only matter for comparisons (the only binary operation that is + // valid with interfaces), but in that case the assignability check should take + // care of the conversion. Verify and possibly eliminate this extra test. if isNonTypeParamInterface(x.typ) || isNonTypeParamInterface(y.typ) { return true } + // A boolean type can only convert to another boolean type. if allBoolean(x.typ) != allBoolean(y.typ) { return false } + // A string type can only convert to another string type. if allString(x.typ) != allString(y.typ) { return false } - if x.isNil() && !hasNil(y.typ) { - return false + // Untyped nil can only convert to a type that has a nil. + if x.isNil() { + return hasNil(y.typ) + } + if y.isNil() { + return hasNil(x.typ) } - if y.isNil() && !hasNil(x.typ) { + // An untyped operand cannot convert to a pointer. + // TODO(gri) generalize to type parameters + if isPointer(x.typ) || isPointer(y.typ) { return false } return true } - if canMix(x, &y) { + if mayConvert(x, &y) { check.convertUntyped(x, y.typ) if x.mode == invalid { return diff --git a/src/go/types/expr.go b/src/go/types/expr.go index da9cd67826..e09b461d8c 100644 --- a/src/go/types/expr.go +++ b/src/go/types/expr.go @@ -1084,26 +1084,50 @@ func (check *Checker) binary(x *operand, e ast.Expr, lhs, rhs ast.Expr, op token return } - // TODO(gri) make canMix more efficient - called for each binary operation - canMix := func(x, y *operand) bool { + // mayConvert reports whether the operands x and y may + // possibly have matching types after converting one + // untyped operand to the type of the other. + // If mayConvert returns true, we try to convert the + // operands to each other's types, and if that fails + // we report a conversion failure. + // If mayConvert returns false, we continue without an + // attempt at conversion, and if the operand types are + // not compatible, we report a type mismatch error. + mayConvert := func(x, y *operand) bool { + // If both operands are typed, there's no need for an implicit conversion. + if isTyped(x.typ) && isTyped(y.typ) { + return false + } + // An untyped operand may convert to its default type when paired with an empty interface + // TODO(gri) This should only matter for comparisons (the only binary operation that is + // valid with interfaces), but in that case the assignability check should take + // care of the conversion. Verify and possibly eliminate this extra test. if isNonTypeParamInterface(x.typ) || isNonTypeParamInterface(y.typ) { return true } + // A boolean type can only convert to another boolean type. if allBoolean(x.typ) != allBoolean(y.typ) { return false } + // A string type can only convert to another string type. if allString(x.typ) != allString(y.typ) { return false } - if x.isNil() && !hasNil(y.typ) { - return false + // Untyped nil can only convert to a type that has a nil. + if x.isNil() { + return hasNil(y.typ) + } + if y.isNil() { + return hasNil(x.typ) } - if y.isNil() && !hasNil(x.typ) { + // An untyped operand cannot convert to a pointer. + // TODO(gri) generalize to type parameters + if isPointer(x.typ) || isPointer(y.typ) { return false } return true } - if canMix(x, &y) { + if mayConvert(x, &y) { check.convertUntyped(x, y.typ) if x.mode == invalid { return diff --git a/src/internal/types/errors/codes.go b/src/internal/types/errors/codes.go index 7a0c0e16b8..acddcbb9c5 100644 --- a/src/internal/types/errors/codes.go +++ b/src/internal/types/errors/codes.go @@ -882,7 +882,7 @@ const ( // context in which it is used. // // Example: - // var _ = 1 + new(int) + // var _ = 1 + []int{} InvalidUntypedConversion // BadOffsetofSyntax occurs when unsafe.Offsetof is called with an argument diff --git a/src/internal/types/testdata/fixedbugs/issue57160.go b/src/internal/types/testdata/fixedbugs/issue57160.go new file mode 100644 index 0000000000..446d019900 --- /dev/null +++ b/src/internal/types/testdata/fixedbugs/issue57160.go @@ -0,0 +1,10 @@ +// Copyright 2022 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 _(x *int) { + _ = 0 < x // ERROR "invalid operation" + _ = x < 0 // ERROR "invalid operation" +}