Skip to content
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

Improve math errors #9190

Merged
merged 6 commits into from
Sep 9, 2022
Merged

Improve math errors #9190

merged 6 commits into from
Sep 9, 2022

Conversation

faho
Copy link
Member

@faho faho commented Sep 8, 2022

Description

This introduces a specific error for division/modulo by zero and augments some error reporting with the length of the location.

Examples:

> math '1 - 1      pi'
math: Error: Missing operator
'1 - 1      pi'
      ^~~~~^
> math '2 + notafunction 5'
math: Error: Unknown function
'2 + notafunction 5'
     ^~~~~~~~~~~^
> math '1 - 1 / (5 - 5)'
math: Error: Division by zero
'1 - 1 / (5 - 5)'
       ^

Note: The division-by-zero error technically changes behavior, in that it now errors out early instead of carrying on and only complaining at the end that "Result is infinite" or "not a number".

That means something like

math min 1 / 0, 5

will now error out instead of returning 5.

I believe that is the correct choice - division by zero is typically expected to not be defined.

TODOs:

  • Changes to fish usage are reflected in user documentation/manpages.
  • Tests have been added for regressions fixed
  • User-visible changes noted in CHANGELOG.rst

This errored out *later* because the result was infinite or NaN, but
it didn't actually stop evaluation.

I'm not sure if there is a way to get floating point math to turn an
infinity back into something that doesn't depend on a literal
infinity, but division by zero conceptually isn't a thing we can
support.

There's entire branches of maths dedicated to figuring out what
dividing by "basically zero" means and we don't have to get into it.
Like we now do for syntax errors, this marks the extent of the error.

Currently for unknown functions only, would be cool for division too
@faho faho added this to the fish 3.6.0 milestone Sep 8, 2022
@faho faho merged commit bc1a5ba into fish-shell:master Sep 9, 2022
@faho faho deleted the math-errors branch September 11, 2022 07:56
@ChristoferK

This comment was marked as resolved.

@faho
Copy link
Member Author

faho commented Feb 9, 2023

In the context of a scripting language, used by an ordinary person, it's true.

When they divide by zero, it's very very likely a mistake, and so the error is helpful.

If you want to "implement a min algorithm", use the "min" function. If you want to do more complicated math, use another tool. When you're doing physics, use R or something. The right tool for the job, not your shell.

It is entirely okay if fish doesn't solve all problems everywhere. Scientific calculation is a very complicated field that we're never gonna be competitive in, and this comes at the expense of detecting legitimate errors.

Sorry, I don't want to change this.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants