Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Variadic numeric comparison operators #1628

Closed
njordhov opened this issue May 24, 2020 · 6 comments
Closed

Variadic numeric comparison operators #1628

njordhov opened this issue May 24, 2020 · 6 comments

Comments

@njordhov
Copy link
Contributor

In Clarity and typical for Lisp languages, algebraic numeric operators like + and * are variadic, taking an arbitrary number of arguments. This allows for concise expressions like:

(* 2 3 5 7 11)   ;; multiply the numbers

However, Clarity's numeric comparison operators like < only support two arguments, while these typically are variadic in Lisp languages. Only is-eq is variadic.

Proposal: Make all numerical comparison operators variadic rather than limited to two arguments.

This would make Clarity more in line with other Lisp languages and support concise expressions like:

(< 1 value 9)   ;; true if the value is between 1 and 9
@psq
Copy link
Contributor

psq commented May 25, 2020

love the fact this will make the language definition more consistent, and this will make for more concise than using

(and (< 1 value) (< value 9))

and probably not too hard to implement

@jcnelson
Copy link
Member

I have pretty strong reservations against this. There's not an obvious way to deal with the case where the items in the list are computed using a function that mutates state. For example:

(define-data-var foo uint 1)
(define-private (get-inc-foo) 
  (begin
     (var-set foo (+ 1 (var-get foo)))
     (var-get foo)))
(define-public (compare-foo) (< (get-inc-foo) (get-inc-foo) (get-inc-foo)))

What does (compare-foo) evaluate to? Does < evaluate (get-inc-foo) each time in the list, or just once? Does (< (get-inc-foo) (get-inc-foo) (get-inc-foo)) expand to (and (< (get-inc-foo) (get-inc-foo)) (< (get-inc-foo) (get-inc-foo)))? In what order are (get-inc-foo) calls evaluated? There's no right or wrong answer to this, but the result is surprising regardless of the choices we make.

By restricting comparison operators to be binary operators, we get less surprises, since there's already lots of language precedent for understanding how to evaluate (<operator> <operand-1> <operand-2>): you evaluate <operand-1>, and then <operand-2>, and feed the results into the <operator> function. If you want to compare across a list of operands, you'd use fold or you'd write out the full conjunction.

@psq
Copy link
Contributor

psq commented May 30, 2020

This would not be introducing something we don't already have to be cognizant of with Clarity.

Today, I can define this:

(define-read-only (less-than (a b c))
  (and (< a b) (< b c))
)

which I can call with:

(define-data-var foo uint 1)
(define-private (get-inc-foo) 
  (begin
	(var-set foo (+ 1 (var-get foo)))
	(var-get foo)
  )
)

(define-read-only (compare-foo) (less-than (get-inc-foo) (get-inc-foo) (get-inc-foo)))

The same concern would apply, and someone doing this would have to be pretty clear on how get-inc-foo gets called to predict what is going to be executed. Not encouraged, for sure!

What it comes down to is having a strong language specification that defines how many times each function argument is evaluated, and in which order. And maybe it is, I just wouldn't know where that is. I've assumed that each parameter was evaluated once, from left to right, even if one is not being used (unless otherwise specified, i.e. and and or so far?), but this may be incorrect. And if that's not specified yet, I think it should be.

Having this specification is important for people using the language, but also, possibly down the road, if someone decides to build a new implementation of the Clarity vm (bitcoin does have more than one implementation, same for ETH) so that no matter what implementation executes the contract, you'd get the same predictable result.

Edited to add references to and and or

@njordhov
Copy link
Contributor Author

There's not an obvious way to deal with the case where the items in the list are computed using a function that mutates state.

@jcnelson Consider that the actual problem has to do with using global variables and functions that mutate state for the arguments. How can Clarity be improved to avoid this foot gun for developers?

@stale
Copy link

stale bot commented Feb 14, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Feb 14, 2021
@stale
Copy link

stale bot commented Feb 21, 2021

This issue has been automatically closed. Please reopen if needed.

@stale stale bot closed this as completed Feb 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants