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.mem.Allocator: replace create signature with construct signature #1141
std.mem.Allocator: replace create signature with construct signature #1141
Conversation
std/mem.zig
Outdated
/// Alias of create | ||
/// Call destroy with the result | ||
pub fn construct(self: *Allocator, init: var) Error!*@typeOf(init) { | ||
debug.warn("std.mem.Allocator.construct is deprecated;\n"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should just remove it. Zig has few users, no big codebases (std
is the biggest) and there have already been many breaking changes over the past month :)
Besides, if create
's signature is changed anyways, then this is already a breaking change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andrewrk thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, saw this just now after I already merged it. I agree with @Hejsil's logic and I removed it in a fixup commit as part of merging. 👍
@@ -193,7 +193,11 @@ fn BaseLinkedList(comptime T: type, comptime ParentType: type, comptime field_na | |||
/// A pointer to the new node. | |||
pub fn allocateNode(list: *Self, allocator: *Allocator) !*Node { | |||
comptime assert(!isIntrusive()); | |||
return allocator.create(Node); | |||
return allocator.create(Node{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It probably doesn't matter too much, but the right replacement is probably return allocator.create(Node(undefined));
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, I was wondering about this!
Looks great! Congrats on your first PR. |
ref #733