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

std.ArrayList(Aligned): copy default fields from Unmanaged #19169

Conversation

BratishkaErik
Copy link
Contributor

Synchronizes default fields and doc comments between Unmanaged and Managed versions:

  • Make default values for the items and capacity fields same in all ArrayList versions. This allows to initialize ArrayList "directly" (= via struct initialization) and removes inconsistency with Unmanaged:
var unmanaged: ArrayListUnmanaged(u8) = .{};
errdefer unmanaged.deinit();

// ...

var managed: ArrayList(u8) = .{ .allocator = allocator };
errdefer managed.deinit();

// ...
  • Mention "direct" initialization and initCapacity in doc comments, also removes inconsistency.

Synchronizes default fields and doc comments between Unmanaged
and Managed versions:
 * Make default values for the `items` and `capacity` fields same
   in all ArrayList versions. This allows to initialize ArrayList
   "directly" and removes inconsistency with Unmanaged:
```zig
var unmanaged: ArrayListUnmanaged(u8) = .{};
errdefer unmanaged.deinit();

// ...

var managed: ArrayList(u8) = .{ .allocator = allocator };
errdefer managed.deinit();

// ...
```

 * Mention "direct" initialization and `initCapacity` in doc comments,
   also removes inconsistency.

Signed-off-by: Eric Joldasov <bratishkaerik@getgoogleoff.me>
@andrewrk andrewrk closed this in cb419a1 Mar 15, 2024
@BratishkaErik
Copy link
Contributor Author

I don't understand how this method is dangerous, but OK.

@ifreund
Copy link
Member

ifreund commented Mar 16, 2024

I don't understand how this method is dangerous, but OK.

As explained in the documentation andrew added, this allows initializing the Arraylist with an inconsistent state, for example:

var list: std.ArrayList(u32) = .{
    .capacity = 42,
    .allocator = allocator,
};

This code would look a lot more instantly wrong with the .items = &.{}, initial value in the initializer.

Note that the existing Unmanaged data structures tend to violate this principle as well.

@BratishkaErik
Copy link
Contributor Author

Note that the existing Unmanaged data structures tend to violate this principle as well.

This is what I had in mind too, and I got a little bit cofused by referenced doc:

Default field values are only appropriate when the data invariants of a struct cannot
be violated by omitting that field from an initialization.

IIUC it doesn't apply to the ArrayList* at all, since it requires only allocator field — same as in init function — and default fields can be safely omitted.

Thanks for explanation.

@BratishkaErik BratishkaErik deleted the std.array_list/same-defaults branch March 16, 2024 14:34
RossComputerGuy pushed a commit to ExpidusOS-archive/zig that referenced this pull request Mar 20, 2024
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.

3 participants