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

Remove PackedIntArray #21682

Merged
merged 5 commits into from
Oct 14, 2024

Conversation

der-teufel-programming
Copy link
Contributor

@der-teufel-programming der-teufel-programming commented Oct 12, 2024

Fixes #21676

@alexrp
Copy link
Member

alexrp commented Oct 12, 2024

Should we send it to the orphanage?

@nektro
Copy link
Contributor

nektro commented Oct 12, 2024

given that part of the reasoning for proposing its removal were design flaws i dont think that's necessary. anyone who goes looking for it will still be able to find it in git

Comment on lines 434 to 440
std.mem.writePackedInt(
u2,
&if_kind,
if_level * 2,
until_endif,
native_endian,
);
Copy link
Contributor

Choose a reason for hiding this comment

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

It's a small thing, but you could use writePackedIntNative here to save a line in each instance.

} else if (if_level == 1) {
guard_name = null;
}
switch (if_kind.get(if_level)) {
switch (std.mem.readPackedInt(u2, &if_kind, if_level * 2, native_endian)) {
Copy link
Contributor

@tgschultz tgschultz Oct 12, 2024

Choose a reason for hiding this comment

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

..and readPackedIntNative in these instances, though it hardly saves anything.

@tgschultz
Copy link
Contributor

given that part of the reasoning for proposing its removal were design flaws i don't think that's necessary. anyone who goes looking for it will still be able to find it in git

For what it is worth I agree. Anyone looking to replicate the more complicated functionality is better off implementing it from scratch using read/writePackedInt functions. I was actually going to do that, but I couldn't find any instances of anyone actually using the more complicated functionality anyway.

@@ -430,12 +430,12 @@ fn preprocessExtra(pp: *Preprocessor, source: Source) MacroError!TokenWithExpans
if_level = sum;

if (try pp.expr(&tokenizer)) {
if_kind.set(if_level, until_endif);
std.mem.writePackedIntNative(u2, &if_kind, if_level * 2, until_endif);
Copy link
Contributor

Choose a reason for hiding this comment

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

if_level * 2 will overflow if it's greater than 127. I have a PR up for aro Vexu/arocc#787 that you can grab once it's approved and passing CI (it's waiting for @splat for arrays to be available in pre-built zig).

@andrewrk andrewrk merged commit 3bf89f5 into ziglang:master Oct 14, 2024
10 checks passed
@andrewrk
Copy link
Member

Thanks!

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.

Remove PackedIntArray from std
6 participants