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 LoggingAllocator prototype #2338

Merged
merged 1 commit into from
Jun 27, 2019

Conversation

daurnimator
Copy link
Contributor

@daurnimator daurnimator commented Apr 22, 2019

Concept for #2337

Open questions:

  • how to allow custom logging function? trying
    pub fn init(parent_allocator: *Allocator, logFn: fn (comptime fmt: []const u8, args: ...) void) LoggingAllocator {
    fails with error: parameter of type 'fn([]const u8,var)var' must be declared comptime
  • Log message formatting

@Hejsil
Copy link
Contributor

Hejsil commented Apr 24, 2019

Instead of a logFn, you could store an *std.io.OutStream.

@andrewrk
Copy link
Member

Please feel free to open a new PR with something that is ready for review.

@andrewrk andrewrk closed this May 28, 2019
@daurnimator
Copy link
Contributor Author

This was ready to review?

@andrewrk andrewrk reopened this May 28, 2019
@andrewrk
Copy link
Member

Screenshot_2019-05-27_23-12-40

@daurnimator
Copy link
Contributor Author

I guess I was looking for a 2nd opinion on the phrasing in the log messages.

@daurnimator daurnimator marked this pull request as ready for review May 28, 2019 03:19
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.

This is related to a proposal I've been meaning to type up. I'll go ahead and do that now.

@@ -565,6 +565,53 @@ pub fn StackFallbackAllocator(comptime size: usize) type {
};
}

pub const NoErrorOutStream = std.io.OutStream(error{});
pub const LoggingAllocator = struct {
Copy link
Member

Choose a reason for hiding this comment

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

doc comments please. when should someone use this? what are the semantics?

@@ -565,6 +565,53 @@ pub fn StackFallbackAllocator(comptime size: usize) type {
};
}

pub const NoErrorOutStream = std.io.OutStream(error{});
Copy link
Member

Choose a reason for hiding this comment

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

How does one acquire a NoErrorOutStream? If I were to for example use (try std.io.getStdErr()).outStream() this outstream has errors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've used this before....:

diff --git a/std/debug.zig b/std/debug.zig
index c5741f390..6425a7526 100644
--- a/std/debug.zig
+++ b/std/debug.zig
@@ -2339,7 +2339,15 @@ fn readILeb128(in_stream: var) !i64 {
 }
 
 /// This should only be used in temporary test programs.
-pub const global_allocator = &global_fixed_allocator.allocator;
+var NoErrorStdErrStream = std.heap.NoErrorOutStream {
+    .writeFn = struct {
+        pub fn writeFn(out_stream: *std.heap.NoErrorOutStream, bytes: []const u8) error{}!void {
+            const stderr = io.getStdErr() catch return;
+            stderr.write(bytes) catch return;
+        }
+    }.writeFn,
+};
+pub const global_allocator = &std.heap.LoggingAllocator.init(&global_fixed_allocator.allocator, &NoErrorStdErrStream).allocator;

@andrewrk
Copy link
Member

This proposal would remove the need for the OutStream component: #2586

@andrewrk andrewrk merged commit ad7927a into ziglang:master Jun 27, 2019
@daurnimator daurnimator deleted the logging-allocator branch June 27, 2019 16:50
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