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 #7489.

LGTM=adg
R=golang-codereviews, adg
CC=golang-codereviews
https://golang.org/cl/149780043

diff --git a/src/text/template/doc.go b/src/text/template/doc.go
index 7c6efd59cded112d1f201bc5e805a43c7279e3d1..223c595c25d69f507d4fe7f14b3bb6e142b2ac28 100644 (file)
--- 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 663aaf3af8c515e05d5782a39f2f9a4f379090cd..3bffcc1599a2cb70cdb5e6b600c5b8322b89d6a4 100644 (file)
--- 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 e854122624e1aa92e0cac5fed727a05ae69f4d4a..39ee5ed68fbc36b87d1ab2920e5f99646bea0b71 100644 (file)
--- 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
 }