text/template: allow comparison functions to work between any integers

Previously, signed and unsigned integers could not be compared, but
this has problems with things like comparing 'x' with a byte in a string.
Since signed and unsigned integers have a well-defined ordering,
even though their types are different, and since we already allow
comparison regardless of the size of the integers, why not allow it
regardless of the sign?

Integers only, a fine place to draw the line.

Fixes issue 7489.

LGTM=adg
R=golang-codereviews, adg
CC=golang-codereviews
https://codereview.appspot.com/149780043

3 files changed, 105 insertions, 56 deletions

diff --git a/src/text/template/doc.go b/src/text/template/doc.go
index 7c6efd59c..223c595c2 100644
--- a/src/text/template/doc.go
+++ b/src/text/template/doc.go
@@ -338,10 +338,11 @@ arguments will be evaluated.)
 The comparison functions work on basic types only (or named basic
 types, such as "type Celsius float32"). They implement the Go rules
 for comparison of values, except that size and exact type are
-ignored, so any integer value may be compared with any other integer
-value, any unsigned integer value may be compared with any other
-unsigned integer value, and so on. However, as usual, one may not
-compare an int with a float32 and so on.
+ignored, so any integer value, signed or unsigned, may be compared
+with any other integer value. (The arithmetic value is compared,
+not the bit pattern, so all negative integers are less than all
+unsigned integers.) However, as usual, one may not compare an int
+with a float32 and so on.
 
 Associated templates
 
