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

embrace RLS even harder #2131

Merged
merged 1 commit into from
Jan 5, 2025
Merged

Conversation

xdBronch
Copy link
Contributor

@xdBronch xdBronch commented Jan 4, 2025

No description provided.

@@ -184,7 +184,7 @@ pub const Handle = struct {
/// private field
impl: struct {
/// @bitCast from/to `Status`
status: std.atomic.Value(u32) = .init(@bitCast(Status{})),
status: std.atomic.Value(u32) = .init(0),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

was a little bit iffy on this one, it arguably loses a bit of meaning but im not sure it matters too much?

Copy link
Member

Choose a reason for hiding this comment

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

initalizing to zero assumes that Status{} is all zeroes. This may not be the case in the future so I would prefer the old code.

zoir_outdated: bool = undefined,
_: u23 = undefined,
zoir_outdated: bool = false,
_: u23 = 0,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not related to RLS at all but defaulting to undefined always feels really gross to me so this is more personal opinion than an objective change, fine with reverting it along with the above change though.

Copy link
Member

Choose a reason for hiding this comment

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

This is done because the [zir;zoir]_outdated field should never be accessed before checking has_[zir;zoir]. I don't mind this change so feel free to keep it. What should be reverted however is the change from _: u23 = undefined, to _: u23 = 0,. This field should never be accessed so there is no need to initalize it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

seems reasonable, part of why i changed these was so that i could change the .init in the other comment but no need anymore

},
.child_declarations = .{
.small = [_]Declaration.OptionalIndex{.none} ** Scope.ChildDeclarations.small_size,
.small = @splat(.none),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

just wanna acknowledge how great this one is, thanks mlugg

@Techatrix Techatrix merged commit f253553 into zigtools:master Jan 5, 2025
7 checks passed
@xdBronch xdBronch deleted the push-prwwnulqkvuk branch January 5, 2025 07:53
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.

2 participants