Skip to content

Commit

Permalink
Move the cmp function into the vals package
Browse files Browse the repository at this point in the history
Move the value comparison logic into `pkg/eval/vals` so it can be used
by packges other than `eval`. For example, the vals package `repr`
implementation as well as other Elvish modules which may want to compare
Elvish values.

This also augments the comparison logic to handle types that do not have
an obvious, explicitly handled, ordering. For example, Elvish exceptions,
functions, and styled text. If the types are the same then the values are
considered equal. This means you can now do things like `compare $nil $nil`
or `put ?(fail y) ?(fail x) | order` without raising an exception.

Related #1495
  • Loading branch information
Kurtis Rader committed May 16, 2023
1 parent 19bd75d commit 0d282c5
Show file tree
Hide file tree
Showing 5 changed files with 165 additions and 115 deletions.
116 changes: 4 additions & 112 deletions pkg/eval/builtin_fn_pred.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
package eval

import (
"math"
"math/big"

"src.elv.sh/pkg/eval/errs"
"src.elv.sh/pkg/eval/vals"
)
Expand Down Expand Up @@ -59,119 +56,14 @@ var ErrUncomparable = errs.BadValue{
Valid: "comparable values", Actual: "uncomparable values"}

func compare(fm *Frame, a, b any) (int, error) {
switch cmp(a, b) {
case less:
switch vals.Cmp(a, b) {
case vals.CmpLess:
return -1, nil
case equal:
case vals.CmpEqual:
return 0, nil
case more:
case vals.CmpMore:
return 1, nil
default:
return 0, ErrUncomparable
}
}

type ordering uint8

const (
less ordering = iota
equal
more
uncomparable
)

func cmp(a, b any) ordering {
switch a := a.(type) {
case int, *big.Int, *big.Rat, float64:
switch b.(type) {
case int, *big.Int, *big.Rat, float64:
a, b := vals.UnifyNums2(a, b, 0)
switch a := a.(type) {
case int:
return compareInt(a, b.(int))
case *big.Int:
return compareInt(a.Cmp(b.(*big.Int)), 0)
case *big.Rat:
return compareInt(a.Cmp(b.(*big.Rat)), 0)
case float64:
return compareFloat(a, b.(float64))
default:
panic("unreachable")
}
}
case string:
if b, ok := b.(string); ok {
switch {
case a == b:
return equal
case a < b:
return less
default: // a > b
return more
}
}
case vals.List:
if b, ok := b.(vals.List); ok {
aIt := a.Iterator()
bIt := b.Iterator()
for aIt.HasElem() && bIt.HasElem() {
o := cmp(aIt.Elem(), bIt.Elem())
if o != equal {
return o
}
aIt.Next()
bIt.Next()
}
switch {
case a.Len() == b.Len():
return equal
case a.Len() < b.Len():
return less
default: // a.Len() > b.Len()
return more
}
}
case bool:
if b, ok := b.(bool); ok {
switch {
case a == b:
return equal
//lint:ignore S1002 using booleans as values, not conditions
case a == false: // b == true is implicit
return less
default: // a == true && b == false
return more
}
}
}
return uncomparable
}

func compareInt(a, b int) ordering {
if a < b {
return less
} else if a > b {
return more
}
return equal
}

func compareFloat(a, b float64) ordering {
// For the sake of ordering, NaN's are considered equal to each
// other and smaller than all numbers
switch {
case math.IsNaN(a):
if math.IsNaN(b) {
return equal
}
return less
case math.IsNaN(b):
return more
case a < b:
return less
case a > b:
return more
default: // a == b
return equal
}
}
12 changes: 12 additions & 0 deletions pkg/eval/builtin_fn_pred_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,18 @@ func TestCompare(t *testing.T) {
That("compare [a, a] [a, b]").Puts(-1),
That("compare [x, y] [x, y]").Puts(0),

// Comparing nil.
That("compare $nil $nil").Puts(0),

// Comparing exceptions.
That("compare ?(fail x) ?(fail y)").Puts(0),

// Comparing functions.
That("compare {|| put x} {|| put y}").Puts(0),

// Comparing styled text.
That("compare (styled x blue) (styled y green)").Puts(0),

// Uncomparable values.
That("compare 1 (num 1)").Throws(ErrUncomparable),
That("compare x [x]").Throws(ErrUncomparable),
Expand Down
6 changes: 3 additions & 3 deletions pkg/eval/builtin_fn_stream.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,12 +208,12 @@ func (s *slice) Less(i, j int) bool {

if s.lessThan == nil {
// Use the builtin comparator.
o := cmp(a, b)
if o == uncomparable {
o := vals.Cmp(a, b)
if o == vals.CmpUncomparable {
s.err = ErrUncomparable
return true
}
return o == less
return o == vals.CmpLess
}

// Use the &less-than callback.
Expand Down
9 changes: 9 additions & 0 deletions pkg/eval/builtin_fn_stream_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,15 @@ func TestOrder(t *testing.T) {
Puts(vals.EmptyList, vals.MakeList("a"),
vals.MakeList("a", 1), vals.MakeList("a", 2)),

// Attempting to order types we don't have explicit support for should
// treat the values as equal if the types are the same. Thus the order
// of the values should be unchanged since `order` performs a stable
// sort.
That("put $nil $nil | order").
Puts(nil, nil),
That("put ?(fail y) ?(fail z) ?(fail x) | order | each {|e| put $e[reason] }").
Puts(FailError{"y"}, FailError{"z"}, FailError{"x"}),

// Attempting to order uncomparable values
That("put (num 1) 1 | order").
Throws(ErrUncomparable, "order"),
Expand Down
137 changes: 137 additions & 0 deletions pkg/eval/vals/compare.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
package vals

import (
"math"
"math/big"
"reflect"
)

type ordering uint8

const (
CmpLess ordering = iota
CmpEqual
CmpMore
CmpUncomparable
)

// Cmp compares two arbitrary Elvish types and returns a value that indicates
// whether the values are not comparable or the first value is less, equal, or
// greater than the second value.
func Cmp(a, b any) ordering {
switch a := a.(type) {
case int, *big.Int, *big.Rat, float64:
switch b.(type) {
case int, *big.Int, *big.Rat, float64:
a, b := UnifyNums2(a, b, 0)
switch a := a.(type) {
case int:
return compareInt(a, b.(int))
case *big.Int:
return compareInt(a.Cmp(b.(*big.Int)), 0)
case *big.Rat:
return compareInt(a.Cmp(b.(*big.Rat)), 0)
case float64:
return compareFloat(a, b.(float64))
default:
panic("unreachable")
}
}
case string:
if b, ok := b.(string); ok {
switch {
case a == b:
return CmpEqual
case a < b:
return CmpLess
default: // a > b
return CmpMore
}
}
case List:
if b, ok := b.(List); ok {
aIt := a.Iterator()
bIt := b.Iterator()
for aIt.HasElem() && bIt.HasElem() {
o := Cmp(aIt.Elem(), bIt.Elem())
if o != CmpEqual {
return o
}
aIt.Next()
bIt.Next()
}
switch {
case a.Len() == b.Len():
return CmpEqual
case a.Len() < b.Len():
return CmpLess
default: // a.Len() > b.Len()
return CmpMore
}
}
// TODO: Figure out a sane solution for comparing maps. Obviously the
// definition of "less than" is ill-defined for maps but it would be nice if
// we had a solution similar to that for the List case above. One solution
// is to treat each map as an ordered list of [key value] pairs (i.e.,
// sorted by their keys) and use the List comparison logic above. Yes, that
// behavior is hard to document but has the virtue of providing an
// unambiguous means of sorting maps.
case Map:
if _, ok := b.(Map); ok {
return CmpEqual
}
case bool:
if b, ok := b.(bool); ok {
switch {
case a == b:
return CmpEqual
//lint:ignore S1002 using booleans as values, not conditions
case a == false: // b == true is implicit
return CmpLess
default: // a == true && b == false
return CmpMore
}
}
}

// We've been handed a type we don't explicitly handle above. If the types
// are the same assume the values are equal. This handles types such as nil,
// Elvish exceptions, functions, styled text, etc. for which there is not an
// obvious ordering.
aType := reflect.TypeOf(a)
bType := reflect.TypeOf(b)
if aType == bType {
return CmpEqual
}

return CmpUncomparable
}

func compareInt(a, b int) ordering {
if a < b {
return CmpLess
} else if a > b {
return CmpMore
}
return CmpEqual
}

func compareFloat(a, b float64) ordering {
// For the sake of ordering, NaN's are considered equal to each
// other and smaller than all numbers
switch {
case math.IsNaN(a):
if math.IsNaN(b) {
return CmpEqual
}
return CmpLess
case math.IsNaN(b):
return CmpMore
case a < b:
return CmpLess
case a > b:
return CmpMore
default: // a == b
return CmpEqual
}
}

0 comments on commit 0d282c5

Please sign in to comment.