diff --git a/src/text/template/exec_test.go b/src/text/template/exec_test.go
index 663aaf3af..3bffcc159 100644
--- a/src/text/template/exec_test.go
+++ b/src/text/template/exec_test.go
@@ -902,8 +902,8 @@ var cmpTests = []cmpTest{
 	{"eq 1 2", "false", true},
 	{"eq `xy` `xy`", "true", true},
 	{"eq `xy` `xyz`", "false", true},
-	{"eq .Xuint .Xuint", "true", true},
-	{"eq .Xuint .Yuint", "false", true},
+	{"eq .Uthree .Uthree", "true", true},
+	{"eq .Uthree .Ufour", "false", true},
 	{"eq 3 4 5 6 3", "true", true},
 	{"eq 3 4 5 6 7", "false", true},
 	{"ne true true", "false", true},
@@ -916,16 +916,16 @@ var cmpTests = []cmpTest{
 	{"ne 1 2", "true", true},
 	{"ne `xy` `xy`", "false", true},
 	{"ne `xy` `xyz`", "true", true},
-	{"ne .Xuint .Xuint", "false", true},
-	{"ne .Xuint .Yuint", "true", true},
+	{"ne .Uthree .Uthree", "false", true},
+	{"ne .Uthree .Ufour", "true", true},
 	{"lt 1.5 1.5", "false", true},
 	{"lt 1.5 2.5", "true", true},
 	{"lt 1 1", "false", true},
 	{"lt 1 2", "true", true},
 	{"lt `xy` `xy`", "false", true},
 	{"lt `xy` `xyz`", "true", true},
-	{"lt .Xuint .Xuint", "false", true},
-	{"lt .Xuint .Yuint", "true", true},
+	{"lt .Uthree .Uthree", "false", true},
+	{"lt .Uthree .Ufour", "true", true},
 	{"le 1.5 1.5", "true", true},
 	{"le 1.5 2.5", "true", true},
 	{"le 2.5 1.5", "false", true},
@@ -935,9 +935,9 @@ var cmpTests = []cmpTest{
 	{"le `xy` `xy`", "true", true},
 	{"le `xy` `xyz`", "true", true},
 	{"le `xyz` `xy`", "false", true},
-	{"le .Xuint .Xuint", "true", true},
-	{"le .Xuint .Yuint", "true", true},
-	{"le .Yuint .Xuint", "false", true},
+	{"le .Uthree .Uthree", "true", true},
+	{"le .Uthree .Ufour", "true", true},
+	{"le .Ufour .Uthree", "false", true},
 	{"gt 1.5 1.5", "false", true},
 	{"gt 1.5 2.5", "false", true},
 	{"gt 1 1", "false", true},
@@ -945,9 +945,9 @@ var cmpTests = []cmpTest{
 	{"gt 1 2", "false", true},
 	{"gt `xy` `xy`", "false", true},
 	{"gt `xy` `xyz`", "false", true},
-	{"gt .Xuint .Xuint", "false", true},
-	{"gt .Xuint .Yuint", "false", true},
-	{"gt .Yuint .Xuint", "true", true},
+	{"gt .Uthree .Uthree", "false", true},
+	{"gt .Uthree .Ufour", "false", true},
+	{"gt .Ufour .Uthree", "true", true},
 	{"ge 1.5 1.5", "true", true},
 	{"ge 1.5 2.5", "false", true},
 	{"ge 2.5 1.5", "true", true},
@@ -957,11 +957,40 @@ var cmpTests = []cmpTest{
 	{"ge `xy` `xy`", "true", true},
 	{"ge `xy` `xyz`", "false", true},
 	{"ge `xyz` `xy`", "true", true},
-	{"ge .Xuint .Xuint", "true", true},
-	{"ge .Xuint .Yuint", "false", true},
-	{"ge .Yuint .Xuint", "true", true},
+	{"ge .Uthree .Uthree", "true", true},
+	{"ge .Uthree .Ufour", "false", true},
+	{"ge .Ufour .Uthree", "true", true},
+	// Mixing signed and unsigned integers.
+	{"eq .Uthree .Three", "true", true},
+	{"eq .Three .Uthree", "true", true},
+	{"le .Uthree .Three", "true", true},
+	{"le .Three .Uthree", "true", true},
+	{"ge .Uthree .Three", "true", true},
+	{"ge .Three .Uthree", "true", true},
+	{"lt .Uthree .Three", "false", true},
+	{"lt .Three .Uthree", "false", true},
+	{"gt .Uthree .Three", "false", true},
+	{"gt .Three .Uthree", "false", true},
+	{"eq .Ufour .Three", "false", true},
+	{"lt .Ufour .Three", "false", true},
+	{"gt .Ufour .Three", "true", true},
+	{"eq .NegOne .Uthree", "false", true},
+	{"eq .Uthree .NegOne", "false", true},
+	{"ne .NegOne .Uthree", "true", true},
+	{"ne .Uthree .NegOne", "true", true},
+	{"lt .NegOne .Uthree", "true", true},
+	{"lt .Uthree .NegOne", "false", true},
+	{"le .NegOne .Uthree", "true", true},
+	{"le .Uthree .NegOne", "false", true},
+	{"gt .NegOne .Uthree", "false", true},
+	{"gt .Uthree .NegOne", "true", true},
+	{"ge .NegOne .Uthree", "false", true},
+	{"ge .Uthree .NegOne", "true", true},
+	{"eq (index `x` 0) 'x'", "true", true}, // The example that triggered this rule.
+	{"eq (index `x` 0) 'y'", "false", true},
 	// Errors
 	{"eq `xy` 1", "", false},    // Different types.
+	{"eq 2 2.0", "", false},     // Different types.
 	{"lt true true", "", false}, // Unordered types.
 	{"lt 1+0i 1+0i", "", false}, // Unordered types.
 }
@@ -969,13 +998,14 @@ var cmpTests = []cmpTest{
 func TestComparison(t *testing.T) {
 	b := new(bytes.Buffer)
 	var cmpStruct = struct {
-		Xuint, Yuint uint
-	}{3, 4}
+		Uthree, Ufour uint
+		NegOne, Three int
+	}{3, 4, -1, 3}
 	for _, test := range cmpTests {
 		text := fmt.Sprintf("{{if %s}}true{{else}}false{{end}}", test.expr)
 		tmpl, err := New("empty").Parse(text)
 		if err != nil {
-			t.Fatal(err)
+			t.Fatalf("%q: %s", test.expr, err)
 		}
 		b.Reset()
 		err = tmpl.Execute(b, &cmpStruct)
diff --git a/src/text/template/funcs.go b/src/text/template/funcs.go
index e85412262..39ee5ed68 100644
--- a/src/text/template/funcs.go
+++ b/src/text/template/funcs.go
@@ -314,25 +314,34 @@ func eq(arg1 interface{}, arg2 ...interface{}) (bool, error) {
 		if err != nil {
 			return false, err
 		}
-		if k1 != k2 {
-			return false, errBadComparison
-		}
 		truth := false
-		switch k1 {
-		case boolKind:
-			truth = v1.Bool() == v2.Bool()
-		case complexKind:
-			truth = v1.Complex() == v2.Complex()
-		case floatKind:
-			truth = v1.Float() == v2.Float()
-		case intKind:
-			truth = v1.Int() == v2.Int()
-		case stringKind:
-			truth = v1.String() == v2.String()
-		case uintKind:
-			truth = v1.Uint() == v2.Uint()
-		default:
-			panic("invalid kind")
+		if k1 != k2 {
+			// Special case: Can compare integer values regardless of type's sign.
+			switch {
+			case k1 == intKind && k2 == uintKind:
+				truth = v1.Int() >= 0 && uint64(v1.Int()) == v2.Uint()
+			case k1 == uintKind && k2 == intKind:
+				truth = v2.Int() >= 0 && v1.Uint() == uint64(v2.Int())
+			default:
+				return false, errBadComparison
+			}
+		} else {
+			switch k1 {
+			case boolKind:
+				truth = v1.Bool() == v2.Bool()
+			case complexKind:
+				truth = v1.Complex() == v2.Complex()
+			case floatKind:
+				truth = v1.Float() == v2.Float()
+			case intKind:
+				truth = v1.Int() == v2.Int()
+			case stringKind:
+				truth = v1.String() == v2.String()
+			case uintKind:
+				truth = v1.Uint() == v2.Uint()
+			default:
+				panic("invalid kind")
+			}
 		}
 		if truth {
 			return true, nil
@@ -360,23 +369,32 @@ func lt(arg1, arg2 interface{}) (bool, error) {
 	if err != nil {
 		return false, err
 	}
-	if k1 != k2 {
-		return false, errBadComparison
-	}
 	truth := false
-	switch k1 {
-	case boolKind, complexKind:
-		return false, errBadComparisonType
-	case floatKind:
-		truth = v1.Float() < v2.Float()
-	case intKind:
-		truth = v1.Int() < v2.Int()
-	case stringKind:
-		truth = v1.String() < v2.String()
-	case uintKind:
-		truth = v1.Uint() < v2.Uint()
-	default:
-		panic("invalid kind")
+	if k1 != k2 {
+		// Special case: Can compare integer values regardless of type's sign.
+		switch {
+		case k1 == intKind && k2 == uintKind:
+			truth = v1.Int() < 0 || uint64(v1.Int()) < v2.Uint()
+		case k1 == uintKind && k2 == intKind:
+			truth = v2.Int() >= 0 && v1.Uint() < uint64(v2.Int())
+		default:
+			return false, errBadComparison
+		}
+	} else {
+		switch k1 {
+		case boolKind, complexKind:
+			return false, errBadComparisonType
+		case floatKind:
+			truth = v1.Float() < v2.Float()
+		case intKind:
+			truth = v1.Int() < v2.Int()
+		case stringKind:
+			truth = v1.String() < v2.String()
+		case uintKind:
+			truth = v1.Uint() < v2.Uint()
+		default:
+			panic("invalid kind")
+		}
 	}
 	return truth, nil
 }