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

math command is extremely slow -- it should be a builtin #3157

Closed
jichu4n opened this Issue Jun 19, 2016 · 85 comments

Comments

Projects
None yet
@jichu4n
Copy link

jichu4n commented Jun 19, 2016

The math builtin in fish is extremely slow (~30x slower than bash arithmetic).

Reproduction Steps:

  1. Run the math builtin for simple arithmetic (tested on fish 2.3.0):

    $ time -p fish -c 'for i in (seq 100); math "$i * 100000" > /dev/null; end'
    
  2. Test bash's builtin arithmetic:

    $ time -p bash -c 'for i in $(seq 100); do echo $(( $i * 100000 )) > /dev/null; done'
    

Expected behavior:

The execution time of the math builtin should be on the same order of magnitude as bash arithmetic.

Observed behavior:

The math builtin is ~30x slower than bash arithmetic for the simple test case above, as measured on a Raspberry Pi B+:

$ time -p fish -c 'for i in (seq 100); math "$i * 100000" > /dev/null; end'
real 5.25
user 1.98
sys 1.49

$ time -p bash -c 'for i in $(seq 100); do echo $(( $i * 100000 )) > /dev/null; done'
real 0.16
user 0.06
sys 0.00

Additional information:

I ran into this problem when developing the fish-command-timer script, which makes multiple consecutive calls to math to breakdown a timestamp difference into hours, minutes and seconds. On slower hardware such as the Raspberry Pi, the multiple math invocations take ~1s to execute, which renders the experience painful. There aren't any alternatives to the math builtin without additional dependencies.


Fish version: 2.3.0

Operating system: Arch Linux ARM for Raspberry Pi

Terminal or terminal emulator: MATE Terminal 1.12.1

@krader1961 krader1961 added this to the fish-tank milestone Jun 19, 2016

@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented Jun 19, 2016

That's because math isn't a builtin:

$ type -t math
function

Specifically, it's a function that wraps the bc command. There is precedent for adding new builtins in place of relying on external commands. See issue #152. Personally, I'm surprised math isn't a builtin but that probably reflects how little it's used in the core scripts. I'm in favor of making it a builtin but we'll need consensus from the core dev team before that happens.

@krader1961 krader1961 changed the title math builtin is extremely slow math command is extremely slow -- should it be a builtin? Jun 19, 2016

@floam

This comment has been minimized.

Copy link
Member

floam commented Jun 19, 2016

It seems like the kind of thing that should probably be a builtin. For both performance and the sake of getting rid of the runtime dependency of bc it'd also be nice.

@faho

This comment has been minimized.

Copy link
Member

faho commented Jun 19, 2016

Making a builtin would also work around bc's suckiness (and the other available tools also have issues) - see #1643.

@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented Jun 19, 2016

The bc code is covered by GPLv3 and fish is currently GPLv2 so there shouldn't be any legal reason we can't simply include the code with the fish source. Technically it should be straightforward to turn the bc source into a builtin. The provides maximum compatibility albeit at the cost of an implementation that includes a lot of features we wouldn't otherwise support and continuing to bear the cost of bc quirks.

The alternative is to either write an implementation from scratch (which is something a second year CS student should be able to do) or use another implementation, such as the calc command which is covered by the LGPL.

@krader1961 krader1961 modified the milestones: fish-future, fish-tank Jun 19, 2016

@floam

This comment has been minimized.

Copy link
Member

floam commented Jun 20, 2016

I think it'd be more productive to look into big integer/arbitrary precision (if that's what we need) C++ math libraries rather than trying to shove into fish an entire CLI tool written in C.

@floam

This comment has been minimized.

Copy link
Member

floam commented Jun 20, 2016

And I'm not convinced we need big ints or arbitrary precision - I suppose perhaps some platforms we are used on don't have satisfactory native types, to the point that we need that to ensure script compatibility? Either way I don't think we should worry about emulating bc. Frankly I'd say it was a mistake we wrapped bc at all. Why not just let people use bc themselves if that was all we had to offer?

@faho

This comment has been minimized.

Copy link
Member

faho commented Jun 20, 2016

The bc code is covered by GPLv3 and fish is currently GPLv2 so there shouldn't be any legal reason we can't simply include the code with the fish source

I don't think that's how it works. GPL has a "no more restrictions"-clause, and v3 has more restrictions than v2. We also can't relicense fish (or we'd have to contact every contributor, including Axel), so this is pretty much impossible.

Also, this would mean doubling down on bc's weirdness and limitations.

@jichu4n

This comment has been minimized.

Copy link

jichu4n commented Jun 20, 2016

Maybe we can use the GNU libmatheval library? It's GPL3, widely available, and handles basic arithmetic expressions (+, -, *, /, parentheses, etc). It also supports a bunch of math functions like log, exp, sqrt, sin / cos, etc. The full list is described here.

It's also pretty easy to integrate. A trivial example:

#include <matheval.h>
...
void* evaluator = evaluator_create(const_cast<char*>("2 * (3 + 5)"));
assert(evaluator != nullptr);
double result = evaluator_evaluate(evaluator, 0, nullptr, nullptr);
evaluator_destroy(evaluator);
cout << result << endl;  // prints 16

A read-evaluate loop example:

#include <matheval.h>
#include <iostream>
#include <string>
#include <algorithm>
using namespace std;

int main() {
  while (cin) {
    cout << "> ";
    string line;
    getline(cin, line);
    line.erase(remove_if(line.begin(), line.end(), ::isspace), line.end());
    if (line.empty()) {
      continue;
    }

    void* evaluator = evaluator_create(const_cast<char*>(line.c_str()));
    if (evaluator == nullptr) {
      cerr << "Invalid expression" << endl;
      continue;
    }
    double result = evaluator_evaluate(evaluator, 0, nullptr, nullptr);
    evaluator_destroy(evaluator);

    cout << result << endl;
  }
  return 0;
}
@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented Jun 20, 2016

Libmatheval looks great but it is covered by GPLv3 which, as noted by @faho, isn't compatible with the GPLv2 license used by fish. However, this stackexchange discussion implies that we can legally do so. In no small part because our doc_src/license.hdr states "This code is licensed under the LGPL, version 2 or later.".

@floam

This comment has been minimized.

Copy link
Member

floam commented Jun 20, 2016

Would license compatibility even be an issue if we're just linking to a library?

@jichu4n

This comment has been minimized.

Copy link

jichu4n commented Jun 20, 2016

Ouch - looks like the GPLv3 license is a dealbreaker :(

  1. Turns out that the GPLv3 FAQs state quite clearly that a program that links against a GPLv3 library, statically or dynamically, is considered a combined work and would be covered by GPLv3.
  2. In the same doc, they also explain that it's not possible to create a combined work from GPLv2 and GPLv3 code.
  3. Unfortunately, fish's license.hdr only says that the main code is released under GPLv2, without the "or later" clause.

Putting it all together, it looks like it's not possible for fish to link against a GPLv3 library. Is that correct?

@ridiculousfish

This comment has been minimized.

Copy link
Member

ridiculousfish commented Jun 20, 2016

Correct, no GPLv3 in fish. Incorporating GPLv3 code would require that we can no longer license the rest of fish under GPLv2

@floam

This comment has been minimized.

Copy link
Member

floam commented Jun 20, 2016

Lazy Googling suggests ev3 is a C++ library that has a more permissive license that may be a libmatheval alternative.

@imgx64

This comment has been minimized.

Copy link

imgx64 commented Jun 27, 2016

The GNU website seems to be wrong about bc's license. bc is still under GPLv2+ simply because the latest release and even the latest alpha release were released before GPLv3 was released (2007), and so the code still says:

This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; either version 2 of the License , or
(at your option) any later version.
@faho

This comment has been minimized.

Copy link
Member

faho commented Jun 27, 2016

Lazy Googling suggests ev3 is a C++ library that has a more permissive license that may be a libmatheval alternative.

@floam: Unfortunately it's under the CPL, which is GPL-incompatible.

@jichu4n

This comment has been minimized.

Copy link

jichu4n commented Jun 30, 2016

Stumbled upon this GitHub repo which benchmarks 7 open source math expression parsers. (The author built ExprTk, one of the libraries.) The repo also helpfully lists the license of each library.

According to GNU's list, LGPLv3, CPL and CPOL are all incompatible with GPLv2, which eliminates a couple of the better libraries. MIT however is compatible.

In the remaining list, the most promising seems to be muParser (MIT license).

muParser is readily available everywhere I checked - including the official repos of Debian, Ubuntu, Fedora, openSUSE, Mageia, Gentoo, Arch, FreeBSD, NetBSD, and Homebrew.

The usage is fairly straightforward:

#include <cassert>
#include <muParser.h>

int main() {
  mu::Parser parser;
  parser.SetExpr("1 + 2 * 3");
  double result = parser.Eval();
  assert(result == 7);
}

What do you guys think?

@anordal

This comment has been minimized.

Copy link
Contributor

anordal commented Jul 15, 2016

Let me add libcalc2 (LGPL 2.1, part of calc), which strangely is not on that list.

As @tannhuber suggested in #1643, we should probably have wrapped calc instead of bc, for its better syntax and similar availability.

Wouldn't it be nice if the math and calc commands were 100% interchangeable? Right now, I don't see any reason to use math at all.

@FluxIX

This comment has been minimized.

Copy link

FluxIX commented Jul 15, 2016

GPL has a "no more restrictions"-clause, and v3 has more restrictions than v2. We also can't relicense fish (or we'd have to contact every contributor, including Axel), so this is pretty much impossible.

