Uh oh!
There was an error while loading. Please reload this page.
Remove InternPool.Key.runtime_value, clean up Sema value resolution functions - #17691
Merged
Conversation
mluggforce-pushed
the
no-interned-runtime-value
branch
from
October 24, 2023 06:46
4b9f667 to
cf7be45CompareThe main goal of this commit is to remove the `runtime_value` field from `InternPool.Key` (and its associated representation), but there are a few dominos. Specifically, this mostly eliminates the "maybe runtime" concept from value resolution in Sema: so some resolution functions like `resolveMaybeUndefValAllowVariablesMaybeRuntime` are gone. This required a small change to struct/union/array initializers, to no longer use `runtime_value` if a field was a `variable` - I'm not convinced this case was even reachable, as `variable` should only ever exist as the trivial value of a global runtime `var` decl. Now, the only case in which a `Sema.resolveMaybeUndefVal`-esque function can return the `variable` key is `resolveMaybeUndefValAllowVariables`, which is directly called from `Sema.resolveInstValueAllowVariables` (previously `Sema.resolveInstValue`), which is only used for resolving the value of a Decl from `Module.semaDecl`. While changing these functions, I also slightly reordered and restructured some of them, and updated their doc comments.
Having simplified these functions in a previous commit, I felt inclined to refactor their names, which were previously quite inconsistent. There are now 4 "core" functions: * `resolveValue` (previously `resolveMaybeUndefVal`) allows runtime-known and undef values. * `resolveConstValue` (previously `resolveConstMaybeUndefVal`) allows undef but not runtime-known values. * `resolveDefinedValue` (name unchanged) allows runtime-known values but not comptime-known undef. * `resolveConstDefinedValue` (previously `resolveConstValue`) does not allow runtime-known or undef values. You can see the inconsistencies in the old names here - sometimes we specified "maybe undef", and sometimes we went the other way by specifying "defined". With the new names, the most common function, `resolveValue`, has the shortest name and does the most general thing, and is the baseline that the other functions are adding logic to. Some other functions were also renamed: * `resolveMaybeUndefLazyVal` -> `resolveValueResolveLazy` * `resolveMaybeUndefValIntable` -> `resolveValueIntable` * `resolveMaybeUndefValAllowVariables` -> `resolveValueAllowVariables`
…block This logic is not correct in most cases. If any instruction needs to operate with different semantics within `@TypeOf`, it should be made to do so explicitly. This broke a line in `std.mem`: I have opted to fix this in std for now, since as far as I know it's not yet been discussed which operations (if any) should be special-cased like this within `@TypeOf`.
mluggforce-pushed
the
no-interned-runtime-value
branch
from
October 24, 2023 13:28
cf7be45 to
f2814caCompareandrewrk
commented
Oct 24, 2023
Member
Nice changes, thank you! |
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 freeto 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.
See commit messages for details.
Depends on #17688.