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

FixedBufferAllocator.reset #3238

Merged
merged 2 commits into from Sep 21, 2019
Merged

Conversation

Tetralux
Copy link
Contributor

No description provided.

@andrewrk andrewrk added this to the 0.5.0 milestone Sep 19, 2019
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.

Do we need a reset() function? end_index is considered to be a public field, and it's pretty simple concept to set it to zero. FixedBufferAllocator is a lightweight abstraction. I don't think "encapsulation" is valuable here.

std/heap.zig Outdated Show resolved Hide resolved
@andrewrk andrewrk modified the milestones: 0.5.0, 0.6.0 Sep 20, 2019
@daurnimator daurnimator added the standard library This issue involves writing Zig code for the standard library. label Sep 21, 2019
@Tetralux
Copy link
Contributor Author

Tetralux commented Sep 21, 2019

@andrewrk

Do we need a reset() function? end_index is considered to be a public field, and it's pretty simple concept to set it to zero. FixedBufferAllocator is a lightweight abstraction. I don't think "encapsulation" is valuable here.

It's a public field? Why is it not marked pub then?

If the field is in fact public, despite not being pub, then that needs making much more clear, and warrants explanation, at least in the docs of pub.

I don't really get what you mean about encapsulation here. I'm just making clear that you are able to reset this kind of allocator, where before most people would have assumed that you cannot; especially since the field is non-pub; you don't want to rely on implementation details; if the name of field is changed then it breaks. It makes it more maintainable to have a simple reset procedure for it.

One use of this off the top of my head:

var buf: [1024]u8 = undefined;
var fba = std.heap.FixedBufferAllocator.init(buf[0..]);
var a = &fba.allocator;
while (true) : (fba.reset()) {
    // do some work, use the result, and throw it all away at the end by resetting.
}

@andrewrk andrewrk merged commit 533aff3 into ziglang:master Sep 21, 2019
@andrewrk
Copy link
Member

andrewrk commented Sep 21, 2019

It's a public field? Why is it not marked pub then?

Fair question. pub for fields is having a bit of an existential crisis at the moment. At this point I'm more likely to remove that syntax than make it do anything.

I'm fine with a reset() function, at least for now.

@Tetralux Tetralux deleted the fixed-allocator-reset branch September 22, 2019 00:01
@Tetralux
Copy link
Contributor Author

Cheers!

@andrewrk andrewrk removed this from the 0.6.0 milestone Sep 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants