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

std.math.isnan output is different from comptime_float vs f32 #21198

Open
cowboy8625 opened this issue Aug 25, 2024 · 2 comments
Open

std.math.isnan output is different from comptime_float vs f32 #21198

cowboy8625 opened this issue Aug 25, 2024 · 2 comments
Labels
bug Observed behavior contradicts documented or intended behavior

Comments

@cowboy8625
Copy link

Zig Version

0.13.0

Steps to Reproduce and Observed Behavior

const std = @import("std");

pub fn main() !void {
    const foo = 0.0;
    std.debug.print("foo / foo std.math.isnan({d:.2}) = {any}\n", .{ foo, std.math.isNan(foo / foo) });
    const bar: f32 = 0.0;
    std.debug.print("bar / bar std.math.isnan({d:.2}) = {any}\n", .{ bar, std.math.isNan(bar / bar) });
}

OUTPUT:

foo / foo std.math.isnan(0.00) = false
bar / bar std.math.isnan(0.00) = true

Expected Behavior

I would expect comptime_float and f32 would result in the same output of true.

@cowboy8625 cowboy8625 added the bug Observed behavior contradicts documented or intended behavior label Aug 25, 2024
@ianprime0509
Copy link
Contributor

I was looking into fixing this, since I found that it's due to this logic in Sema which is short-circuiting the evaluation process to yield zero when the numerator is zero in comptime_float division (also applies to comptime_int and comptime-known integers of other types): https://github.com/ziglang/zig/blob/7d54c62c8a55240bbe144ab03c78573a344598ce/src/Sema.zig#L15155-L15205

But while I was doing that, I came across 60c2972 which added a test specifically to ensure that 0.0 / 0.0 == 0.0 with comptime_float:

zig/test/behavior/floatop.zig

Lines 1579 to 1581 in 7d54c62

test "comptime_float zero divided by zero produces zero" {
try expect((0.0 / 0.0) == 0.0);
}

It's unclear to me why this was done, since the commit message doesn't elaborate, and I don't see any mention of this special case in the langref. The existing compile error tests suggest that it is intended for other comptime_float divisions by zero to be compile errors.

The same logic has some other potentially undesired effects (test passes as of 0.14.0-dev.1304+7d54c62c8):

const std = @import("std");
const expect = std.testing.expect;

test {
    // Existing test: comptime_float 0/0 is 0
    try expect(0.0 / 0.0 == 0.0);

    // Same thing applies to comptime_int and comptime-known ints:
    try expect(0 / 0 == 0);
    try expect(@as(u32, 0) / @as(u32, 0) == 0);

    // Negative zero is lost when dividing comptime_floats:
    try expect(std.math.isNegativeZero(@as(f128, -0.0) / @as(f128, 1.0))); // OK for f128
    try expect(!std.math.isNegativeZero(@as(f128, -0.0 / 1.0))); // not for comptime_float (note isNegativeZero doesn't support comptime_float directly)
}

The langref currently says that comptime_float is supposed to have the "same precision and operations of [...] f128", but the behavior documented above and the compile error for 1.0 / 0.0 contradict this, suggesting that perhaps there was an intention for comptime_float to be more restrictive than a regular float type (however, it's worth noting that it's still possible to get comptime_float NaNs and Infs by coercing from fixed-size float types).

@ianprime0509
Copy link
Contributor

I moved my findings above to a proposal (#21205), since deciding how (or if) to fix this requires changing evidently intentional language features (as demonstrated by the linked test cases).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants