Skip to content

[std.fmt] Added parsing support for f80 #20462

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

Merged
merged 2 commits into from
Jul 13, 2024
Merged

Conversation

hmcty
Copy link
Contributor

@hmcty hmcty commented Jun 30, 2024

Resolves: #19263

Test plan:

  • zig test lib/std/fmt/parse_float.zig passes locally with additional f80 unit tests
  • CI passes

Copy link
Member

@tiehuis tiehuis left a comment

Choose a reason for hiding this comment

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

Looks pretty good.

Can you remove this (no longer needed) check here for float round-tripping? then I will trigger the CI:

if (@bitSizeOf(T) != 80) {

@hmcty
Copy link
Contributor Author

hmcty commented Jul 6, 2024

Addressed all comments @tiehuis

@tiehuis
Copy link
Member

tiehuis commented Jul 7, 2024

Windows round trip error:

C:\Users\ci\george2\_work\zig\zig\build-release\zig-local-cache\o\ef0547664f7b4931546bf402c80a8059\test.exe --listen=- 
test
+- test-std
   +- run test std-x86_64-windows.win11_ge...win11_ge-gnu-skylake-Debug 2657/2724 passed, 1 failed, 66 skipped
error: 'fmt.format_float.test.format f80' failed: expected 907682844368439810595840, found 907682844368439810596864
C:\Users\ci\george2\_work\zig\zig\lib\std\testing.zig:93:17: 0xe829e3 in expectEqualInner__anon_35380 (test.exe.obj)
                return error.TestExpectedEqual;
                ^
C:\Users\ci\george2\_work\zig\zig\lib\std\fmt\format_float.zig:1530:9: 0x12db79a in check__anon_46210 (test.exe.obj)
        try std.testing.expectEqual(value_bits, o_bits);
        ^
C:\Users\ci\george2\_work\zig\zig\lib\std\fmt\format_float.zig:1655:5: 0x12dcd86 in test.format f80 (test.exe.obj)
    try check(f80, -2.109808898695963e16, "-2.109808898695963e16");
    ^

I'll have a look during the week and see if I can reproduce via wine.


Linux-debug segfault as well to look at. I'll rerun given the rebase and see if this has persisted. This looks resolved after the rebase.

From https://github.com/ziglang/zig
   bf588f67d..c40708a2c  master     -> origin/master
+ git fetch --tags
+ git clean -fd
+ rm -rf zig-out
+ cc -o bootstrap bootstrap.c
+ ./bootstrap
cc -o zig-wasm2c stage1/wasm2c.c -O2 -std=c99
./zig-wasm2c stage1/zig1.wasm zig1.c
cc -o zig1 zig1.c stage1/wasi.c -std=c99 -Os -lm
./zig1 lib build-exe -ofmt=c -lc -OReleaseSmall --name zig2 -femit-bin=zig2.c -target x86_64-linux --dep build_options --dep aro --mod root src/main.zig --mod build_options config.zig --mod aro lib/compiler/aro/aro.zig
./zig1 lib build-obj -ofmt=c -OReleaseSmall --name compiler_rt -femit-bin=compiler_rt.c -target x86_64-linux --dep build_options --mod root lib/compiler_rt.zig --mod build_options config.zig
cc -o zig2 zig2.c compiler_rt.c -std=c99 -O2 -fno-stack-protector -Istage1 -Wl,-z,stack-size=0x10000000 -pthread
+ ./zig2 build -Dno-lib
+ ./zig-out/bin/zig test test/behavior.zig
Segmentation fault

@tiehuis
Copy link
Member

tiehuis commented Jul 7, 2024

Just noting I hit #17535 when trying to test under wine.

@tiehuis
Copy link
Member

tiehuis commented Jul 13, 2024

I wasn't able to diagnose a fix on the windows CI failures so have added a skip for the round-trip tests for now for this target, and will add an issue noting the failure for future. I've manually rebased your commits and will merge when CI passes.

@tiehuis tiehuis merged commit 944c6d4 into ziglang:master Jul 13, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

std.fmt.parseFloat: add f80 float parsing support
2 participants