@faho Although it would be an effort, trying to contacting the developers would be good. A legal, restricted waiver could afford to the project the ability to decide how it should be licensed, now and the future, or simply a statement that any code committed into the project needs to be licensed under a given license. In the past when projects have tried to contact their contributors, I believe a good-faith, best-effort to contact their contributors was all that was required (given how people change emails and on-line presences, it is to be expected not everyone will be respond).

@floam

This comment has been minimized.

Copy link
Member

floam commented Sep 17, 2016

There's also "would we really want fish to be GPL3" - I don't know and wouldn't presume to know what others think but I don't think it's the case that we're currently stuck with a stifling license in the grand scheme of things. I mean the "problem": a shell may need to have the smart people working on it implement arithmetic from scratch because something off-the-shelf wasn't compatible -- boo-hoo. To me it's not really an exigent issue.

@kbauer

This comment has been minimized.

Copy link

kbauer commented Nov 7, 2016

Would it be viable to add math operator functions instead? E.g.

set i (+ $i 1)
set percent_left (/ (* 100 (- $total $progress)) $total)

With that, the external dependency could be avoided entirely as only number-literal parsing would be needed. The usual math symbols collide with other shell syntax though...

Regarding performance, the current implementation is especially painful on cygwin, where a subprocess invocation involves an overhead on the order of 100ms. Using the test cases from the original post, fish comes out 100 times slower than bash here:

>> time -p fish -c 'for i in (seq 100); math "$i * 100000" > /dev/null; end'
real 12.34
user 1.16
sys 6.84
>> time -p bash -c 'for i in $(seq 100); do echo $(( $i * 100000 )) > /dev/null; done'
real 0.12
user 0.01
sys 0.06
@ridiculousfish

This comment has been minimized.

Copy link
Member

ridiculousfish commented Nov 8, 2016

