diff --git a/CHANGELOG.md b/CHANGELOG.md index e692a8ef0..e21372ff2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ This section is for changes merged to the `major` branch that are not also merge - `bind` has a new `--silent` option to ignore bind requests for named keys not available under the current `$TERMINAL` (#4188, #4431) - `read` writes directly to stdout if called without arguments (#4407) - Globs are faster (#4579) +- "Bare variables" in math are no longer accepted. math uses normal variable expansion. (#4393) ## Other significant changes - Command substitution output is now limited to 10 MB by default (#3822). diff --git a/src/builtin_math.cpp b/src/builtin_math.cpp index 696749580..8e733b1cb 100644 --- a/src/builtin_math.cpp +++ b/src/builtin_math.cpp @@ -115,52 +115,6 @@ static const wchar_t *math_get_arg(int *argidx, wchar_t **argv, wcstring *storag return math_get_arg_argv(argidx, argv); } -// The MuParser mechanism for dynamic lookup of variables requires that we return a unique address -// for each variable. The following limit is arbitrary but anyone writing a math expression in fish -// that references more than one hundred unique variables is abusing fish. -#define MAX_RESULTS 100 -static double double_results[MAX_RESULTS]; -static int next_result = 0; - -/// Return a fish var converted to a double. This allows the user to use a bar var name in the -/// expression. That is `math a + 1` rather than `math $a + 1`. -static double *retrieve_var(const wchar_t *var_name, void *user_data) { - UNUSED(user_data); - static double zero_result = 0.0; - - auto var = env_get(var_name, ENV_DEFAULT); - if (!var) { - // We could report an error but we normally don't treat missing vars as a fatal error. - // throw mu::ParserError(L"Var '%ls' does not exist."); - return &zero_result; - } - if (var->empty()) { - return &zero_result; - } - - const wchar_t *first_val = var->as_list()[0].c_str(); - wchar_t *endptr; - errno = 0; - double result = wcstod(first_val, &endptr); - if (*endptr != L'\0' || errno) { - wchar_t errmsg[500]; - swprintf(errmsg, sizeof(errmsg) / sizeof(wchar_t), - _(L"Var '%ls' not a valid floating point number: '%ls'."), var_name, first_val); - throw mu::ParserError(errmsg); - } - - // We need to return a unique address for the var. If we used a `static double` var and returned - // it's address then multiple vars in the expression would all refer to the same value. - if (next_result == MAX_RESULTS - 1) { - wchar_t errmsg[500]; - swprintf(errmsg, sizeof(errmsg) / sizeof(wchar_t), - _(L"More than %d var names in math expression."), MAX_RESULTS); - throw mu::ParserError(errmsg); - } - double_results[next_result++] = result; - return double_results + next_result - 1; -} - /// Implement integer modulo math operator. static double moduloOperator(double v, double w) { return (int)v % std::max(1, (int)w); }; @@ -168,12 +122,9 @@ 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; try { mu::Parser p; - // 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"%"); diff --git a/tests/math.err b/tests/math.err index 240044f45..5a7ab5130 100644 --- a/tests/math.err +++ b/tests/math.err @@ -3,4 +3,4 @@ # Validate basic expressions #################### -# Validate how bare variables in an epxression are handled +# Validate how variables in an expression are handled diff --git a/tests/math.in b/tests/math.in index 8fee37df4..1e13114d7 100644 --- a/tests/math.in +++ b/tests/math.in @@ -14,11 +14,11 @@ math 5 \* -2 math -- -4 / 2 math -- '-4 * 2' -logmsg Validate how bare variables in an epxression are handled -math x + 1 +logmsg Validate how variables in an expression are handled +math $x + 1 set x 1 -math x + 1 +math $x + 1 set x 3 set y 1.5 -math '-x * y' -math -s1 '-x * y' +math "-$x * $y" +math -s1 "-$x * $y" diff --git a/tests/math.out b/tests/math.out index f0a91e8ce..df8607f71 100644 --- a/tests/math.out +++ b/tests/math.out @@ -17,7 +17,7 @@ -8 #################### -# Validate how bare variables in an epxression are handled +# Validate how variables in an expression are handled 1 2 -4