Sema: rewrite comptime arithmetic by mlugg · Pull Request #23177 · ziglang/zig

mlugg · 2025-03-10T04:17:24Z

Hey @kristoff-it, this is what that "simple little one-line fix" evolved into! Isn't that fun?

This commit reworks how Sema handles arithmetic on comptime-known values, fixing many bugs in the process.

The general pattern is that arithmetic on comptime-known values is now handled by the new namespace Sema.arith. Functions handling comptime arithmetic no longer live on Value; this is because some of them can emit compile errors, so some can't go on Value. Only semantic analysis should really be doing arithmetic on Values anyway, so it makes sense for it to integrate more tightly with Sema.

This commit also implements more coherent rules surrounding how undefined interacts with comptime and mixed-comptime-runtime arithmetic. The rules are as follows.

If an operation cannot trigger Illegal Behavior, and any operand is undefined, the result is undefined. This includes operations like 0 *| undef, where the LHS logically could be used to determine a defined result. This is partly to simplify the language, but mostly to permit codegen backends to represent undefined values as completely invalid states.
If an operation can trigger Illegal Behvaior, and any operand is undefined, then Illegal Behavior results. This occurs even if the operand in question isn't the one that "decides" illegal behavior; for instance, undef / 1 triggers IB. This is for the same reasons as described above.
An operation which would trigger Illegal Behavior, when evaluated at comptime, instead triggers a compile error. Additionally, if one operand is comptime-known undef, such that the other (runtime-known) operand isn't needed to determine that Illegal Behavior would occur, the compile error is triggered.
The only situation in which an operation with one comptime-known operand has a comptime-known result is if that operand is undefined, in which case the result is either undefined or a compile error per the above rules. This could potentially be loosened in future (for instance, 0 * rt could be comptime-known 0 with a runtime assertion that rt is not undefined), but at least for now, defining it more conservatively simplifies the language and allows us to easily change this in future if desired.

This commit fixes many bugs regarding the handling of undefined, particularly in vectors. Along with a collection of smaller tests, two very large test cases are added to check arithmetic on undefined.

The operations which have been rewritten in this PR are:

+, +%, +|, @addWithOverflow
-, -%, -|, @subWithOverflow
*, *%, *|, @mulWithOverflow
/, @divFloor, @divTrunc, @divExact
%, @rem, @mod

Other arithmetic operations are currently unchanged.

Resolves: #22743
Resolves: #22745
Resolves: #22748
Resolves: #22749
Resolves: #22914

test/behavior.zig

andrewrk

This could potentially be loosened in future (for instance, 0 * rt could be comptime-known 0 with a runtime assertion that rt is not undefined),

I suspect this will be a desirable change in the future, but, let's proceed with this at least for now.

Thanks for doing a thorough investigation and doing all this work to make the semantics consistent.

This commit reworks how Sema handles arithmetic on comptime-known values, fixing many bugs in the process. The general pattern is that arithmetic on comptime-known values is now handled by the new namespace `Sema.arith`. Functions handling comptime arithmetic no longer live on `Value`; this is because some of them can emit compile errors, so some *can't* go on `Value`. Only semantic analysis should really be doing arithmetic on `Value`s anyway, so it makes sense for it to integrate more tightly with `Sema`. This commit also implements more coherent rules surrounding how `undefined` interacts with comptime and mixed-comptime-runtime arithmetic. The rules are as follows. * If an operation cannot trigger Illegal Behavior, and any operand is `undefined`, the result is `undefined`. This includes operations like `0 *| undef`, where the LHS logically *could* be used to determine a defined result. This is partly to simplify the language, but mostly to permit codegen backends to represent `undefined` values as completely invalid states. * If an operation *can* trigger Illegal Behvaior, and any operand is `undefined`, then Illegal Behavior results. This occurs even if the operand in question isn't the one that "decides" illegal behavior; for instance, `undef / 1` is undefined. This is for the same reasons as described above. * An operation which would trigger Illegal Behavior, when evaluated at comptime, instead triggers a compile error. Additionally, if one operand is comptime-known undef, such that the other (runtime-known) operand isn't needed to determine that Illegal Behavior would occur, the compile error is triggered. * The only situation in which an operation with one comptime-known operand has a comptime-known result is if that operand is undefined, in which case the result is either undefined or a compile error per the above rules. This could potentially be loosened in future (for instance, `0 * rt` could be comptime-known 0 with a runtime assertion that `rt` is not undefined), but at least for now, defining it more conservatively simplifies the language and allows us to easily change this in future if desired. This commit fixes many bugs regarding the handling of `undefined`, particularly in vectors. Along with a collection of smaller tests, two very large test cases are added to check arithmetic on `undefined`. The operations which have been rewritten in this PR are: * `+`, `+%`, `+|`, `@addWithOverflow` * `-`, `-%`, `-|`, `@subWithOverflow` * `*`, `*%`, `*|`, `@mulWithOverflow` * `/`, `@divFloor`, `@divTrunc`, `@divExact` * `%`, `@rem`, `@mod` Other arithmetic operations are currently unchanged. Resolves: ziglang#22743 Resolves: ziglang#22745 Resolves: ziglang#22748 Resolves: ziglang#22749 Resolves: ziglang#22914

