Adding new ThreeWayComparable interface (#457)
* Add ThreewayComparable Interface to value.go
* Use ThreeWayComparable interface for int type
* refactor time.go to use ThreeWayComparable interface
* Cleanup
* Apply suggestions from review
* Apply changes from code reviews
diff --git a/lib/time/time.go b/lib/time/time.go
index 7316333..0f78142 100644
--- a/lib/time/time.go
+++ b/lib/time/time.go
@@ -209,16 +209,15 @@
}
}
-// CompareSameType implements comparison of two Duration values. required by
-// starlark.Comparable interface.
-func (d Duration) CompareSameType(op syntax.Token, v starlark.Value, depth int) (bool, error) {
- cmp := 0
+// Cmp implements comparison of two Duration values. required by
+// starlark.TotallyOrdered interface.
+func (d Duration) Cmp(v starlark.Value, depth int) (int, error) {
if x, y := d, v.(Duration); x < y {
- cmp = -1
+ return -1, nil
} else if x > y {
- cmp = 1
+ return 1, nil
}
- return threeway(op, cmp), nil
+ return 0, nil
}
// Binary implements binary operators, which satisfies the starlark.HasBinary
@@ -392,18 +391,17 @@
)
}
-// CompareSameType implements comparison of two Time values. required by
-// starlark.Comparable interface.
-func (t Time) CompareSameType(op syntax.Token, yV starlark.Value, depth int) (bool, error) {
+// Cmp implements comparison of two Time values. Required by
+// starlark.TotallyOrdered interface.
+func (t Time) Cmp(yV starlark.Value, depth int) (int, error) {
x := time.Time(t)
y := time.Time(yV.(Time))
- cmp := 0
if x.Before(y) {
- cmp = -1
+ return -1, nil
} else if x.After(y) {
- cmp = 1
+ return 1, nil
}
- return threeway(op, cmp), nil
+ return 0, nil
}
// Binary implements binary operators, which satisfies the starlark.HasBinary
@@ -485,23 +483,3 @@
sort.Strings(names)
return names
}
-
-// Threeway interprets a three-way comparison value cmp (-1, 0, +1)
-// as a boolean comparison (e.g. x < y).
-func threeway(op syntax.Token, cmp int) bool {
- switch op {
- case syntax.EQL:
- return cmp == 0
- case syntax.NEQ:
- return cmp != 0
- case syntax.LE:
- return cmp <= 0
- case syntax.LT:
- return cmp < 0
- case syntax.GE:
- return cmp >= 0
- case syntax.GT:
- return cmp > 0
- }
- panic(op)
-}
diff --git a/starlark/int.go b/starlark/int.go
index 5c5ea12..a264e9d 100644
--- a/starlark/int.go
+++ b/starlark/int.go
@@ -190,14 +190,16 @@
}
return 12582917 * uint32(lo+3), nil
}
-func (x Int) CompareSameType(op syntax.Token, v Value, depth int) (bool, error) {
+
+// Required by the TotallyOrdered interface
+func (x Int) Cmp(v Value, depth int) (int, error) {
y := v.(Int)
xSmall, xBig := x.get()
ySmall, yBig := y.get()
if xBig != nil || yBig != nil {
- return threeway(op, x.bigInt().Cmp(y.bigInt())), nil
+ return x.bigInt().Cmp(y.bigInt()), nil
}
- return threeway(op, signum64(xSmall-ySmall)), nil
+ return signum64(xSmall - ySmall), nil // safe: int32 operands
}
// Float returns the float value nearest i.
diff --git a/starlark/value.go b/starlark/value.go
index 7dc0cba..da21795 100644
--- a/starlark/value.go
+++ b/starlark/value.go
@@ -131,15 +131,41 @@
CompareSameType(op syntax.Token, y Value, depth int) (bool, error)
}
+// A TotallyOrdered is a type whose values form a total order:
+// if x and y are of the same TotallyOrdered type, then x must be less than y,
+// greater than y, or equal to y.
+//
+// It is simpler than Comparable and should be preferred in new code,
+// but if a type implements both interfaces, Comparable takes precedence.
+type TotallyOrdered interface {
+ Value
+ // Cmp compares two values x and y of the same totally ordered type.
+ // It returns negative if x < y, positive if x > y, and zero if the values are equal.
+ //
+ // Implementations that recursively compare subcomponents of
+ // the value should use the CompareDepth function, not Cmp, to
+ // avoid infinite recursion on cyclic structures.
+ //
+ // The depth parameter is used to bound comparisons of cyclic
+ // data structures. Implementations should decrement depth
+ // before calling CompareDepth and should return an error if depth
+ // < 1.
+ //
+ // Client code should not call this method. Instead, use the
+ // standalone Compare or Equals functions, which are defined for
+ // all pairs of operands.
+ Cmp(y Value, depth int) (int, error)
+}
+
var (
- _ Comparable = Int{}
- _ Comparable = False
- _ Comparable = Float(0)
- _ Comparable = String("")
- _ Comparable = (*Dict)(nil)
- _ Comparable = (*List)(nil)
- _ Comparable = Tuple(nil)
- _ Comparable = (*Set)(nil)
+ _ TotallyOrdered = Int{}
+ _ TotallyOrdered = Float(0)
+ _ Comparable = False
+ _ Comparable = String("")
+ _ Comparable = (*Dict)(nil)
+ _ Comparable = (*List)(nil)
+ _ Comparable = Tuple(nil)
+ _ Comparable = (*Set)(nil)
)
// A Callable value f may be the operand of a function call, f(x).
@@ -439,9 +465,9 @@
return math.Abs(f) <= math.MaxFloat64
}
-func (x Float) CompareSameType(op syntax.Token, y_ Value, depth int) (bool, error) {
+func (x Float) Cmp(y_ Value, depth int) (int, error) {
y := y_.(Float)
- return threeway(op, floatCmp(x, y)), nil
+ return floatCmp(x, y), nil
}
// floatCmp performs a three-valued comparison on floats,
@@ -1299,6 +1325,14 @@
return xcomp.CompareSameType(op, y, depth)
}
+ if xcomp, ok := x.(TotallyOrdered); ok {
+ t, err := xcomp.Cmp(y, depth)
+ if err != nil {
+ return false, err
+ }
+ return threeway(op, t), nil
+ }
+
// use identity comparison
switch op {
case syntax.EQL:
