go/types, types2: report better error messages for slice expressions

Explicitly compute the common underlying type and while doing
so report better slice-expression relevant error messages.
Streamline message format for index and slice errors.

This removes the last uses of the coreString and match functions.
Delete them.

Change-Id: I4b50dda1ef7e2ab5e296021458f7f0b6f6e229cd
Reviewed-on: https://go-review.googlesource.com/c/go/+/655935
Reviewed-by: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>

src/cmd/compile/internal/types2/index.go

@@ -183,7 +183,7 @@ func (check *Checker) indexExpr(x *operand, e *syntax.IndexExpr) (isFuncInst boo
 	}
 
 	if !valid {
-		check.errorf(e.Pos(), NonSliceableOperand, invalidOp+"cannot index %s", x)
+		check.errorf(e.Pos(), NonSliceableOperand, "cannot index %s", x)
 		check.use(e.Index)
 		x.mode = invalid
 		return false
@@ -213,11 +213,51 @@ func (check *Checker) sliceExpr(x *operand, e *syntax.SliceExpr) {
 		return
 	}
 
+	// determine common underlying type cu
+	var ct, cu Type // type and respective common underlying type
+	var hasString bool
+	typeset(x.typ, func(t, u Type) bool {
+		if u == nil {
+			check.errorf(x, NonSliceableOperand, "cannot slice %s: no specific type in %s", x, x.typ)
+			cu = nil
+			return false
+		}
+
+		// Treat strings like byte slices but remember that we saw a string.
+		if isString(u) {
+			u = NewSlice(universeByte)
+			hasString = true
+		}
+
+		// If this is the first type we're seeing, we're done.
+		if cu == nil {
+			ct, cu = t, u
+			return true
+		}
+
+		// Otherwise, the current type must have the same underlying type as all previous types.
+		if !Identical(cu, u) {
+			check.errorf(x, NonSliceableOperand, "cannot slice %s: %s and %s have different underlying types", x, ct, t)
+			cu = nil
+			return false
+		}
+
+		return true
+	})
+	if hasString {
+		// If we saw a string, proceed with string type,
+		// but don't go from untyped string to string.
+		cu = Typ[String]
+		if !isTypeParam(x.typ) {
+			cu = under(x.typ) // untyped string remains untyped
+		}
+	}
+
 	valid := false
 	length := int64(-1) // valid if >= 0
-	switch u := coreString(x.typ).(type) {
+	switch u := cu.(type) {
 	case nil:
-		check.errorf(x, NonSliceableOperand, invalidOp+"cannot slice %s: %s has no common underlying type", x, x.typ)
+		// error reported above
 		x.mode = invalid
 		return
 
@@ -247,7 +287,7 @@ func (check *Checker) sliceExpr(x *operand, e *syntax.SliceExpr) {
 		valid = true
 		length = u.len
 		if x.mode != variable {
-			check.errorf(x, NonSliceableOperand, invalidOp+"%s (slice of unaddressable value)", x)
+			check.errorf(x, NonSliceableOperand, "cannot slice unaddressable value %s", x)
 			x.mode = invalid
 			return
 		}
@@ -266,7 +306,7 @@ func (check *Checker) sliceExpr(x *operand, e *syntax.SliceExpr) {
 	}
 
 	if !valid {
-		check.errorf(x, NonSliceableOperand, invalidOp+"cannot slice %s", x)
+		check.errorf(x, NonSliceableOperand, "cannot slice %s", x)
 		x.mode = invalid
 		return
 	}

src/cmd/compile/internal/types2/under.go

@@ -135,70 +135,3 @@ func commonUnder(t Type, cond func(t, u Type) *typeError) (Type, *typeError) {
 	}
 	return cu, nil
 }
-
-// coreString is like coreType but also considers []byte
-// and strings as identical. In this case, if successful and we saw
-// a string, the result is of type (possibly untyped) string.
-func coreString(t Type) Type {
-	// This explicit case is needed because otherwise the
-	// result would be string if t is an untyped string.
-	if !isTypeParam(t) {
-		return under(t) // untyped string remains untyped
-	}
-
-	var su Type
-	hasString := false
-	typeset(t, func(_, u Type) bool {
-		if u == nil {
-			return false
-		}
-		if isString(u) {
-			u = NewSlice(universeByte)
-			hasString = true
-		}
-		if su != nil {
-			u = match(su, u)
-			if u == nil {
-				su = nil
-				hasString = false
-				return false
-			}
-		}
-		// su == nil || match(su, u) != nil
-		su = u
-		return true
-	})
-	if hasString {
-		return Typ[String]
-	}
-	return su
-}
-
-// If x and y are identical, match returns x.
-// If x and y are identical channels but for their direction
-// and one of them is unrestricted, match returns the channel
-// with the restricted direction.
-// In all other cases, match returns nil.
-func match(x, y Type) Type {
-	// Common case: we don't have channels.
-	if Identical(x, y) {
-		return x
-	}
-
-	// We may have channels that differ in direction only.
-	if x, _ := x.(*Chan); x != nil {
-		if y, _ := y.(*Chan); y != nil && Identical(x.elem, y.elem) {
-			// We have channels that differ in direction only.
-			// If there's an unrestricted channel, select the restricted one.
-			switch {
-			case x.dir == SendRecv:
-				return y
-			case y.dir == SendRecv:
-				return x
-			}
-		}
-	}
-
-	// types are different
-	return nil
-}

src/go/types/index.go

@@ -185,7 +185,7 @@ func (check *Checker) indexExpr(x *operand, e *indexedExpr) (isFuncInst bool) {
 
 	if !valid {
 		// types2 uses the position of '[' for the error
-		check.errorf(x, NonIndexableOperand, invalidOp+"cannot index %s", x)
+		check.errorf(x, NonIndexableOperand, "cannot index %s", x)
 		check.use(e.indices...)
 		x.mode = invalid
 		return false
@@ -215,11 +215,51 @@ func (check *Checker) sliceExpr(x *operand, e *ast.SliceExpr) {
 		return
 	}
 
+	// determine common underlying type cu
+	var ct, cu Type // type and respective common underlying type
+	var hasString bool
+	typeset(x.typ, func(t, u Type) bool {
+		if u == nil {
+			check.errorf(x, NonSliceableOperand, "cannot slice %s: no specific type in %s", x, x.typ)
+			cu = nil
+			return false
+		}
+
+		// Treat strings like byte slices but remember that we saw a string.
+		if isString(u) {
+			u = NewSlice(universeByte)
+			hasString = true
+		}
+
+		// If this is the first type we're seeing, we're done.
+		if cu == nil {
+			ct, cu = t, u
+			return true
+		}
+
+		// Otherwise, the current type must have the same underlying type as all previous types.
+		if !Identical(cu, u) {
+			check.errorf(x, NonSliceableOperand, "cannot slice %s: %s and %s have different underlying types", x, ct, t)
+			cu = nil
+			return false
+		}
+
+		return true
+	})
+	if hasString {
+		// If we saw a string, proceed with string type,
+		// but don't go from untyped string to string.
+		cu = Typ[String]
+		if !isTypeParam(x.typ) {
+			cu = under(x.typ) // untyped string remains untyped
+		}
+	}
+
 	valid := false
 	length := int64(-1) // valid if >= 0
-	switch u := coreString(x.typ).(type) {
+	switch u := cu.(type) {
 	case nil:
-		check.errorf(x, NonSliceableOperand, invalidOp+"cannot slice %s: %s has no common underlying type", x, x.typ)
+		// error reported above
 		x.mode = invalid
 		return
 
@@ -249,7 +289,7 @@ func (check *Checker) sliceExpr(x *operand, e *ast.SliceExpr) {
 		valid = true
 		length = u.len
 		if x.mode != variable {
-			check.errorf(x, NonSliceableOperand, invalidOp+"cannot slice %s (value not addressable)", x)
+			check.errorf(x, NonSliceableOperand, "cannot slice unaddressable value %s", x)
 			x.mode = invalid
 			return
 		}
@@ -268,7 +308,7 @@ func (check *Checker) sliceExpr(x *operand, e *ast.SliceExpr) {
 	}
 
 	if !valid {
-		check.errorf(x, NonSliceableOperand, invalidOp+"cannot slice %s", x)
+		check.errorf(x, NonSliceableOperand, "cannot slice %s", x)
 		x.mode = invalid
 		return
 	}

src/go/types/under.go

@@ -138,70 +138,3 @@ func commonUnder(t Type, cond func(t, u Type) *typeError) (Type, *typeError) {
 	}
 	return cu, nil
 }
-
-// coreString is like coreType but also considers []byte
-// and strings as identical. In this case, if successful and we saw
-// a string, the result is of type (possibly untyped) string.
-func coreString(t Type) Type {
-	// This explicit case is needed because otherwise the
-	// result would be string if t is an untyped string.
-	if !isTypeParam(t) {
-		return under(t) // untyped string remains untyped
-	}
-
-	var su Type
-	hasString := false
-	typeset(t, func(_, u Type) bool {
-		if u == nil {
-			return false
-		}
-		if isString(u) {
-			u = NewSlice(universeByte)
-			hasString = true
-		}
-		if su != nil {
-			u = match(su, u)
-			if u == nil {
-				su = nil
-				hasString = false
-				return false
-			}
-		}
-		// su == nil || match(su, u) != nil
-		su = u
-		return true
-	})
-	if hasString {
-		return Typ[String]
-	}
-	return su
-}
-
-// If x and y are identical, match returns x.
-// If x and y are identical channels but for their direction
-// and one of them is unrestricted, match returns the channel
-// with the restricted direction.
-// In all other cases, match returns nil.
-func match(x, y Type) Type {
-	// Common case: we don't have channels.
-	if Identical(x, y) {
-		return x
-	}
-
-	// We may have channels that differ in direction only.
-	if x, _ := x.(*Chan); x != nil {
-		if y, _ := y.(*Chan); y != nil && Identical(x.elem, y.elem) {
-			// We have channels that differ in direction only.
-			// If there's an unrestricted channel, select the restricted one.
-			switch {
-			case x.dir == SendRecv:
-				return y
-			case y.dir == SendRecv:
-				return x
-			}
-		}
-	}
-
-	// types are different
-	return nil
-}

src/internal/types/testdata/check/typeparams.go

@@ -134,11 +134,11 @@ func _[T interface{ ~string }] (x T, i, j, k int) { var _ T = x[i:j:k /* ERROR "
 type myByte1 []byte
 type myByte2 []byte
 func _[T interface{ []byte | myByte1 | myByte2 }] (x T, i, j, k int) { var _ T = x[i:j:k] }
-func _[T interface{ []byte | myByte1 | []int }] (x T, i, j, k int) { var _ T = x /* ERROR "no common underlying type" */ [i:j:k] }
+func _[T interface{ []byte | myByte1 | []int }] (x T, i, j, k int) { var _ T = x /* ERROR "[]byte and []int have different underlying types" */ [i:j:k] }
 
 func _[T interface{ []byte | myByte1 | myByte2 | string }] (x T, i, j, k int) { var _ T = x[i:j] }
 func _[T interface{ []byte | myByte1 | myByte2 | string }] (x T, i, j, k int) { var _ T = x[i:j:k /* ERROR "3-index slice of string" */ ] }
-func _[T interface{ []byte | myByte1 | []int | string }] (x T, i, j, k int) { var _ T = x /* ERROR "no common underlying type" */ [i:j] }
+func _[T interface{ []byte | myByte1 | []int | string }] (x T, i, j, k int) { var _ T = x /* ERROR "[]byte and []int have different underlying types" */ [i:j] }
 
 // len/cap built-ins
 

test/complit1.go

@@ -18,9 +18,9 @@ func fp() *[3]int
 var mp map[int]*[3]int
 
 var (
-	_ = [3]int{1, 2, 3}[:] // ERROR "slice of unaddressable value"
+	_ = [3]int{1, 2, 3}[:] // ERROR "cannot slice unaddressable value"
-	_ = m[0][:]            // ERROR "slice of unaddressable value"
+	_ = m[0][:]            // ERROR "cannot slice unaddressable value"
-	_ = f()[:]             // ERROR "slice of unaddressable value"
+	_ = f()[:]             // ERROR "cannot slice unaddressable value"
 
 	_ = 301[:]  // ERROR "cannot slice|attempt to slice object that is not"
 	_ = 3.1[:]  // ERROR "cannot slice|attempt to slice object that is not"

test/fixedbugs/bug022.go

@@ -9,7 +9,7 @@ package main
 func putint(digits *string) {
 	var i byte;
 	i = (*digits)[7];  // compiles
-	i = digits[7];  // ERROR "illegal|is not|invalid"
+	i = digits[7];  // ERROR "illegal|is not|cannot index"
 	_ = i;
 }