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

Add a with_capacity function #29

Merged
merged 4 commits into from
Oct 29, 2019
Merged

Add a with_capacity function #29

merged 4 commits into from
Oct 29, 2019

Conversation

TethysSvensson
Copy link
Contributor

No description provided.

@TethysSvensson TethysSvensson changed the title Add a with_capacity function capacity Add a with_capacity function Oct 26, 2019
Copy link
Owner

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

Looks good, but I'd like to add a little more of a smoke test before merging:

Can you add a test that creates a Bump with a funky capacity (something that is not footer aligned, like 11) and then also allocate things out of that bump arena that require more alignment (like u128s)? At least enough to cause the bump arena to allocate a new chunk.

This probably belongs in tests/tests.rs rather than the doc comment.

Thanks!

@TethysSvensson
Copy link
Contributor Author

Can you add a test that creates a Bump with a funky capacity (something that is not footer aligned, like 11) and then also allocate things out of that bump arena that require more alignment (like u128s)? At least enough to cause the bump arena to allocate a new chunk.

Yes, I can definitely add a test like that. :)

I have also simplified the code a bit, to make it more obvious that this will never allocate less than a normal call to Bump::new() would.

@TethysSvensson
Copy link
Contributor Author

@fitzgen I've now added the test.

Copy link
Owner

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

Thanks!

@fitzgen fitzgen merged commit 9b22a64 into fitzgen:master Oct 29, 2019
@TethysSvensson TethysSvensson deleted the with-capacity branch October 29, 2019 19:16
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