Variadic functions for `math` and tinyexpr rewrite.

[juntuu]

Hi,

I've played around with the tinyexpr included in fish. I rewrote it here using more of c++ (than the very c-like code), and added variadic function support.

Changes:

• direct evaluation since fish didn't use any features (e.g. variables) that required separate parse or optimise steps
• support for variadic functions (and example functions minimum and maximum)
• functions are stored in a std::unordered_map, so that alphabetical order is no longer required and the lookup is simpler

Would this be of interest to you? If you'd like, I could open a pull request.

The current implementation of variadic functions doesn't support reporting errors from the functions, which might be good to have at some point. Also a question: Should some existing functions be made variadic, or should some variadic functions be added?

[faho]

Would this be of interest to you?

Sure, sounds cool. How's the performance?

Should some existing functions be made variadic

I think the only ones where that really makes sense are minimum and maximum, I don't think e.g. "log2" makes sense as a variadic function.

There are possibilities here for optional arguments, e.g. log(num, base) or round(num, digits), but I wouldn't add them initially because I'm not entirely sold on them.

The current implementation of variadic functions doesn't support reporting errors from the functions

I'm not sure what you mean here. Things like division by zero? E.g. the function itself makes no sense to apply?

[juntuu]

How's the performance?

Running the math benchmark shows a small speedup on my machine: 1.02 ± 0.01 times faster. Otherwise I haven't noticed any significant difference, nor would I expect to see one. I also ran the benchmark with few other expressions (using other operators and including function calls) with very similar results.

With enough arguments (1000 +) the variadic functions e.g. minimum n1,n2,...,nk start getting faster than equivalent expression using two argument functions min n1, min n2, ..., min n(k-1), nk. I would assume the difference comes from the number of function lookups and calls.

I think the only ones where that really makes sense are minimum and maximum

Do you think these are better to have separately, or changing the current min and max to be variadic?

I'm not sure what you mean here.

I was mostly thinking if the function requires at least some number of arguments, or has other limitations on the number of arguments. I agree the optional arguments might not be a good choice, but that would be an example of variadic function with extra arity requirements.

[floam]

Seems to me min and max without arguments could be like, the numeric minimum and maximum we can work with. Parens are optional anyhow - it'd feel like using a constant like pi?

[faho]

Seems to me min and max without arguments could be like, the numeric minimum and maximum we can work with

I do not see anyone querying for these, ever.

If you do math "max()" it is sooooooo much more likely that you forgot an argument (or it was empty) than that you actually wanted the maximum number math can handle, and it's not close.

If this was something we wanted to have a way to display, I wouldn't even make it a constant in math. I'd make it an option - math --print-maximum-number. But I do not see a need for that.

[floam]

Yeah good point the mode of a disappeared argument resulting in something like INT_MAX by surprise might be a good enough reason to scuttle that.

[juntuu]

I do not see anyone querying for these, ever.

The one case I can imagine, is when you get the Result magnitude is too large error.
I remember couple times doing a manual binary search for the approximate limit (before I had seen the source obviously).

The limit isn't too obvious either ± (2 ^ 53 - 1), unless you are very familiar with IEEE-754 binary64 format and know it is used in the implementation.

[ridiculousfish]

This looks great! Variadic min/max and direct evaluation is very welcome. I especially like how the direct evaluation cleans up the memory management. I look forwards to merging this after 3.4.0 release.

Please remove the unordered_map change (398bcb4). The reason is that this runs before main() so it affects launch time. Eventually we'll have some abstractions that make this stuff sane, but for now sorted lists are the way to go. Take a look at ASSERT_SORTED_BY_NAME to statically enforce that it's sorted.

[juntuu]

Please remove the unordered_map

That's understandable, removed it!

ASSERT_SORTED_BY_NAME

Cool, I wasn't aware of this. Added it (and some required constexpr specifiers).