diff --git a/CHANGELOG.md b/CHANGELOG.md index 4989f8d7f..c781ee48e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,7 @@ fish 3.0 is a major release which brings with it both improvements in functional - Path completions now support expansions, meaning expressions like `python ~/` now provides file suggestions just like any other relative or absolute path. (This includes support for other expansions, too.) - The `string` builtin has new commands `split0` and `join0` for working with NUL-delimited output. - The `-d` option to `functions` to set the description of an existing function now works; before 3.0 it was documented but unimplemented. Note that the long form `--description` continues to work. (#5105) +- `test` and `[` now support floating point values in numeric comparisons. ## Other significant changes - Command substitution output is now limited to 10 MB by default (#3822). diff --git a/src/builtin_test.cpp b/src/builtin_test.cpp index e0f891978..00b066f97 100644 --- a/src/builtin_test.cpp +++ b/src/builtin_test.cpp @@ -10,6 +10,7 @@ #include #include +#include #include #include #include @@ -74,6 +75,36 @@ enum token_t { test_paren_close, // ")", close paren }; +/// Our number type. We support both doubles and long longs. We have to support these separately +/// because some integers are not representable as doubles; these may come up in practice (e.g. +/// inodes). +class number_t { + // A number has an integral base and a floating point delta. + // Conceptually the number is base + delta. + // We enforce the property that 0 <= delta < 1. + long long base; + double delta; + + public: + number_t(long long base, double delta) : base(base), delta(delta) { + assert(0.0 <= delta && delta < 1.0 && "Invalid delta"); + } + number_t() : number_t(0, 0.0) {} + + // Compare two numbers. Returns an integer -1, 0, 1 corresponding to whether we are less than, + // equal to, or greater than the rhs. + int compare(number_t rhs) const { + if (this->base != rhs.base) return (this->base > rhs.base) - (this->base < rhs.base); + return (this->delta > rhs.delta) - (this->delta < rhs.delta); + } + + // Return true if the number is a tty()/ + bool isatty() const { + if (delta != 0.0 || base > INT_MAX || base < INT_MIN) return false; + return ::isatty(static_cast(base)); + } +}; + static bool binary_primary_evaluate(test_expressions::token_t token, const wcstring &left, const wcstring &right, wcstring_list_t &errors); static bool unary_primary_evaluate(test_expressions::token_t token, const wcstring &arg, @@ -599,22 +630,54 @@ bool parenthetical_expression::evaluate(wcstring_list_t &errors) { return contents->evaluate(errors); } +// Parse a double from arg. Return true on success, false on failure. +static bool parse_double(const wchar_t *arg, double *out_res) { + // Consume leading spaces. + while (arg && *arg != L'\0' && iswspace(*arg)) arg++; + errno = 0; + wchar_t *end = NULL; + *out_res = wcstod_l(arg, &end, fish_c_locale()); + // Consume trailing spaces. + while (end && *end != L'\0' && iswspace(*end)) end++; + return errno == 0 && end > arg && *end == L'\0'; +} + // IEEE 1003.1 says nothing about what it means for two strings to be "algebraically equal". For // example, should we interpret 0x10 as 0, 10, or 16? Here we use only base 10 and use wcstoll, // which allows for leading + and -, and whitespace. This is consistent, albeit a bit more lenient // since we allow trailing whitespace, with other implementations such as bash. -static bool parse_number(const wcstring &arg, long long *out, wcstring_list_t &errors) { - *out = fish_wcstoll(arg.c_str()); - if (errno) { - errors.push_back(format_string(_(L"invalid integer '%ls'"), arg.c_str())); +static bool parse_number(const wcstring &arg, number_t *number, wcstring_list_t &errors) { + const wchar_t *argcs = arg.c_str(); + double floating = 0; + bool got_float = parse_double(argcs, &floating); + + errno = 0; + long long integral = fish_wcstoll(argcs); + bool got_int = (errno == 0); + if (got_int) { + // Here the value is just an integer; ignore the floating point parse because it may be + // invalid (e.g. not a representable integer). + *number = number_t{integral, 0.0}; + return true; + } else if (got_float) { + // Here we parsed an (in range) floating point value that could not be parsed as an integer. + // Break the floating point value into base and delta. Ensure that base is <= the floating + // point value. + double intpart = std::floor(floating); + double delta = floating - intpart; + *number = number_t{static_cast(intpart), delta}; + return true; + } else { + // We could not parse a float or an int. + errors.push_back(format_string(_(L"invalid number '%ls'"), arg.c_str())); + return false; } - return !errno; } static bool binary_primary_evaluate(test_expressions::token_t token, const wcstring &left, const wcstring &right, wcstring_list_t &errors) { using namespace test_expressions; - long long left_num, right_num; + number_t ln, rn; switch (token) { case test_string_equal: { return left == right; @@ -623,28 +686,28 @@ static bool binary_primary_evaluate(test_expressions::token_t token, const wcstr return left != right; } case test_number_equal: { - return parse_number(left, &left_num, errors) && - parse_number(right, &right_num, errors) && left_num == right_num; + return parse_number(left, &ln, errors) && parse_number(right, &rn, errors) && + ln.compare(rn) == 0; } case test_number_not_equal: { - return parse_number(left, &left_num, errors) && - parse_number(right, &right_num, errors) && left_num != right_num; + return parse_number(left, &ln, errors) && parse_number(right, &rn, errors) && + ln.compare(rn) != 0; } case test_number_greater: { - return parse_number(left, &left_num, errors) && - parse_number(right, &right_num, errors) && left_num > right_num; + return parse_number(left, &ln, errors) && parse_number(right, &rn, errors) && + ln.compare(rn) > 0; } case test_number_greater_equal: { - return parse_number(left, &left_num, errors) && - parse_number(right, &right_num, errors) && left_num >= right_num; + return parse_number(left, &ln, errors) && parse_number(right, &rn, errors) && + ln.compare(rn) >= 0; } case test_number_lesser: { - return parse_number(left, &left_num, errors) && - parse_number(right, &right_num, errors) && left_num < right_num; + return parse_number(left, &ln, errors) && parse_number(right, &rn, errors) && + ln.compare(rn) < 0; } case test_number_lesser_equal: { - return parse_number(left, &left_num, errors) && - parse_number(right, &right_num, errors) && left_num <= right_num; + return parse_number(left, &ln, errors) && parse_number(right, &rn, errors) && + ln.compare(rn) <= 0; } default: { errors.push_back(format_string(L"Unknown token type in %s", __func__)); @@ -657,7 +720,6 @@ static bool unary_primary_evaluate(test_expressions::token_t token, const wcstri wcstring_list_t &errors) { using namespace test_expressions; struct stat buf; - long long num; switch (token) { case test_filetype_b: { // "-b", for block special files return !wstat(arg, &buf) && S_ISBLK(buf.st_mode); @@ -704,7 +766,8 @@ static bool unary_primary_evaluate(test_expressions::token_t token, const wcstri return !wstat(arg, &buf) && buf.st_size > 0; } case test_filedesc_t: { // "-t", whether the fd is associated with a terminal - return parse_number(arg, &num, errors) && num == (int)num && isatty((int)num); + number_t num; + return parse_number(arg, &num, errors) && num.isatty(); } case test_fileperm_r: { // "-r", read permission return !waccess(arg, R_OK); diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index 103ad17b5..129c6f368 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -2247,6 +2247,18 @@ static void test_test() { // https://github.com/fish-shell/fish-shell/issues/601 do_test(run_test_test(0, L"-S = -S")); do_test(run_test_test(1, L"! ! ! A")); + + // Verify that 1. doubles are treated as doubles, and 2. integers that cannot be represented as + // doubles are still treated as integers. + do_test(run_test_test(0, L"4611686018427387904 -eq 4611686018427387904")); + do_test(run_test_test(0, L"4611686018427387904.0 -eq 4611686018427387904.0")); + do_test(run_test_test(0, L"4611686018427387904.00000000000000001 -eq 4611686018427387904.0")); + do_test(run_test_test(1, L"4611686018427387904 -eq 4611686018427387905")); + do_test(run_test_test(0, L"-4611686018427387904 -ne 4611686018427387904")); + do_test(run_test_test(0, L"-4611686018427387904 -le 4611686018427387904")); + do_test(run_test_test(1, L"-4611686018427387904 -ge 4611686018427387904")); + do_test(run_test_test(1, L"4611686018427387904 -gt 4611686018427387904")); + do_test(run_test_test(0, L"4611686018427387904 -ge 4611686018427387904")); } /// Testing colors. diff --git a/src/wutil.h b/src/wutil.h index 72ac10cbb..1641b161a 100644 --- a/src/wutil.h +++ b/src/wutil.h @@ -125,6 +125,7 @@ int fish_wcstoi(const wchar_t *str, const wchar_t **endptr = NULL, int base = 10 long fish_wcstol(const wchar_t *str, const wchar_t **endptr = NULL, int base = 10); long long fish_wcstoll(const wchar_t *str, const wchar_t **endptr = NULL, int base = 10); unsigned long long fish_wcstoull(const wchar_t *str, const wchar_t **endptr = NULL, int base = 10); +double fish_wcstod(const wchar_t *str, const wchar_t **endptr); /// Class for representing a file's inode. We use this to detect and avoid symlink loops, among /// other things. While an inode / dev pair is sufficient to distinguish co-existing files, Linux