Introducing commands for arithmetic operators is interesting, but is quite the land-grab, and it conflicts with globbing. Arithmetic is also not common enough to justify special syntax for it in fish (like bash does with $((). Having a math command is definitely the way to go.

@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented Nov 8, 2016

Having a math command is definitely the way to go.

I agree despite being a heavy user of the $((math expr)) syntax introduced by ksh93. We should not introduce special syntax for mathematical expressions. We should instead make it less expensive, and more expressive, to perform math using the fish (command-substitution) syntax.

@floam

This comment has been minimized.

Copy link
Member

floam commented Nov 8, 2016

It's too bad we used bc instead of expr - it's a lot faster. It'd probably not be so hard to - in fish script - translate expr 5+5-3 to expr 5 + 5 - 3. We could have simply made bc be used if someone passed a flag asking for floating point operations.

I'm even half-inclined as a stop-gap to add an incr function - one that quickly takes a number and increments it. This is something we end up using bc for a lot as we have no easy way to do x++.

@floam

This comment has been minimized.

Copy link
Member

floam commented Nov 8, 2016

I kind of do like that RPN-ey math operators/functions idea. However my graphing calculator is an HP and I'm weird. Nobody will find it super intuitive. The globing thing is kind of a problem - easily worked around though by using words like mult divide pow plus, etc.

@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented Aug 23, 2017

What do we want to do about compatibility with bc's crazy rounding behavior? For example bc says 3 / 2 is 1 when the correct answer when rounded to zero decimal points is 2. And because MuParser uses floats (double actually) internally it returns 1.5 which gets rounded to 2 with the default scale precision of zero. Similarly for 10 / 6 which is 1.667 and thus rounds to 2 but bc returns 1.

krader1961 added a commit to krader1961/fish-shell that referenced this issue Aug 23, 2017

Check-in MuParser source
First step in fixing issue fish-shell#3157 is to check-in the source code and hook
it into our build system.

The inclusion of the MuParser source adds the MIT License to those that
apply to fish. Update our documentation to reflect that fact.

The MuParser documentation is at
http://beltoforion.de/article.php?a=muparser.  The source was downloaded
from https://github.com/beltoforion/muparser/releases. It is also hosted
on Github, https://github.com/beltoforion/muparser/. I did not download
it from Github because that source contained just a couple of cleanup
changes which don't affect its behavior.

krader1961 added a commit to krader1961/fish-shell that referenced this issue Aug 23, 2017

Implement bare minimum builtin `math`
This is the second baby step in resolving fish-shell#3157. Implement a bare minimum
builtin `math` command. This is solely to ensure that fish can be built
and run in the Travis build environments. This is okay since anyone running
`builtin math` today is already getting an error response.

Also, much more work is needed to support bare var references, multiple
result values, etc.
@floam

This comment has been minimized.

Copy link
Member

floam commented Aug 23, 2017

What does sqrt (2) * 2 do? A lot of my cheap calculators fail this, I hope this shouldn't.

@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented Aug 23, 2017

$ builtin math 'sqrt(2*2)'
2
$ builtin math -s 3 'sqrt(2)^2'
2.000
@floam

This comment has been minimized.

Copy link
Member

floam commented Aug 23, 2017

This is the new library? Maybe I misunderstood but I thought -s could go now.

@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented Aug 23, 2017

I thought -s could go now.

Not if we name the builtin math. Even if we gave the builtin a different name we need a mechanism to either request floating point output or force integer output since 95% of the time people using it in a fish script expect integer results.

krader1961 added a commit that referenced this issue Aug 23, 2017

Check-in MuParser source
First step in fixing issue #3157 is to check-in the source code and hook
it into our build system.

The inclusion of the MuParser source adds the MIT License to those that
apply to fish. Update our documentation to reflect that fact.

The MuParser documentation is at
http://beltoforion.de/article.php?a=muparser.  The source was downloaded
from https://github.com/beltoforion/muparser/releases. It is also hosted
on Github, https://github.com/beltoforion/muparser/. I did not download
it from Github because that source contained just a couple of cleanup
changes which don't affect its behavior.

krader1961 added a commit that referenced this issue Aug 23, 2017

Implement bare minimum builtin `math` command
This is the second baby step in resolving #3157. Implement a bare minimum
builtin `math` command. This is solely to ensure that fish can be built
and run in the Travis build environments. This is okay since anyone running
`builtin math` today is already getting an error response.

Also, more work is needed to support bare var references, multiple result
values, etc.
@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented Aug 23, 2017

From the benchmark in the opening comment of this issue we get a 46x speedup:

$ time -p fish -c 'for i in (seq 1000); math "$i * 100000"; end' >m1
real         5.09
user         2.56
sys          2.80
$ time -p ./fish -c 'for i in (seq 1000); builtin math "$i * 100000"; end' >m2
real         0.11
user         0.09
sys          0.01
@floam

This comment has been minimized.

Copy link
Member

floam commented Aug 23, 2017

How many KB does this add to a statically linked fish? Personally, I fee like fish does not net a CAS or symbolic library? Am I alone thinking such such a solution might be overkill? We only need to implement basic math operators. I feel like this is a little overkill, not our purview. We are good at writing parsers. What examples can you find of people using a symbolic math library in any shell?

Literally, I think the most common thing someone will ever do with fish is $var +1. And a few basic operations. Enough that I nearly wrote a incr builtin once.

We aren't great a lot of things but a simple, limited parser is within our purview.

@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented Aug 23, 2017

This does not implement symbolic math in the usual sense of the word such as done by Wolfram Alpha. It is just a math expression evaluator. On my Mac adding this code increases the size of the fish binary from 1577672 to 2010816 bytes. A delta of 423 KB. And we'll obviously link against a dynamic library if one is available although that is TBD by someone like @zanchey.

I guess I'm a trifle perplexed why you're just raising this concern now, @floam. This issue has been open for more than a year. And we've had extensive discussions about what to replace bc with starting moments after the issue was opened.

@floam

This comment has been minimized.

Copy link
Member

floam commented Aug 23, 2017

We didn't know until today that this would bring fish until the 2BM range. We'd be effectively looking at a 2MB shell. Have you any ideas on how to ameliorate this? PCRE was bad enough -- an originally it was brought in with the goal of significantly making it smaller for our purposes. This is getting out of Han

@floam

This comment has been minimized.

Copy link
Member

floam commented Aug 23, 2017

We don't need a symbolic math library. 2MB? Because we want basic arehmetid? QNY barely-programmable calculator would fit for most purposes. 23 just need the most basic math operations. We want a basic calulator, that's it.

@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented Aug 23, 2017

@floam, why do you keep saying "symbolic math library"? MuParser is nothing of the sort.

I built fish without muparser and both without (the normal config) and with exceptions enabled. Just enabling exceptions causes fish to grow 234 KB which is more than half the increase in size when linking in muparser. So one obvious thing to do is apply our own changes to muparser so that it doesn't throw exceptions. Since the project is in bug fix only mode and the most recent change was two years ago that is probably a reasonable thing to do. However, if we did that it would mean we couldn't use any shared muparser library installed on the system. We'd always have to use our custom statically linked version.

Lastly, with 400+ open issues writing our own math expression evaluator is not the best use of our limited resources. But nobody is going to stop you from writing one.

krader1961 added a commit that referenced this issue Aug 24, 2017

Change how `math` rounds integer results
We need our `math` builtin to behave like `bc` with respect to rounding
floating point values to integer to avoid breaking to many existing
uses. So when scale is zero round down to the nearest integer.

Another change for #3157.

krader1961 added a commit that referenced this issue Aug 24, 2017

Make builtin `math` the default implementation
Remove our `math` function that wraps `bc`. Our math builtin is now good
enough that it can be the default implementation.

Another step in resolving #3157.
@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented Aug 24, 2017

Builtin math is now the default implementation. I've removed the math function. I opted to make the builtin behave like bc and round down to the nearest integer when the scale is zero (integer results). The rounding to nearest behavior would break too many uses and probably surprise too many people.

The only other change to be done is implement support for bare vars so you can type math x + 1 rather than math $x + 1. Beyond that are things like possibly changing the muparser code to not throw exceptions to shave 200+ KB off our binary size.

@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented Aug 24, 2017

Don't forget that due to issue #4314 you'll need to manually delete the math.fish script since make install will leave it in place.

krader1961 added a commit to krader1961/fish-shell that referenced this issue Aug 24, 2017

Implement support for bare vars by `math`
This change allows you to type `math x + 3` rather than `math $x + 3`.

Another step to resolving issue fish-shell#3157.

krader1961 added a commit that referenced this issue Aug 24, 2017

Implement support for bare vars by `math`
This change allows you to type `math x + 3` rather than `math $x + 3`.

Another step to resolving issue #3157.
@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented Aug 24, 2017

This is resolved / fixed. There are other performance improvements possible but they would only benefit an insignificant number of invocations.

@krader1961 krader1961 closed this Aug 24, 2017

@alphapapa

This comment has been minimized.

Copy link

alphapapa commented Aug 24, 2017

Don't forget that due to issue #4314 you'll need to manually delete the math.fish script since make install will leave it in place.

Should this be documented? Please ignore if this has already been done. :)

@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented Aug 24, 2017

Okay, the new implementation was already much faster but I've pushed a change to add an expression LRU cache. For pathological benchmarks like the one below the new implementation is 98% faster (two orders of magnitude):

$ time -p fish26 -c 'for i in (seq 10000); math "$i / 5"; math "$i*5"; math "$i*$i"; end' >~/m1
real       132.28
user        69.09
sys         82.23
$ time -p fish -c 'for i in (seq 10000); math "i / 5"; math "i*5"; math "i*i"; end' >~/m2
real         2.43
user         1.77
sys          0.65

And we get some handy new capabilities like min() and max() functions:

$ math 'min(9, 3, 5)'
3
$ math 'max(9, 3, 5)'
9
@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented Aug 30, 2017

I see from the conversation on Gitter there are some misconceptions about why I merged a math replacement based on MuParser. First, I did not choose it because it supports trigonometric operations and similar features which should probably never be used in a shell script. Second, I keep seeing people refer to MuParser as a "symbolic math library" when it is nothing of the sort. It is a "CAS" (computer algebra system) implementation using a very generous definition of the term based on the Wikipedia definition. But I didn't choose it based on that criteria either.

There were also many comments about the paucity of math uses in fish script. And therefore there is no reason to make them fast. I can only say that I've deliberately avoided using math because I knew it was hideously expensive. My changes to the umask function to make it work correctly would have been considerably simpler had the new MuParser based math command been available.

I chose the MuParser library because, out of all the third-party libraries that had been discussed, it was the only one with a) a compatible license and b) was the only one that was borderline trivial to integrate with fish, and c) satisfy our requirements. I would have loved to use libcalc2 instead given its implementation in C rather than C++. But it isn't a practical option until someone updates it to use autoconf. FYI, I sent an email to the owner of that project and they responded that they would be happy to see me (or someone) do so:

