Skip to content

Sema: return undefined on comparison of runtime value against undefined #21428

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
Sep 17, 2024

Conversation

mlugg
Copy link
Member

@mlugg mlugg commented Sep 15, 2024

I can't find a corresponding issue for this -- if anyone knows of any, note them down here and I'll close them with this PR.

@mlugg mlugg enabled auto-merge (rebase) September 15, 2024 18:18
@Rexicon226
Copy link
Contributor

if anyone knows of any, note them down here and I'll close them with this PR

#17798 ?

@SilasLock
Copy link

SilasLock commented Sep 15, 2024

Potentially #10703 could also be closed by this, since it describes the following "expected" behavior for comparisons between runtime values and undefined:

Thus, any comparison with undefined should be invalid.

... which is inconsistent with #1947, and this PR makes undefined comparisons (like other operations without side effects where at least one of the operands is undefined) work the way #1947 says they should? On the other hand, it also describes a secondary problem (i.e. logging the results of a comparison between a runtime undefined value and undefined) that to my knowledge isn't strictly covered by this PR.

Let me know if I'm off base, though.

@mlugg mlugg force-pushed the compare-to-undef branch 5 times, most recently from 0b7e674 to a5c9221 Compare September 16, 2024 16:11
There is one minor language change here, which is that comparisons of
the form `comptime_inf < runtime_f32` have their results comptime-known.
This is consistent with comparisons against comptime NaN for instance,
which are always comptime known. A corresponding behavior test is added.

This fixes a bug with int comparison elision which my previous commit
somehow triggered. `Sema.compareIntsOnlyPossibleResult` is much cleaner
now!
@mlugg mlugg disabled auto-merge September 17, 2024 10:55
@mlugg mlugg enabled auto-merge September 17, 2024 10:55
@mlugg mlugg merged commit 41330c9 into ziglang:master Sep 17, 2024
10 checks passed
DivergentClouds pushed a commit to DivergentClouds/zig that referenced this pull request Sep 24, 2024
Sema: return undefined on comparison of runtime value against undefined
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.

3 participants