DONOT Merge Add formatted printing of pointers #1285
Conversation
|
This has debug logging (wink_log) which will be removed before this is merged. |
std/fmt/index.zig
Outdated
| /// Renders fmt string with args, calling output with slices of bytes. | ||
| /// If `output` returns an error, the error is returned from `format` and | ||
| /// `output` is not called again. | ||
| pub fn format(context: var, comptime Errors: type, output: fn (@typeOf(context), []const u8) Errors!void, comptime fmt: []const u8, args: ...) Errors!void { | ||
| //wink_log_strln("format:+ ", fmt); |
There was a problem hiding this comment.
you can use std.debug.warn("format:+ ", fmt);
or even @compileLog("format:+", fmt)
There was a problem hiding this comment.
The problem with using warn is that it uses std.format so its recursive and generates extra output, which is why I used "wink_log" which outputs the directly to stdout.
| var value = "abc"; | ||
| try testFmt("var []u8: abc\n", "var []u8: {}\n", value); | ||
| } | ||
| { |
There was a problem hiding this comment.
you could use @intToPtr to test some values here.
There was a problem hiding this comment.
I don't understand, I added these just because there weren't tests of quoted strings so I thought I'd add a couple. Could you provide a more information?
There was a problem hiding this comment.
You're only checking that the prefix is correct, because pointers have non-deterministic values at runtime. But you could use @intToPtr(*T, 0xdeadbeef) and then you can look for "T@0xdeadbeef" using testFmt.
There was a problem hiding this comment.
Got it, another option is to use @ptr2Int using "real" addresses:
{
var buf1: [32]u8 = undefined;
var buf2: [32]u8 = undefined;
const value = "abc";
// Print Pointer Path3
const expected = try bufPrint(buf2[0..], "&value: u8@{x}", @ptrToInt(&value));
const actual = try bufPrint(buf1[0..], "&value: {p}", &value);
wink_log_strln("expected=", expected);
wink_log_strln("actual=", actual);
try testExpectedActual(expected, actual);
}
fn testExpectedActual(expected: []const u8, actual: []const u8) !void {
if (mem.eql(u8, expected, actual)) return;
std.debug.warn("\n====== expected this output: =========\n");
std.debug.warn("{}", expected);
std.debug.warn("\n======== instead found this: =========\n");
std.debug.warn("{}", actual);
std.debug.warn("\n======================================\n");
return error.TestFailed;
}
I like using "real" addresses where we can, do you have a preference?
winksaville
force-pushed
the
add-formatted-printing-of-pointers
branch
from
4fad90b
to
bb97863
Compare
|
Just rebased since it had been a few days. Also, this will kick off a new CI build as the last one failed in appveyro for now apparent reason. |
winksaville
force-pushed
the
add-formatted-printing-of-pointers
branch
from
bb97863
to
63aad0a
Compare
|
Lastest change, 63aad0a, removes wink_log and is rebased on ToT as of a few minutes ago. |
Needs support for {p} support I added to std/fmt/index.zig:
ziglang/zig#1285
Should be in master soon.
winksaville
changed the title
|
I found a bug, you can't print the address of a struct with a custom format routine. Will add a test and try to fix. |
Added 'p' as a new formatting character. Add tests that
print pointers of various sizes and covers all the paths.
One existing test needed to be change, the Struct test at line 1055,
a {} changed to {p}.
winksaville
force-pushed
the
add-formatted-printing-of-pointers
branch
from
63aad0a
to
1b26c81
Compare
|
The solution I came up with to allow a custom format routine of a structure to have its address printed was to add a pubic formatAddress routine and then a custom format routine can invoke it to print the address. There are certainly other solutions, such as detect the {p} before calling the custom format routine, but it gives the custom format routine more control. This has passed all tests locally. I'll leave the DONOT Merge on this change until you review. |
Needs support for {p} support I added to std/fmt/index.zig:
ziglang/zig#1285
Should be in master soon.
|
@andrewrk, review please. |
|
I'll have a look at this tomorrow morning and get it working. |
|
Txs, AFAIK it is working, but I'm sure there are improvements :) One thing is I'd like to see testExpectedActual be public, but not sure where it might go. I'll be glad to make any changes you'd like to see |
|
I did a simpler implementation, and used |
Added 'p' as a new formatting character. Add tests that
print pointers of various sizes and covers all the paths.
One existing test needed to be change, the Struct test at line 1109,
a {} changed to {p}.