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

New allocator corrections #5734

Closed
wants to merge 1 commit into from
Closed

New allocator corrections #5734

wants to merge 1 commit into from

Conversation

data-man
Copy link
Contributor

No description provided.

@Snektron
Copy link
Collaborator

StackFallbackAllocator is also broken as of #5064. Perhaps it should be replaced by a generic fallback allocator which wraps
two allocators? (instead of hardcoding it to be FixedBufferAllocator.)

Comment on lines +4 to +7
/// This allocator is used in front of another allocator and logs to the provided writer
/// on every call to the allocator. Writer errors are ignored.
/// If https://github.com/ziglang/zig/issues/2586 is implemented, this API can be improved.
pub fn LoggingAllocator(comptime OutStreamType: type) type {
pub fn LoggingAllocator(comptime WriterType: type) type {
Copy link
Member

Choose a reason for hiding this comment

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

This should be changed to use std.log facilities, and it no longer needs to be a generic type.

Copy link
Contributor

Choose a reason for hiding this comment

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

Does that mean we won't be able to test the logging output anymore?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

This gave me another idea: #5738

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@andrewrk

This should be changed to use std.log facilities

What to use?
log.notice, log.info, log.debug?

What scope? .loggingAllocator?

we can add a "compare output" test case for it

How? With log function in compare_output.zig?

@data-man data-man marked this pull request as draft June 28, 2020 19:19
@andrewrk
Copy link
Member

stale draft, please re-open when it's ready for review

@andrewrk andrewrk closed this Jul 24, 2020
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.

4 participants