Skip to content

add a fuzz test for zig fmt and fix two bugs #23793

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

gooncreeper
Copy link
Contributor

Adds a new fuzz test for zig fmt. This fuzz test checks that Ast.render
succeeds for parsed inputs. Additionally, for inputs it knows that
Ast.render cannot change the order of, it checks that they are not
rewritten.

This fuzz test has been very successful. Using #23416, it has found
three bugs (one was a TODO); two of them I have fixed and the other is
in #23754. I have run the test for 650,000,000 iterations (about 2
hours) and haven't found any more bugs.

Some functions in the fuzz test have instrumentation disabled because
their branches are not interesting to the fuzzer; doing this found a
~40% boost to the runs per second. Additionally, the fuzz test handles
tokenization itself since so it can determine if the input can be
rewritten.

Adds a new fuzz test for zig fmt. This fuzz test checks that Ast.render
succeeds for parsed inputs. Additionally, for inputs it knows that
Ast.render cannot change the order of, it checks that they are not
rewritten.

This fuzz test has been very successful. Using ziglang#23416, it has found
three bugs (one was a TODO); two of them I have fixed and the other is
in ziglang#23754. I have run the test for 650,000,000 iterations (about 2
hours) and haven't found any more bugs.

Some functions in the fuzz test have instrumentation disabled because
their branches are not interesting to the fuzzer; doing this found a
~40% boost to the runs per second. Additionally, the fuzz test handles
tokenization itself since so it can determine if the input can be
rewritten.
@andrewrk
Copy link
Member

andrewrk commented May 4, 2025

Very nice! You know what else might be a nice property to test is that running fmt a second time does not change the output.

Also, I think this test will be much more efficient once we have a smith. I don't know how far along @mlugg got on that, but that's a fun project. Basically take a fuzz input and convert it into a valid AST.

@mlugg
Copy link
Member

mlugg commented May 4, 2025

My zig-smith has taken a back seat for now, but I also don't think it would be a great test of the parser. My goal with that project has been to emit programs which compile correctly and do interesting things in the context of incremental compilation, and which, crucially, are deterministic at runtime and do not trigger IB. Those are big restrictions, so the thing I was working on was unfortunately quite limited in terms of syntax. I would definitely be interested in something more generalized, but I'd have to figure out how to make it useful in the context of testing incremental compilation!

@andrewrk
Copy link
Member

andrewrk commented May 6, 2025

don't think it would be a great test of the parser

Right but we're talking about testing the renderer. The renderer happens to also be a great way to test the parser, which is why they are related. But my point is that it would provide the ability to test the idempotency property of zig fmt. That's going to be more efficient with valid AST.

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