Skip to content
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

Follow-up to #19414 #19437

Merged
merged 14 commits into from Mar 27, 2024
Merged

Follow-up to #19414 #19437

merged 14 commits into from Mar 27, 2024

Conversation

mlugg
Copy link
Member

@mlugg mlugg commented Mar 26, 2024

See commit messages for more details. To summarize:

  • Remove the ty field of Zcu.Decl
  • Introduce separate MutableValue representation for comptime-mutable memory
  • Eliminate legacy Value representation
  • Eliminate the "alive Decl" system
  • Eliminate TypedValue

@mlugg
Copy link
Member Author

mlugg commented Mar 26, 2024

Note to avoid blindsiding Andrew: merging this PR would implement #19435.

mlugg added 10 commits March 26, 2024 13:48
`Decl` can no longer store un-interned values, so this field is now
unnecessary. The type can instead be fetched with the new `typeOf`
helper method, which just gets the type of the Decl's `Value`.
…utable memory

Perhaps someday, we will make Sema operate on mutable values more
generally. For now, it makes sense to split out this representation,
since it is only used in comptime pointer accesses.

There are some currently unused methods on `MutableValue` which will
be used once I rewrite the comptime pointer access logic to be less
terrible.

The commit following this one will - at long last - delete the legacy
Value representation
Good riddance!

Most of these changes are trivial. There's a fix for a minor bug this
exposed in `Value.readFromPackedMemory`, but aside from that, it's all
just things like changing `intern` calls to `toIntern`.
This commit also performs some refactors to `TypedValue.print` in
preparation for improved comptime pointer access logic. Once that logic
exists, `TypedValue.print` can use Sema to access pointers for more
helpful printing.

This commit also implements proposal ziglang#19435, because the existing logic
there relied on some blatantly incorrect code in `Value.sliceLen`.

Resolves: ziglang#19435
Legacy anon decls now have three uses:
* Type owner decls
* Function owner decls
* `@export` and `@extern`

Therefore, there are no longer any cases where we wish to explicitly
omit legacy anon decls from the binary. This means we can remove the
concept of an "alive" vs "dead" `Decl`, which also allows us to remove
the separate `anon_work_queue` in `Compilation`.
Now that the legacy `Value` representation is eliminated, we can begin
to phase out the redundant `TypedValue` type.
…n decls

Also removes some unnecessary uses of legacy anon decls for constructing
the array of test functions for the test runner.
The only logic which remained in this file was the Value printing logic.
This has been moved into a new `print_value.zig`.
@mlugg mlugg force-pushed the value-cleanups branch 2 times, most recently from cacee1a to a546258 Compare March 26, 2024 15:48
Notably, this improves string printing from
`@as(*[5:0]u8, &@as([5:0]u8, "hello".*))` to `@as(*[5:0]u8, "hello")`,
omitting the pointless ref-deref pair.
Copy link
Member

@andrewrk andrewrk left a comment

Choose a reason for hiding this comment

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

So good!

I believe you can also delete Module.deleteUnusedDecl.

@mlugg
Copy link
Member Author

mlugg commented Mar 26, 2024

Oh, yep! I'll do that in a moment, and have a quick check for any more newly-unused functions.

@mlugg
Copy link
Member Author

mlugg commented Mar 26, 2024

Not "newly" unused, but there were quite a few old methods on Decl! That's handy, it means I won't have to worry about them when splitting up Decl into Nav and Cau.

@andrewrk
Copy link
Member

auto-merge

Feel free to merge when the only remaining checks are aarch64-macos. We're expecting those runners to be offline for one day.

@mlugg
Copy link
Member Author

mlugg commented Mar 26, 2024

AFAICT my permissions don't allow me to manually merge through the GitHub UI. I could do it on the command-line (I assume; I can't say I've ever tried to push to master), but IME GitHub is pretty piss-poor at detecting that as a merge. Is it possible to get the relevant permission to do the merge on GitHub?

@mlugg mlugg disabled auto-merge March 27, 2024 04:10
@andrewrk andrewrk merged commit 5140f27 into ziglang:master Mar 27, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants