-
Notifications
You must be signed in to change notification settings - Fork 675
Const expr array length #28687
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
Const expr array length #28687
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
0becbf4
to
a8b255e
Compare
5d5f10b
to
00292fd
Compare
00292fd
to
512b577
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks great.
tests/expectations/compiler/array/array_initialization_fail.out
Outdated
Show resolved
Hide resolved
tests/expectations/compiler/core/algorithms/bhp1024_commit_to_address.out
Show resolved
Hide resolved
bdc619b
to
fe3982e
Compare
mikebenfield
approved these changes
Jun 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
fe3982e
to
7357e93
Compare
7357e93
to
58fbe9f
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #28676
This PR allows array lengths to be arbitrary expressions. Of course, we still want those expression to evaluate to a literal at compile time. We error out otherwise. Due to monomorphization and loop unrolling, type checking now has to be done multiple times which is why I had to add it, along with symbol table creation, to
const_propagation_and_unrolling.rs
. This seems to work fine except for a few small issues that I had to work around such as making sure we're not emitting duplicate warnings.The default length type is
u32
. That is, if the length provided is an unsuffixed literal, we will assume it's au32
.Because types can now have expressions in them (this will also be the case when we add const generic structs), I had to add a
TypeVisitor
andTypeReconstructor
and address types where appropriate. Most of the changes happen in early passes since later passes can assume that types have been resolved (so no need to visit their expressions at that point).The interpreter now treats const generics as regular inputs so that calling generic
inline
now works fine. For that, I also had to moveProcessingScript
until afterConstPropagationAndUnrolling
. Otherwise, we were sometimes error out in the build stage ofleo test
because we were removing scripts before monomorphizing.Unsuffixied literal now get converted to type literals in const propagation when possible.