From: Landon Curt Noll calc-tester-owner@asthe.com

Hello Kurtis,

Yes, we are interested in converting to use autoconf. Calc predated autoconf and it has been a calc TODO to consider converting to use it.
Want to give it a try?

chongo () /\oo/\

After the brouhaha over the addition of MuParser as a requirement to use fish 3.0 I spent a large number of hours trying to remove the dependency on C++ exceptions. You can read the outcome here. I still have that branch if anyone wants to take a look. And having made that attempt I can say the MuParser code is not an example of best practices. It is in roughly the same shape as the fish code when I first looked at it in October 2015. That is, the style is all over the map and it has tons of lint warnings. But it does seem to be correct and produce the correct results. I would not have used it if it had not seemed like the only viable third-party library.

Personally I would love to see someone write an equivalent implementation from scratch that does not use C++ exceptions. Also, an implementation which is tightly integrated with fish like my argparse builtin so that we don't have to do things like register a callback for looking up variable values. That would also allow supporting patterns like math 'min(x)' where the var x has more than one value. Anyone willing to commit to do that work?

@krader1961 krader1961 removed their assignment Aug 30, 2017

@anordal

This comment has been minimized.

Copy link
Contributor

anordal commented Aug 30, 2017

Oh my. The sort of problem rustaceans would beg for:

  • Parser
  • Library
  • No exceptions
  • C bindings
  • < 200 KiB
  • From scratch
@faho

This comment has been minimized.

Copy link
Member

faho commented Nov 13, 2018

For future visitors: We did merge a builtin math based on muparser, but in #4736 I ended up replacing it with one based on tinyexpr, because that is, as the name implies, much smaller. That means:

  • It's easier to swallow for distributions - we do include a heavily modified copy (I added trivialities like error handling)

  • It's easier to hack (~700 lines)

  • It has less features, so it's easier to document all of it (though it's not bare-bones, there's still trigonometric functions in there, and pi and e as constants, exponentiation, ....)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment