Rework std.math.{order, Order}, and fix illegal behavior for NaN values with total ordering#23291
Rework std.math.{order, Order}, and fix illegal behavior for NaN values with total ordering#23291SeanTUT wants to merge 6 commits intoziglang:masterfrom
std.math.{order, Order}, and fix illegal behavior for NaN values with total ordering#23291Conversation
- Moved implementations of `std.math.order` and `std.math.Order` into their own file (lib/std/math/order.zig) - Floats use total ordering, eliminating the possibility of reaching unreachable code in `std.math.order` - `.lt`, `.eq`, and `.gt` now have vales of `-1`, `0`, and `1` respectively, allowing for some optimizations in ordering integers inverting orderings - `std.mem.order` now utilizes `std.math.Order.differ` instead of manually implementing the functionality - Modified up some existing tests to use `std.testing.expectEqual` instead of `std.testing.expect` when appropriate
…nto order-patch
|
To clarify, this is not a breaking change. The only change in behavior is that NaN values are ordered according to IEEE-754 total ordering (whereas ordering NaN values previously resulted in illegal behavior) |
|
I'm seeing some strange behavior in these CI failures. The error message claims that |
| const T = @TypeOf(lhs, rhs); | ||
| switch (@typeInfo(T)) { | ||
| .int, .comptime_int => { | ||
| return @enumFromInt(@as(i2, @intFromBool(lhs > rhs)) - @intFromBool(lhs < rhs)); |
There was a problem hiding this comment.
I can fix the errors by changing this back to the original implementation of std.math.order, but the fact that the error message is incorrect here is concerning
|
After some testing, I have found that this bug occurs on the stage2 compiler when using |
In math/order.zig, cast the results of the comparison to i8 before subtracting. This sidesteps the discovered bug.
This test was intended for investigating a compiler bug which will be looked into seperately
|
I have tweaked the code to sidestep the previously mentioned bug, but I will continue to look into this. |
1 participant
std.math.order/std.math.Orderinto a new file (std/math/order.zig)std.math.order(The previous implementation would cause illegal behavior any time one of the operands was nan).lt,.eq, and.gtnow have vales of-1,0, and1respectively, allowing for some optimizations in ordering integers inverting orderingsstd.mem.ordernow utilizesstd.math.Order.differinstead of manually implementing the functionalitystd.testing.expectEqualinstead ofstd.testing.expectwhen appropriatestd.math.order, and a test on total ordering for floats