Re: [PATCH 2/2] compare function

[Edward Welbourne]

Jouke Witteveen (8 June 2020 22:20)
> This is an implementation of a $(compare) function as proposed by
> Edward Welbourne.

Yay ;^>
Looks promising in general.
As ever, review consists mostly of criticism, so let me just say I
approve of the general plan, before I start in on that ...

> It differs from his original proposal in that it behaves differently
> when given 4 or 5 arguments. In short, it's signature is
>
>  $(compare lhs,rhs,less-than[,greater-or-equal])
>  $(compare lhs,rhs,less-than,equal,greater-than)
>
> Documentation and a proper commit message is still missing, but this
> proof-of-concept shows that it is in fact pretty simple to implement.

Simplicity of implementation was not far from my thoughts when I
proposed it; it is also easy to understand ;^>

I note that your implementation only supports numeric lhs and rhs.
Might it be worth falling back, if either of them isn't numeric, to
doing a strcmp() comparison instead ?  Possibly after stripping leading
space from each.  There is value in lexical comparison, too, after all.

I am not convinced it is good to have $5 default to $4, which is what
you've done in effect.  IIRC, most of make's existing functions treat
missing argument as empty; if my memory is correct, it would then be
more consistent to have

  $(compare $lhs,$rhs,$less[,$same[,$more]])

with each of $same and $more empty if omitted.  (I make $less
non-optional because it's perverse to do a comparison and return false
regardless; but it could be optional, "for symmetry", if you like;
albeit that might lead to a temptation to make it default to a non-empty
string, so that $(compare $lhs,$rhs) serves as $lhs < $rhs as a
conditional; but this would violate the very symmetry that motivated
making $less optional.)

As I noted before, the cases where two branches want the same verbose
value can readily enough be handled by using $(if ...) on a condition
using $(compare ...) with two of its output values empty and the other
some non-empty string (due to Lisp influence, I'd use t), or vice versa;
or you could use $(let ...) to put the repeated value in a local
variable that you use in both intended branches, when this more verbose
form proves more readable (as it sometimes shall).

> Additionally, I feel that the interface is clean. In that way, it
> differs from the various proposals for integer operations. After
> thinking about them some more, I came to dislike all current proposals
> because of the unintuitive behavior of subtraction and division.

I think this depends a lot on your intuitions, which can be trained.  I
find $(math $op, $a $b ... $z) = $a $op $b $op ... $op $z to be
entirely intuitive, but then I've been exposed to many ways of doing
algebra and perpetrated some related to this.  (I would describe this as
a "bulk action", for reference; see
  http://www.chaos.org.uk/~eddy/maths/found/bulk.xhtml
for a more formal treatment of the associative (i.e. + and * but not -
and /) case.  It generalises sum and product.  I think some folk call
these monads.  However, the fact that I write stuff like that may fairly
be considered grounds for doubting my intuitions are shared by many
others.)

> We should only support integer mathematics, so division is always
> going to be integer division, which suggests that we need a modulus
> operator as well. Moreover, 1/$x will not be supported, so we can't
> implement the same behavior for $(math -,$x) as for $(math
> /,$x). While verbose, the only clean way I can think of is simply
> $(add $x,$y), $(subtract $x,$y), etc. All supporting precisely two
> arguments. Multi argument versions can be defined like
>
>  sum = $(let first rest,$1, \
>       $(if $(rest),$(add $(first),$(call sum,$(rest))), \
>                    $(first)))

which is surely somewhat expensive, since we're bouncing back and forth
between the make "interpreter" and the $(add ...) function over and over
again, instead of just taking one trip via $(math ...).  Not that performance
is likely to be a deal-breaker, here.

        Eddy.