fix: unify NaN checks across all arithmetic evaluation paths#955
Merged
stephenamar-db merged 2 commits intoJun 18, 2026
Merged
Conversation
He-Pin
force-pushed
the
fix/nan-comprehension-consistency
branch
from
1b7b098 to
c9da3ad
Compare
Motivation: NaN handling was inconsistent across four arithmetic evaluation paths: the main evaluator, comprehension fast path, inline optimizer, and double-fast-path. Some paths detected NaN results and errored, while others silently propagated NaN values. Modification: - Added NaN result checks to OP_+, OP_-, OP_* in all four arithmetic paths - Added NaN checks to OP_% and OP_/ where they were previously missing - All paths now consistently report "not a number" when arithmetic produces NaN - Improved NaN guards test with precise overflow assertions Result: All arithmetic evaluation paths now consistently detect and report NaN results, eliminating silent NaN propagation.
He-Pin
force-pushed
the
fix/nan-comprehension-consistency
branch
from
c9da3ad to
22e3077
Compare
Motivation: The comprehension fast path (evalBinaryOpNumNum) had NaN checks for OP_+, OP_-, and OP_* but was missing them for OP_/ and OP_%. This left the very inconsistency the PR aims to fix: Infinity / Infinity and Infinity % Infinity would silently produce NaN in comprehensions. Modification: - Add NaN check after ld / rd in OP_/, before the existing Infinity check - Add NaN check after ld % rd in OP_% Result: All four arithmetic evaluation paths now consistently detect NaN from division and modulo operations, including in array comprehensions.
stephenamar-db
approved these changes
Jun 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
NaN handling was inconsistent across four arithmetic evaluation paths: the main evaluator, comprehension fast path, inline optimizer, and double-fast-path. Some paths detected NaN results and errored, while others silently propagated NaN values.
Modification
OP_+,OP_-,OP_*in all four arithmetic pathsOP_%andOP_/where they were previously missingInfinity + (-Infinity),0 * Infinity)Result
Arithmetic operations that produce NaN now consistently error across all evaluation paths, eliminating the behavioral inconsistency between array comprehension and top-level evaluation.
References
local x = std.pow(2, 1024); x + (-x)local x = std.pow(2, 1024); 0 * x1 + 2333✅3✅