Revert "Cache math expressions"

This reverts commit 56d9134534.

An LRU cache in the shell for math seems like overkill.

README.md

@@ -74,6 +74,7 @@ Running fish requires:
   `uniq`, `wc`, and `whoami`
 * a number of common UNIX utilities:
     * `awk`
+    * `bc`, the "basic calculator" program
     * `find`
     * `grep`
     * `hostname`

README_muparser.md

@@ -0,0 +1,3 @@
+## Math expression evaluation
+
+Fish 1.x and 2.x releases relied on the `bc` command for handling math expressions via the `math` command. Starting with fish 3.0.0 fish uses the MuParser library. It's documentation is [here](http://beltoforion.de/article.php?a=muparser) and the source was downloaded from [here](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. See issue #3157 where making `math` a builtin was discussed.

doc_src/math.txt

@@ -7,25 +7,9 @@ math [-sN | --scale=N] [--] EXPRESSION
 
 \subsection math-description Description
 
-`math` is used to perform mathematical calculations. It supports all the usual operations such as addition, subtraction, etc. As well as functions like `abs()`, `sqrt()` and `log2()`. It also has several basic statistical functions like `min()`, `max`, and `avg()`. Internally all calculations are performed using floating point representation. Integer values are first converted to floating point before being used. The default output format is integer which causes floating point results to be rounded down to the nearest integer. The `--scale` option can be used to get floating point output.
+`math` is used to perform mathematical calculations. It is based on the MuParser library which is documented <a href="http://beltoforion.de/article.php?a=muparser&hl=en&p=features&s=idPageTop#idPageTop">here</a>. You can use bare variable names (i.e., without the dollar-sign).  The stock MuParser does not support the modulo, `%` operator but fish implements it using integer semantics.
 
-The `math` command is based on the MuParser library which is documented <a href="http://beltoforion.de/article.php?a=muparser&hl=en&p=features&s=idPageTop#idPageTop">here</a>. The stock MuParser does not support the modulo, `%`, operator but fish implements it using integer semantics.
+Keep in mind that parameter expansion takes place on any expressions before they are evaluated. This can be very useful in order to perform calculations involving shell variables or the output of command substitutions, but it also means that parenthesis and the asterisk glob character have to be escaped.
-
-You can, and should, use bare variable names (i.e., without the dollar-sign). If you use the dollar-sign form the fish parser will expand the variable and you will greatly reduce the effectiveness of the expression cache. In other words do this:
-
-\fish
-math x + 1
-\endfish
-
-rather than this:
-
-\fish
-math $x + 1
-\endfish
-
-Note that if the variable has more than one element (e.g., `set x 1 2 3`) only the first element is used in the expression when you use the bare var name. But if you instead write `math $x + 1` all three elements will be inserted yielding an invalid expression.
-
-Keep in mind that parameter expansion takes before expressions are evaluated. This can be very useful in order to perform calculations involving shell variables or the output of command substitutions, but it also means that parenthesis and the asterisk glob character have to be escaped or quoted.
 
 The `math` command can evaluate multiple expressions separated by commas. The result of each expression is written on a separate line. This means you can evaluate multiple expressions and capture the results in a single invocation just like you can with commands like `string`. See the examples below.
 
