Zir: attach reason to block_comptime and improve corresponding error reporting
#22352
Conversation
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
mlugg
force-pushed
the
zir-comptime-reason
branch
3 times, most recently
from
49d8866 to
a2c14c6
Compare
|
You're so cool mlugg |
Member
Author
|
Glad you like them! I'll push some doc comments shortly and auto-merge. My next target in the Land of Complicated Error Messages will be |
andrewrk
added
the
release notes
To avoid this PR regressing error messages, most of the work here has gone towards improving error notes for why code was comptime-evaluated. ZIR `block_comptime` now stores a "comptime reason", the enum for which is also used by Sema. There are two types in Sema: * `ComptimeReason` represents the reason we started evaluating something at comptime. * `BlockComptimeReason` represents the reason a given block is evaluated at comptime; it's either a `ComptimeReason` with an attached source location, or it's because we're in a function which was called at comptime (and that function's `Block` should be consulted for the "parent" reason). Every `Block` stores a `?BlockComptimeReason`. The old `is_comptime` field is replaced with a trivial `isComptime()` method which returns whether that reason is non-`null`. Lastly, the handling for `block_comptime` has been simplified. It was previously going through an unnecessary runtime-handling path; now, it is a trivial sub block exited through a `break_inline` instruction. Resolves: ziglang#22296
This fixes a bug which exposed a compiler implementation detail (ZIR alloc elision). Previously, `const` declarations with a runtime-known value in a comptime scope were permitted only if AstGen was able to elide the alloc in ZIR, since the error was reported by storing to the comptime alloc. This just adds a new instruction to also emit this error when the alloc is elided.
Some sub-expressions should always be evaluated at comptime -- in particular, type expressions, e.g. `E` in `E!T`. However, bugs in this logic are easy to miss, because the parent scope is usually comptime anyway!
Most calls to `requireRuntimeBlock` in Sema are not correct. This function doesn't deal with all of them, but it does deal with ones which have, in combination with the past few commits, introduced real-world regressions. Related: ziglang#22353
mlugg
force-pushed
the
zir-comptime-reason
branch
from
a2c14c6 to
7d26b52
Compare
mlugg
force-pushed
the
zir-comptime-reason
branch
from
7d26b52 to
106df88
Compare
Member
|
I'm skipping adding release notes for this to save time, feel free to add them if you want by pushing directly to www.ziglang.org repo in the release notes branch. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
Also fixes a bug where a
constin a comptime scope was allowed to contain a runtime value depending on a seemingly arbitrary factor, exposing a compiler implementation detail.Sema.ComptimeReasonis a type which represents a possible reason to be evaluating or resolving a value at comptime. This is either something to do with a comptime-only type, or -- most commonly -- astd.zig.SimpleComptimeReason(which is a simpleu32-backed enum).Sema.BlockComptimeReasonrepresents the reason aBlockis being evaluated at comptime. It is either aSema.ComptimeReasonwith an attachedLazySrcLoc, or it is the value.inlining_parent, meaning that thisBlockis comptime-evaluated becauseblock.inlining.?.call_blockis comptime-evaluated, and that block should therefore be consulted. This value allows us to change thenote: called from herenotes on the error message, moving the ones before the "cause" of comptime evaluation to their "correct" location.ZIR
block_comptimenow embeds astd.zig.SimpleComptimeReasonin its extra data, representing the reason for comptime evaluation in this case. This hugely improves the error messages in some cases.