Skip to content

Sema: allow field access instructions to be emitted at comptime #15287

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Sema: allow field access instructions to be emitted at comptime #15287

wants to merge 2 commits into from

Conversation

mlugg
Copy link
Member

@mlugg mlugg commented Apr 14, 2023

Related to 34e4b07. This makes struct.arr.len succeed at comptime.

Resolves: #15280

@mlugg mlugg marked this pull request as draft April 14, 2023 17:25
@mlugg
Copy link
Member Author

mlugg commented Apr 14, 2023

Going to extend to a few other things (tuples, unions, arrays)

@mlugg mlugg force-pushed the fix/comptime-array-len branch from a3186ea to b3307a3 Compare April 14, 2023 19:05
@mlugg mlugg marked this pull request as ready for review April 14, 2023 19:05
@jayschwa
Copy link
Contributor

#14819 changed some miscellaneous length accesses to magic numbers. It might be good to revert those changes if this PR re-enables the behavior.

Example: addr.un.path.len + 1 was changed to 200: ccf670c#diff-902e94d5dd402cc20a4baa94f9b1e51b95e09437b51ca51ede4a436723eee9fbL102-R102

mlugg added 2 commits April 14, 2023 23:33
@mlugg
Sema: allow field and index accesses to be emitted at comptime
b76d2e5 
Related to 34e4b07. This makes `struct.arr.len` succeed at comptime.

Resolves: ziglang#15280
@mlugg
Sema: ensure results of comptime blocks are comptime-known
fb6ee2f 
Comptime evaluation is allowed to create intermediate runtime
instructions provided the final result is comptime-known, for accesses
such as `runtime_arr.len` or `runtime_struct.comptime_field`.
@mlugg mlugg force-pushed the fix/comptime-array-len branch from b3307a3 to fb6ee2f Compare April 14, 2023 22:33
@mlugg
Copy link
Member Author

mlugg commented Apr 14, 2023

#14819 changed some miscellaneous length accesses to magic numbers. It might be good to revert those changes if this PR re-enables the behavior.

Good shout - done.

@ reviewers: regarding the second commit, it turned out that doing this validation everywhere made a lot of compile errors worse. This makes sense, since it's more useful for e.g. switch expressions to be able to add a note telling you that switch prongs have to be comptime-known. I opted to split block_comptime into two tags: one which validates its result to be comptime-known, and one which defers that responsibility to the use site. If that isn't satisfactory, I can drop that commit for now, since it's not necessary for the first, it was just something I noticed while working on it.

Vexu
Vexu approved these changes Apr 23, 2023
View reviewed changes
Copy link
Member

@Vexu Vexu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a suggestion for a better name but the current one didn't immediately make it clear that the difference comes from explicitly using the comptime syntax.

src/Sema.zig
@@ -24126,6 +24154,8 @@ fn tupleFieldValByIndex(
field_index: u32,
tuple_ty: Type,
) CompileError!Air.Inst.Ref {
_ = src;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this parameter.

@mlugg mlugg mentioned this pull request Apr 28, 2023
Open
@andrewrk andrewrk self-requested a review May 1, 2023 17:32
@Vexu Vexu mentioned this pull request May 11, 2023
Open
@andrewrk
Copy link
Member

I'd like to give this another look - you mind rebasing and conflict fixing, @mlugg?

@mlugg
Copy link
Member Author

mlugg commented Jul 22, 2023

Sure thing. I may be a bit busy over the next couple of days with work stuff, but I'll put this on my list and try to get it done within the next week or so!

@gavo579 gavo579 mentioned this pull request Aug 25, 2023
Open
@wooster0 wooster0 mentioned this pull request Sep 18, 2023
Closed
@andrewrk
Copy link
Member

Closing abandoned PR

@andrewrk andrewrk closed this Oct 19, 2023
@mlugg mlugg deleted the fix/comptime-array-len branch May 18, 2025 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Vexu Vexu Vexu approved these changes

@andrewrk andrewrk Awaiting requested review from andrewrk

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Regression with "Unable to evaluate constant expression" in len of fixed size arrays
4 participants
@mlugg @jayschwa @andrewrk @Vexu