@@ -49,24 +33,14 @@ If the expression is successfully evaluated the return `status` is zero (success
 
 `math -s3 10 / 6` outputs `1.666`.
 
-Capture the result of three expressions:
+Capture the result of three expressions: `set results (math '1+1, 5*3, 10^2')` sets `$results` to 2, 15, and 100.
 
-\fish
+\subsection math-cautions Cautions
-$ set x 5
+
-$ set results (math 'x+x, x*3, x^2')
+You don't need to use `--` before the expression even if it begins with a minus sign which might otherwise be interpreted as an invalid option.
-$ set --show results
+
-$results: not set in local scope
+Note that the modulo operator (`x % y`) is not well defined for floating point arithmetic. Fish rounds down all floating point values to nearest int before performing the modulo operation. So `10.5 % 6.1` is `4`.
-$results: set in global scope, unexported, with 3 elements
-$results[1]: length=2 value=|10|
-$results[2]: length=2 value=|15|
-$results[3]: length=2 value=|25|
-$results: not set in universal scope
-\endfish
 
 \subsection math-notes Compatibility notes
 
-Fish 1.x and 2.x releases relied on the `bc` command for handling `math` expressions. Starting with fish 3.0.0 fish uses the MuParser library and evaluates the expression without the involvement of any external commands.
+Fish 1.x and 2.x releases relied on the `bc` command for handling math expressions via the `math` command. Starting with fish 3.0.0 fish uses the MuParser library.
-
-You don't need to use `--` before the expression even if it begins with a minus sign which might otherwise be interpreted as an invalid option. If you do insert `--` before the expression it will cause option scanning to stop just like for every other command and it won't be part of the expression.
-
-Note that the modulo operator (`x % y`) is not well defined for floating point arithmetic. Fish rounds down all floating point values to the nearest integer before performing the modulo operation. So `10.5 % 6.1` is `4`. Regardless of what `--scale` value is in effect.

src/builtin_math.cpp

@@ -12,7 +12,6 @@
 #include "common.h"
 #include "fallback.h"  // IWYU pragma: keep
 #include "io.h"
-#include "lru.h"
 #include "wgetopt.h"
 #include "wutil.h"  // IWYU pragma: keep
 
@@ -32,23 +31,6 @@ static const struct woption long_options[] = {{L"scale", required_argument, NULL
                                               {L"help", no_argument, NULL, 'h'},
                                               {NULL, 0, NULL, 0}};
 
-class expression_lru_item_t {
-  public:
-    wcstring expression;
-    mu::Parser parser;
-};
-
-class expression_cache_t : public lru_cache_t<expression_cache_t, mu::Parser> {
-#if 0
-    typedef lru_cache_t<wcstring, mu::Parser> super;
-  public:
-    using super::super;
-#endif
-};
-
-//expression_cache_t expression_cache(128);
-expression_cache_t expression_cache;
-
 static int parse_cmd_opts(math_cmd_opts_t &opts, int *optind,  //!OCLINT(high ncss method)
                           int argc, wchar_t **argv, parser_t &parser, io_streams_t &streams) {
     const wchar_t *cmd = L"math";
@@ -186,29 +168,20 @@ static double moduloOperator(double v, double w) { return (int)v % std::max(1, (
 static int evaluate_expression(wchar_t *cmd, parser_t &parser, io_streams_t &streams,
                                math_cmd_opts_t &opts, wcstring &expression) {
     UNUSED(parser);
-
+    next_result = 0;
-    mu::Parser *p = expression_cache.get(expression);
-    if (!p) {
-        mu::Parser parser;
-        // Setup callback so variables can be retrieved dynamically.
-        parser.SetVarFactory(retrieve_var, nullptr);
-        // MuParser doesn't implement the modulo operator so we add it ourselves since there are
-        // likely users of our old math wrapper around bc that expect it to be available.
-        parser.DefineOprtChars(L"%");
-        parser.DefineOprt(L"%", moduloOperator, mu::prINFIX);
-
-        parser.SetExpr(expression);
-        expression_cache.insert(expression, parser);
-        p = expression_cache.get(expression);
-        debug(3, L"math expression_cache.size() = %d", expression_cache.size());
-    }
 
     try {
-        p->ClearVar();  // force muparser to ask for new values of any bare vars
+        mu::Parser p;
-        next_result = 0;
+        // Setup callback so variables can be retrieved dynamically.
+        p.SetVarFactory(retrieve_var, nullptr);
+        // MuParser doesn't implement the modulo operator so we add it ourselves since there are
+        // likely users of our old math wrapper around bc that expect it to be available.
+        p.DefineOprtChars(L"%");
+        p.DefineOprt(L"%", moduloOperator, mu::prINFIX);
 
+        p.SetExpr(expression);
         int nNum;
-        mu::value_type *v = p->Eval(nNum);
+        mu::value_type *v = p.Eval(nNum);
         for (int i = 0; i < nNum; ++i) {
             if (opts.scale == 0) {
                 streams.out.append_format(L"%ld\n", static_cast<long>(v[i]));