Skip to content

Commit

Permalink
go/types, types2: use orig. compiler error message for a shift error
Browse files Browse the repository at this point in the history
Slightly better for cases such as string(1 << s).
Leaves type-checker tests alone for now because
there are multiple dozens.

For #45117.

Change-Id: I47b314c713fabe424c2158674bf965416a8a6f5c
Reviewed-on: https://go-review.googlesource.com/c/go/+/379274
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
  • Loading branch information
griesemer committed Jan 19, 2022
1 parent a4d3c73 commit 1efc581
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 16 deletions.
23 changes: 15 additions & 8 deletions src/cmd/compile/internal/types2/expr.go
Original file line number Diff line number Diff line change
Expand Up @@ -504,8 +504,11 @@ func (check *Checker) invalidConversion(code errorCode, x *operand, target Type)
// Also, if x is a constant, it must be representable as a value of typ,
// and if x is the (formerly untyped) lhs operand of a non-constant
// shift, it must be an integer value.
//
func (check *Checker) updateExprType(x syntax.Expr, typ Type, final bool) {
check.updateExprType0(nil, x, typ, final)
}

func (check *Checker) updateExprType0(parent, x syntax.Expr, typ Type, final bool) {
old, found := check.untyped[x]
if !found {
return // nothing to do
Expand Down Expand Up @@ -548,7 +551,7 @@ func (check *Checker) updateExprType(x syntax.Expr, typ Type, final bool) {
// No operands to take care of.

case *syntax.ParenExpr:
check.updateExprType(x.X, typ, final)
check.updateExprType0(x, x.X, typ, final)

// case *syntax.UnaryExpr:
// // If x is a constant, the operands were constants.
Expand All @@ -559,7 +562,7 @@ func (check *Checker) updateExprType(x syntax.Expr, typ Type, final bool) {
// if old.val != nil {
// break
// }
// check.updateExprType(x.X, typ, final)
// check.updateExprType0(x, x.X, typ, final)

case *syntax.Operation:
if x.Y == nil {
Expand All @@ -580,7 +583,7 @@ func (check *Checker) updateExprType(x syntax.Expr, typ Type, final bool) {
if old.val != nil {
break
}
check.updateExprType(x.X, typ, final)
check.updateExprType0(x, x.X, typ, final)
break
}

Expand All @@ -594,11 +597,11 @@ func (check *Checker) updateExprType(x syntax.Expr, typ Type, final bool) {
} else if isShift(x.Op) {
// The result type depends only on lhs operand.
// The rhs type was updated when checking the shift.
check.updateExprType(x.X, typ, final)
check.updateExprType0(x, x.X, typ, final)
} else {
// The operand types match the result type.
check.updateExprType(x.X, typ, final)
check.updateExprType(x.Y, typ, final)
check.updateExprType0(x, x.X, typ, final)
check.updateExprType0(x, x.Y, typ, final)
}

default:
Expand All @@ -622,7 +625,11 @@ func (check *Checker) updateExprType(x syntax.Expr, typ Type, final bool) {
// We already know from the shift check that it is representable
// as an integer if it is a constant.
if !allInteger(typ) {
check.errorf(x, invalidOp+"shifted operand %s (type %s) must be integer", x, typ)
if check.conf.CompilerErrorMessages {
check.errorf(x, invalidOp+"%s (shift of type %s)", parent, typ)
} else {
check.errorf(x, invalidOp+"shifted operand %s (type %s) must be integer", x, typ)
}
return
}
// Even if we have an integer, if the value is a constant we
Expand Down
21 changes: 14 additions & 7 deletions src/go/types/expr.go
Original file line number Diff line number Diff line change
Expand Up @@ -472,8 +472,11 @@ func (check *Checker) invalidConversion(code errorCode, x *operand, target Type)
// Also, if x is a constant, it must be representable as a value of typ,
// and if x is the (formerly untyped) lhs operand of a non-constant
// shift, it must be an integer value.
//
func (check *Checker) updateExprType(x ast.Expr, typ Type, final bool) {
check.updateExprType0(nil, x, typ, final)
}

func (check *Checker) updateExprType0(parent, x ast.Expr, typ Type, final bool) {
old, found := check.untyped[x]
if !found {
return // nothing to do
Expand Down Expand Up @@ -515,7 +518,7 @@ func (check *Checker) updateExprType(x ast.Expr, typ Type, final bool) {
// No operands to take care of.

case *ast.ParenExpr:
check.updateExprType(x.X, typ, final)
check.updateExprType0(x, x.X, typ, final)

case *ast.UnaryExpr:
// If x is a constant, the operands were constants.
Expand All @@ -526,7 +529,7 @@ func (check *Checker) updateExprType(x ast.Expr, typ Type, final bool) {
if old.val != nil {
break
}
check.updateExprType(x.X, typ, final)
check.updateExprType0(x, x.X, typ, final)

case *ast.BinaryExpr:
if old.val != nil {
Expand All @@ -538,11 +541,11 @@ func (check *Checker) updateExprType(x ast.Expr, typ Type, final bool) {
} else if isShift(x.Op) {
// The result type depends only on lhs operand.
// The rhs type was updated when checking the shift.
check.updateExprType(x.X, typ, final)
check.updateExprType0(x, x.X, typ, final)
} else {
// The operand types match the result type.
check.updateExprType(x.X, typ, final)
check.updateExprType(x.Y, typ, final)
check.updateExprType0(x, x.X, typ, final)
check.updateExprType0(x, x.Y, typ, final)
}

default:
Expand All @@ -566,7 +569,11 @@ func (check *Checker) updateExprType(x ast.Expr, typ Type, final bool) {
// We already know from the shift check that it is representable
// as an integer if it is a constant.
if !allInteger(typ) {
check.invalidOp(x, _InvalidShiftOperand, "shifted operand %s (type %s) must be integer", x, typ)
if compilerErrorMessages {
check.invalidOp(x, _InvalidShiftOperand, "%s (shift of type %s)", parent, typ)
} else {
check.invalidOp(x, _InvalidShiftOperand, "shifted operand %s (type %s) must be integer", x, typ)
}
return
}
// Even if we have an integer, if the value is a constant we
Expand Down
2 changes: 1 addition & 1 deletion test/fixedbugs/issue28079c.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@ package p
import "unsafe"

func f() {
_ = complex(1<<uintptr(unsafe.Pointer(nil)), 0) // ERROR "invalid operation: .*shift of type float64.*|non-integer type for left operand of shift|shifted operand .* must be integer"
_ = complex(1<<uintptr(unsafe.Pointer(nil)), 0) // ERROR "invalid operation: .*shift of type float64.*|non-integer type for left operand of shift"
}

0 comments on commit 1efc581

Please sign in to comment.