Coverage increased (+0.002%) to 84.136% when pulling 3442d21 on itchyny:fold-modulo-constant into 80052e5 on stedolan:master.

Rebased to run tests.

I also updated the constant folding tests.

Maybe parts of your commit message could be a code comment on why infinity is returned?

Oh, hmm, maybe there's a better way to deal with division by zero / modulo zero in this function: if the divisor is zero then return a no-op block so that the caller will not constant-fold this so that the jq program can get a run-time error instead of infinity. Or maybe we should try to make this a compile-time error. We'd need some way for constant_fold() to indicate "error" and then have the callers of gen_binop() FAIL().

What do you think, @itchyny?

I agree, 100 % 0 should not be folded to infinity, it should either not be folded, or generate a compile time error

EDIT: I didn't realise INFINITY is used to signal errors to the caller when I wrote this comment, but yes, making consant_fold() return a no-op block sounds more sensible than using INFINITY, also because the way we are currently detecting division by zero with INFINITY relies on undefined behaviour.

Also (maybe it does not matter much for constant folding since you can't have a constant nan, infinity, etc) things like jv_number_value(a) % jv_number_value(b) implicitly convert double values to integer values, which is undefined behaviour if converting nan and some other values.
Since in C, division/modulo by 0 is unspecified for all values, and casting double to int is unspecified for some values, we should probably have DIV() and MOD() macros or functions that handle all the edge cases, and use them everywhere instead of bare / and % when dealing with values returned by jv_number_value().

Ahh wait, zero division is undefined behavior in C? I didn't know that.

I didn't know either! ^^

I only learned it a few days ago because OSS Fuzz tried to compile 0/0, and the constant folding triggered an UBSAN warning.

I wasn't able to reproduce the UBSAN warning locally for some reason, but I looked at the C standard, and apparently division/modulo with 0 as rhs is unspecified behaviour for all types.

Since in C, division/modulo by 0 is unspecified for all values, and casting double to int is unspecified for some values, we should probably have DIV() and MOD() macros or functions that handle all the edge cases, and use them everywhere instead of bare / and % when dealing with values returned by jv_number_value().

Another alternative solution I like is to somehow export the f_plus, f_mod, f_divide, etc functions from src/builtin.c and use those for constant folding. Those functions should already handle all the edge cases, and do type checking by themselves, so if you use them we don't even have to duplicate the type checking and error messages parts. We can just have a look up table that calls f_plus for op == '+', f_divide for op == '/', etc. and then check if the return value is invalid, and if it is use that as error message.

My preference is that constant_fold() return gen_noop() whenever it's asked to divide by zero. This will push the handling of division by literal zero into run-time if this is all we do.

We could also then handle this as a compile-time error at the call-sites for gen_binop(), though that would be a bit more complex since we would need a convention for constant_fold() to return an error that gen_binop() can return and that the call-sites can check for. Since we're close to a release, I think I'd rather do the simple thing that is pushing the error to run-time by having constant_fold() return gen_noop() when the divisor is zero.