Better error messages for derives #400
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was missed in 37a93d3
shahn
force-pushed
the
derive_errors
branch
3 times, most recently
from
b3b3704 to
b9bc87d
Compare
|
Thank you for your PR! |
XAMPPRocky
approved these changes
Jan 5, 2025
|
I am still working on this, there are some remaining instances of bad error messages left which I am weeding out. |
This helps in benchmarking/profiling, because it removes the overhead of repeatedly parsing the inputs.
This is helpful for finding panics inside the derive macro, which always lead to subpar error messages. To use this with cargo-afl, you might go about it like this: // Use extra fuzzing crate, not part of workspace cd fuzzing // To build cargo afl build --release // To start fuzzing. -a restricts the input to ascii, -G limits the // input size to 512 bytes. Larger mostly just tests the TokenStream // machinery. Leave out both options for slower but more thorough scan cargo afl fuzz -i derive_in -x dict/derive.dict -a text -G 512 -o path_to_output_dir target/release/derive // To reproduce a crash cargo afl run --bin derive < path_to_output_dir/default/crashes/...
|
Should be fine now :) |
This was referenced Jan 5, 2025
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I added error span reporting to the derive macros. While doing so, I was worried about degrading performance, so I added a benchmark. To my surprise, performance improved between 2% and 20% for each of my individual benches. Benches are auto-generated from the rasn code base, by simply parsing all structs and enums which are using derives.
I also added a fuzz test for the derives, to help see if I missed any panics which would definitely not lead to good errors.
Please see which of the commits in this PR you want to take, because only the last one is about the better error messages. I am also happy to provide individual PRs if you think this would make sense. It is probably not useful to just squash all these into a single merge commit, as that removes the ability to reproduce the benchmark.