- Sponsor
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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 tests to formatted printing of pointers #1311
Add tests to formatted printing of pointers #1311
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand what this is supposed to accomplish, and it breaks existing intentional behavior.
std/fmt/index.zig
Outdated
@@ -152,7 +154,7 @@ pub fn formatType( | |||
if (info.child == u8) { | |||
return formatText(value, fmt, context, Errors, output); | |||
} | |||
return format(context, Errors, output, "{}@{x}", @typeName(T.Child), @ptrToInt(value)); | |||
@compileError("Unexpected path 1 for possibly printing a pointer"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is supposed to work. why is this now a compile error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a test that exercises that path and changed the code.
std/fmt/index.zig
Outdated
try testFmt("pointer: i32@deadbeef\n", "pointer: {}\n", value); | ||
try testFmt("pointer: i32@deadbeef\n", "pointer: {*}\n", value); | ||
} | ||
{ | ||
const value: [3]u8 = "abc"; | ||
try testFmt("const [3]u8: abc\n", "const [3]u8: {}\n", value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a redundant test. see line 884 above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing.
var buf2: [32]u8 = undefined; | ||
var value: u127 = 127; | ||
|
||
const expected = try bufPrint(buf1[0..], "&value: u127@{x}", @ptrToInt(&value)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's with all these weird integer size tests? what do they bring to the table? we already have tests for integers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought it a good idea to test the extremes of printing bit fields, I can remove it you prefer.
std/fmt/index.zig
Outdated
var buf2: [32]u8 = undefined; | ||
var value = []u16{1, 2}; | ||
|
||
// Print Pointer Path5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what does this mean, "Path5"? there's no "path 5" for pointer printing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line 203.
Note: these "Print Pointer PathX" use to test PathX but don't do so now because of new Pointer state in format.
I've changed to comments to read:
// Before support for address printing using {*} this exercised Print Pointer PathX
One of the purposes of these new tests is to validate that code that would have previously executed paths that are now @compileerrors do not execute those paths. Rename the tests of have a more consistent style allowing easier use of the test-filter parameter. Add factor testExpectedActual out of testFmt and make it public so this and other tests may use it.
1e51c98
to
f7b1299
Compare
I think this is not better than status quo. |
One of the purposes of these new tests is to validate that code that
would have previously executed paths that are now @compileerrors do not
execute those paths.
Rename the tests of have a more consistent style allowing easier
use of the test-filter parameter.
Add factor testExpectedActual out of testFmt and make it public so this
and other tests may use it.