mlugg · 2025-03-16T03:03:54Z

Latest push was just to fix conflict; CI should pass.

S-YOU · 2025-03-17T04:59:40Z

src/Sema/arith.zig

+) CompileError!Value {
+    const zcu = sema.pt.zcu;
+    if (lhs.isUndef(zcu)) return lhs;
+    if (lhs.isUndef(zcu)) return rhs;


Is those supposed to be rhs on second line? @mlugg

Oh, yes it is, thank you. I'll open a follow-up soon to fix that typo.

…changes, might be forgotten

This is a complementary PR to #23487 (I had only found one typo before). Now I've looked at the whole `arith.zig` file, trying to find other potential problems. Discussion about these changes: #23177 (comment)

This commit expands on the foundations laid by ziglang#23177 and moves even more `Sema`-only functionality from `Value` to `Sema.arith`. Specifically all shift and bitwise operations, `@truncate`, `@bitReverse` and `@byteSwap` have been moved and adapted to the new rules around `undefined`. Especially the comptime shift operations have been basically rewritten, fixing many open issues in the process. Additionally this commit canonicalizes the representation of aggregates with all-undefined members in the `InternPool` by disallowing them and enforcing the usage of a single typed `undef` value instead. This reduces the amount of edge cases and fixes a bunch of bugs related to partially undefined vecs. List of operations directly affected by this patch: * `<<`, `<<|`, `@shlExact`, `@shlWithOverflow` * `>>`, `@shrExact` * `&`, `|`, `~`, `^` and their atomic rmw pendants * `@truncate`, `@bitReverse`, `@byteSwap` Resolves ziglang#16466 `*` Resolves ziglang#21266 Resolves ziglang#21943 Resolves ziglang#23034 Resolves ziglang#24392 `*` (was already kind of resolved before this patch I think, but addresses the comment)

This commit expands on the foundations laid by ziglang#23177 and moves even more `Sema`-only functionality from `Value` to `Sema.arith`. Specifically all shift and bitwise operations, `@truncate`, `@bitReverse` and `@byteSwap` have been moved and adapted to the new rules around `undefined`. Especially the comptime shift operations have been basically rewritten, fixing many open issues in the process. New rules applied to operators: * `<<`, `@shlExact`, `@shlWithOverflow`, `@shrExact`: compile error if any operator is undef * `<<|`, `>>`, all other affected ops: return undef if any operator is undef Additionally this commit canonicalizes the representation of aggregates with all-undefined members in the `InternPool` by disallowing them and enforcing the usage of a single typed `undef` value instead. This reduces the amount of edge cases and fixes a bunch of bugs related to partially undefined vecs. List of operations directly affected by this patch: * `<<`, `<<|`, `@shlExact`, `@shlWithOverflow` * `>>`, `@shrExact` * `&`, `|`, `~`, `^` and their atomic rmw pendants * `@truncate`, `@bitReverse`, `@byteSwap` Resolves ziglang#16466 `*` Resolves ziglang#21266 Resolves ziglang#21943 Resolves ziglang#23034 Resolves ziglang#24392 `*` (was already kind of resolved before this patch I think, but addresses the comment)

This commit expands on the foundations laid by ziglang#23177 and moves even more `Sema`-only functionality from `Value` to `Sema.arith`. Specifically all shift and bitwise operations, `@truncate`, `@bitReverse` and `@byteSwap` have been moved and adapted to the new rules around `undefined`. Especially the comptime shift operations have been basically rewritten, fixing many open issues in the process. New rules applied to operators: * `<<`, `@shlExact`, `@shlWithOverflow`, `@shrExact`: compile error if any operator is undef * `<<|`, `>>`, all other affected ops: return undef if any operator is undef Additionally this commit canonicalizes the representation of aggregates with all-undefined members in the `InternPool` by disallowing them and enforcing the usage of a single typed `undef` value instead. This reduces the amount of edge cases and fixes a bunch of bugs related to partially undefined vecs. List of operations directly affected by this patch: * `<<`, `<<|`, `@shlExact`, `@shlWithOverflow` * `>>`, `@shrExact` * `&`, `|`, `~`, `^` and their atomic rmw pendants * `@truncate`, `@bitReverse`, `@byteSwap`

This commit expands on the foundations laid by ziglang#23177 and moves even more `Sema`-only functionality from `Value` to `Sema.arith`. Specifically all shift and bitwise operations, `@truncate`, `@bitReverse` and `@byteSwap` have been moved and adapted to the new rules around `undefined`. Especially the comptime shift operations have been basically rewritten, fixing many open issues in the process. New rules applied to operators: * `<<`, `@shlExact`, `@shlWithOverflow`, `>>`, `@shrExact`: compile error if any operand is undef * `<<|`, `~`, `^`, `@truncate`, `@bitReverse`, `@byteSwap`: return undef if any operand is undef * `&`, `|`: Return undef if both operands are undef, turn undef into actual `0xAA` bytes otherwise Additionally this commit canonicalizes the representation of aggregates with all-undefined members in the `InternPool` by disallowing them and enforcing the usage of a single typed `undef` value instead. This reduces the amount of edge cases and fixes a bunch of bugs related to partially undefined vecs. List of operations directly affected by this patch: * `<<`, `<<|`, `@shlExact`, `@shlWithOverflow` * `>>`, `@shrExact` * `&`, `|`, `~`, `^` and their atomic rmw + reduce pendants * `@truncate`, `@bitReverse`, `@byteSwap`

andrewrk · 2025-08-19T17:45:52Z

Skipping these release notes; they don't look directly copy pastable. Please add them if you want them mentioned

mlugg added breaking Implementing this issue could cause existing code to no longer compile or have different behavior. release notes This PR should be mentioned in the release notes. labels Mar 10, 2025

alexrp reviewed Mar 10, 2025

View reviewed changes

test/behavior.zig Outdated Show resolved Hide resolved

mlugg force-pushed the comptime-arith-rewrite branch 3 times, most recently from 63d6140 to e5f0c5d Compare March 11, 2025 21:36

andrewrk approved these changes Mar 13, 2025

View reviewed changes

mlugg force-pushed the comptime-arith-rewrite branch from e5f0c5d to 42ea2f1 Compare March 16, 2025 03:03

mlugg enabled auto-merge (rebase) March 16, 2025 03:03

mlugg merged commit 2a4e06b into ziglang:master Mar 16, 2025
9 checks passed

rootbeer mentioned this pull request Mar 16, 2025

std.math.isInf either should support ComptimeFloat or should have a clearer error message #22107

Closed

S-YOU reviewed Mar 17, 2025

View reviewed changes

mlugg pushed a commit that referenced this pull request Apr 7, 2025

#23177, maintainter 'mlugg' wanted to fix that typo, 4 weeks without …

4995509

…changes, might be forgotten

munmaks mentioned this pull request Apr 8, 2025

Sema/arith.zig: Fixing more typos from #23177. #23505

Merged

mlugg deleted the comptime-arith-rewrite branch May 18, 2025 20:00

mlugg mentioned this pull request Jul 30, 2025

Fix Sema unreachable when shfting by vector containing undef elems #24396

Closed

Justus2308 mentioned this pull request Aug 3, 2025

Sema: Improve comptime arithmetic undef handling #24674

Merged

Justus2308 mentioned this pull request Sep 18, 2025

Proposal: make packed undefined semantics consistent with those of integers #25279

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Sema: rewrite comptime arithmetic#23177

Sema: rewrite comptime arithmetic#23177
mlugg merged 1 commit intoziglang:masterfrom
mlugg:comptime-arith-rewrite

mlugg commented Mar 10, 2025 •

edited

Loading

Uh oh!

Uh oh!

andrewrk left a comment

Uh oh!

mlugg commented Mar 16, 2025

Uh oh!

Uh oh!

S-YOU Mar 17, 2025

Uh oh!

mlugg Mar 17, 2025

Uh oh!

andrewrk commented Aug 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

mlugg commented Mar 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

andrewrk left a comment

Choose a reason for hiding this comment

Uh oh!

mlugg commented Mar 16, 2025

Uh oh!

Uh oh!

S-YOU Mar 17, 2025

Choose a reason for hiding this comment

Uh oh!

mlugg Mar 17, 2025

Choose a reason for hiding this comment

Uh oh!

andrewrk commented Aug 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mlugg commented Mar 10, 2025 •

edited

Loading