Skip to content

Commit db56d4d

Browse files
robpikerobpike
committed
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
1 parent 5f739d9 commit db56d4d

File tree

3 files changed

+105
-56
lines changed

3 files changed

+105
-56
lines changed

‎src/text/template/doc.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -338,10 +338,11 @@ arguments will be evaluated.)
338338
The comparison functions work on basic types only (or named basic
339339
types, such as "type Celsius float32"). They implement the Go rules
340340
for comparison of values, except that size and exact type are
341-
ignored, so any integer value may be compared with any other integer
342-
value, any unsigned integer value may be compared with any other
343-
unsigned integer value, and so on. However, as usual, one may not
344-
compare an int with a float32 and so on.
341+
ignored, so any integer value, signed or unsigned, may be compared
342+
with any other integer value. (The arithmetic value is compared,
343+
not the bit pattern, so all negative integers are less than all
344+
unsigned integers.) However, as usual, one may not compare an int
345+
with a float32 and so on.
345346
346347
Associated templates
347348

‎src/text/template/exec_test.go

Lines changed: 48 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -902,8 +902,8 @@ var cmpTests = []cmpTest{
902902
{"eq 1 2", "false", true},
903903
{"eq `xy` `xy`", "true", true},
904904
{"eq `xy` `xyz`", "false", true},
905-
{"eq .Xuint .Xuint", "true", true},
906-
{"eq .Xuint .Yuint", "false", true},
905+
{"eq .Uthree .Uthree", "true", true},
906+
{"eq .Uthree .Ufour", "false", true},
907907
{"eq 3 4 5 6 3", "true", true},
908908
{"eq 3 4 5 6 7", "false", true},
909909
{"ne true true", "false", true},
@@ -916,16 +916,16 @@ var cmpTests = []cmpTest{
916916
{"ne 1 2", "true", true},
917917
{"ne `xy` `xy`", "false", true},
918918
{"ne `xy` `xyz`", "true", true},
919-
{"ne .Xuint .Xuint", "false", true},
920-
{"ne .Xuint .Yuint", "true", true},
919+
{"ne .Uthree .Uthree", "false", true},
920+
{"ne .Uthree .Ufour", "true", true},
921921
{"lt 1.5 1.5", "false", true},
922922
{"lt 1.5 2.5", "true", true},
923923
{"lt 1 1", "false", true},
924924
{"lt 1 2", "true", true},
925925
{"lt `xy` `xy`", "false", true},
926926
{"lt `xy` `xyz`", "true", true},
927-
{"lt .Xuint .Xuint", "false", true},
928-
{"lt .Xuint .Yuint", "true", true},
927+
{"lt .Uthree .Uthree", "false", true},
928+
{"lt .Uthree .Ufour", "true", true},
929929
{"le 1.5 1.5", "true", true},
930930
{"le 1.5 2.5", "true", true},
931931
{"le 2.5 1.5", "false", true},
@@ -935,19 +935,19 @@ var cmpTests = []cmpTest{
935935
{"le `xy` `xy`", "true", true},
936936
{"le `xy` `xyz`", "true", true},
937937
{"le `xyz` `xy`", "false", true},
938-
{"le .Xuint .Xuint", "true", true},
939-
{"le .Xuint .Yuint", "true", true},
940-
{"le .Yuint .Xuint", "false", true},
938+
{"le .Uthree .Uthree", "true", true},
939+
{"le .Uthree .Ufour", "true", true},
940+
{"le .Ufour .Uthree", "false", true},
941941
{"gt 1.5 1.5", "false", true},
942942
{"gt 1.5 2.5", "false", true},
943943
{"gt 1 1", "false", true},
944944
{"gt 2 1", "true", true},
945945
{"gt 1 2", "false", true},
946946
{"gt `xy` `xy`", "false", true},
947947
{"gt `xy` `xyz`", "false", true},
948-
{"gt .Xuint .Xuint", "false", true},
949-
{"gt .Xuint .Yuint", "false", true},
950-
{"gt .Yuint .Xuint", "true", true},
948+
{"gt .Uthree .Uthree", "false", true},
949+
{"gt .Uthree .Ufour", "false", true},
950+
{"gt .Ufour .Uthree", "true", true},
951951
{"ge 1.5 1.5", "true", true},
952952
{"ge 1.5 2.5", "false", true},
953953
{"ge 2.5 1.5", "true", true},
@@ -957,25 +957,55 @@ var cmpTests = []cmpTest{
957957
{"ge `xy` `xy`", "true", true},
958958
{"ge `xy` `xyz`", "false", true},
959959
{"ge `xyz` `xy`", "true", true},
960-
{"ge .Xuint .Xuint", "true", true},
961-
{"ge .Xuint .Yuint", "false", true},
962-
{"ge .Yuint .Xuint", "true", true},
960+
{"ge .Uthree .Uthree", "true", true},
961+
{"ge .Uthree .Ufour", "false", true},
962+
{"ge .Ufour .Uthree", "true", true},
963+
// Mixing signed and unsigned integers.
964+
{"eq .Uthree .Three", "true", true},
965+
{"eq .Three .Uthree", "true", true},
966+
{"le .Uthree .Three", "true", true},
967+
{"le .Three .Uthree", "true", true},
968+
{"ge .Uthree .Three", "true", true},
969+
{"ge .Three .Uthree", "true", true},
970+
{"lt .Uthree .Three", "false", true},
971+
{"lt .Three .Uthree", "false", true},
972+
{"gt .Uthree .Three", "false", true},
973+
{"gt .Three .Uthree", "false", true},
974+
{"eq .Ufour .Three", "false", true},
975+
{"lt .Ufour .Three", "false", true},
976+
{"gt .Ufour .Three", "true", true},
977+
{"eq .NegOne .Uthree", "false", true},
978+
{"eq .Uthree .NegOne", "false", true},
979+
{"ne .NegOne .Uthree", "true", true},
980+
{"ne .Uthree .NegOne", "true", true},
981+
{"lt .NegOne .Uthree", "true", true},
982+
{"lt .Uthree .NegOne", "false", true},
983+
{"le .NegOne .Uthree", "true", true},
984+
{"le .Uthree .NegOne", "false", true},
985+
{"gt .NegOne .Uthree", "false", true},
986+
{"gt .Uthree .NegOne", "true", true},
987+
{"ge .NegOne .Uthree", "false", true},
988+
{"ge .Uthree .NegOne", "true", true},
989+
{"eq (index `x` 0) 'x'", "true", true}, // The example that triggered this rule.
990+
{"eq (index `x` 0) 'y'", "false", true},
963991
// Errors
964992
{"eq `xy` 1", "", false}, // Different types.
993+
{"eq 2 2.0", "", false}, // Different types.
965994
{"lt true true", "", false}, // Unordered types.
966995
{"lt 1+0i 1+0i", "", false}, // Unordered types.
967996
}
968997

969998
func TestComparison(t *testing.T) {
970999
b := new(bytes.Buffer)
9711000
var cmpStruct = struct {
972-
Xuint, Yuint uint
973-
}{3, 4}
1001+
Uthree, Ufour uint
1002+
NegOne, Three int
1003+
}{3, 4, -1, 3}
9741004
for _, test := range cmpTests {
9751005
text := fmt.Sprintf("{{if %s}}true{{else}}false{{end}}", test.expr)
9761006
tmpl, err := New("empty").Parse(text)
9771007
if err != nil {
978-
t.Fatal(err)
1008+
t.Fatalf("%q: %s", test.expr, err)
9791009
}
9801010
b.Reset()
9811011
err = tmpl.Execute(b, &cmpStruct)

‎src/text/template/funcs.go

Lines changed: 52 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -314,25 +314,34 @@ func eq(arg1 interface{}, arg2 ...interface{}) (bool, error) {
314314
if err != nil {
315315
return false, err
316316
}
317-
if k1 != k2 {
318-
return false, errBadComparison
319-
}
320317
truth := false
321-
switch k1 {
322-
case boolKind:
323-
truth = v1.Bool() == v2.Bool()
324-
case complexKind:
325-
truth = v1.Complex() == v2.Complex()
326-
case floatKind:
327-
truth = v1.Float() == v2.Float()
328-
case intKind:
329-
truth = v1.Int() == v2.Int()
330-
case stringKind:
331-
truth = v1.String() == v2.String()
332-
case uintKind:
333-
truth = v1.Uint() == v2.Uint()
334-
default:
335-
panic("invalid kind")
318+
if k1 != k2 {
319+
// Special case: Can compare integer values regardless of type's sign.
320+
switch {
321+
case k1 == intKind && k2 == uintKind:
322+
truth = v1.Int() >= 0 && uint64(v1.Int()) == v2.Uint()
323+
case k1 == uintKind && k2 == intKind:
324+
truth = v2.Int() >= 0 && v1.Uint() == uint64(v2.Int())
325+
default:
326+
return false, errBadComparison
327+
}
328+
} else {
329+
switch k1 {
330+
case boolKind:
331+
truth = v1.Bool() == v2.Bool()
332+
case complexKind:
333+
truth = v1.Complex() == v2.Complex()
334+
case floatKind:
335+
truth = v1.Float() == v2.Float()
336+
case intKind:
337+
truth = v1.Int() == v2.Int()
338+
case stringKind:
339+
truth = v1.String() == v2.String()
340+
case uintKind:
341+
truth = v1.Uint() == v2.Uint()
342+
default:
343+
panic("invalid kind")
344+
}
336345
}
337346
if truth {
338347
return true, nil
@@ -360,23 +369,32 @@ func lt(arg1, arg2 interface{}) (bool, error) {
360369
if err != nil {
361370
return false, err
362371
}
363-
if k1 != k2 {
364-
return false, errBadComparison
365-
}
366372
truth := false
367-
switch k1 {
368-
case boolKind, complexKind:
369-
return false, errBadComparisonType
370-
case floatKind:
371-
truth = v1.Float() < v2.Float()
372-
case intKind:
373-
truth = v1.Int() < v2.Int()
374-
case stringKind:
375-
truth = v1.String() < v2.String()
376-
case uintKind:
377-
truth = v1.Uint() < v2.Uint()
378-
default:
379-
panic("invalid kind")
373+
if k1 != k2 {
374+
// Special case: Can compare integer values regardless of type's sign.
375+
switch {
376+
case k1 == intKind && k2 == uintKind:
377+
truth = v1.Int() < 0 || uint64(v1.Int()) < v2.Uint()
378+
case k1 == uintKind && k2 == intKind:
379+
truth = v2.Int() >= 0 && v1.Uint() < uint64(v2.Int())
380+
default:
381+
return false, errBadComparison
382+
}
383+
} else {
384+
switch k1 {
385+
case boolKind, complexKind:
386+
return false, errBadComparisonType
387+
case floatKind:
388+
truth = v1.Float() < v2.Float()
389+
case intKind:
390+
truth = v1.Int() < v2.Int()
391+
case stringKind:
392+
truth = v1.String() < v2.String()
393+
case uintKind:
394+
truth = v1.Uint() < v2.Uint()
395+
default:
396+
panic("invalid kind")
397+
}
380398
}
381399
return truth, nil
382400
}

0 commit comments

Comments
 (0)