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

Make 0 divided by 0 results in NaN consistently #2253

Merged
merged 1 commit into from
Jul 4, 2023

Conversation

itchyny
Copy link
Contributor

@itchyny itchyny commented Jan 24, 2021

0/0 is folded to nan on parsing but 0 as $x | $x/0 throws a zero-division error so 0/0 is not equal to 0 as $x | $x/0. This is not intuitive. I think 0 as $x | $x/0 should be nan as well as 0/0 is.

 % jq -n '0/0, (0 as $x | $x/0) | isnan'
true
jq: error (at <unknown>): number (0) and number (0) cannot be divided because the divisor is zero

@coveralls
Copy link

Coverage Status

Coverage increased (+0.09%) to 84.228% when pulling 1199685 on itchyny:fix-zero-division-nan into 80052e5 on stedolan:master.

@wader
Copy link
Member

wader commented Oct 22, 2022

I ran into a related issue today when using //. nan as JSON is null but is treated as true.

$ jq -n 'nan | ., if . then "true" else "false" end'
null
"true"
$ jq -n 'nan | ., (. // "alt")'
null
null

Treating it as false might be even more confusing? so maybe error is better?

@pkoppstein
Copy link
Contributor

nan as JSON is null

That is incorrect. nan prints as null, but that’s it. The only falsey values in jq are null and false. But it is true that nan and infinite are a bit weird :-)

@wader
Copy link
Member

wader commented Oct 23, 2022

nan as JSON is null

That is incorrect. nan prints as null, but that’s it. The only falsey values in jq are null and false. But it is true that nan and infinite are a bit weird :-)

Yes, sorry i was a bit unclear, that was what i meant, null is the current JSON representation of nan for jq. That was what made me confused while i was debugging some jq code, i saw null and it took quite a long while to understand that this was actually not of type-null null. But it's hard to imagine what other JSON value would be better so maybe throw error is best?

@pkoppstein
Copy link
Contributor

@wader - Making a special case for just nan // E would be very strange. Anyway, gojq and jaq both have the same semantics regarding falseyness and nan etc, and so at this point, it’s clear that the cat is out of the bag.

@wader
Copy link
Member

wader commented Oct 23, 2022

Now realized that i read the PR commit message a bit fast, I understood it as making it consistently throw error on division by zero. Yeah agree, it's probably too late to change any of this now.

@itchyny itchyny added this to the 1.7 release milestone Jun 25, 2023
@itchyny
Copy link
Contributor Author

itchyny commented Jul 4, 2023

Thank you.

@itchyny itchyny merged commit 886a9b1 into jqlang:master Jul 4, 2023
Comment on lines +383 to 385
if (jv_number_value(b) == 0.0 && jv_number_value(a) != 0.0)
return type_error2(a, b, "cannot be divided because the divisor is zero");
jv r = jv_number(jv_number_value(a) / jv_number_value(b));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is actually problematic because division by 0 is unspecified for all values in C, so 0.0/0.0 will be undefined behaviour.
If we want to make 0/0 return nan, we should hardcode that case.

diff --git a/src/builtin.c b/src/builtin.c
index 09ffc04..1b4aec2 100644
--- a/src/builtin.c
+++ b/src/builtin.c
@@ -383,8 +383,13 @@ static jv f_multiply(jq_state *jq, jv input, jv a, jv b) {
 static jv f_divide(jq_state *jq, jv input, jv a, jv b) {
   jv_free(input);
   if (jv_get_kind(a) == JV_KIND_NUMBER && jv_get_kind(b) == JV_KIND_NUMBER) {
-    if (jv_number_value(b) == 0.0 && jv_number_value(a) != 0.0)
-      return type_error2(a, b, "cannot be divided because the divisor is zero");
+    if (jv_number_value(b) == 0.0) {
+      if (jv_number_value(a) != 0.0)
+        return type_error2(a, b, "cannot be divided because the divisor is zero");
+      jv_free(a);
+      jv_free(b);
+      return jv_number(NAN);
+    }
     jv r = jv_number(jv_number_value(a) / jv_number_value(b));
     jv_free(a);
     jv_free(b);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants