-
Notifications
You must be signed in to change notification settings - Fork 13.5k
Add runtime check to avoid overwrite arg in Diag
#142724
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
base: master
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
733b453
to
e0afd4a
Compare
… restore snapshot when set subdiag arg Signed-off-by: xizheyin <xizheyin@smail.nju.edu.cn>
e0afd4a
to
faa502e
Compare
|
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 time there was a lot less code to modify.
For manual implementation of Subdiagnostic, feel free to call store_args
, restore_args
, remove_arg
...
|
||
// For #[derive(Subdiagnostic)] | ||
// | ||
// - Store args of the main diagnostic for later restore. | ||
// - add args of subdiagnostic. | ||
// - Generate the calls, such as note, label, etc. | ||
// - Remove the arguments for allowing Vec<Subdiagnostic> to be used. | ||
// - Restore the arguments for allowing main and subdiagnostic share the same fields. | ||
Ok(quote! { | ||
#init | ||
#formatting_init | ||
#attr_args | ||
#store_args | ||
#plain_args | ||
#calls | ||
#remove_args | ||
#restore_args |
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.
For #[derive(Subdiagnostic)], I made two changes:
- After the subdiagnostic is rendered, remove all args of this subdiagnostic, which allows for usage like
Vec<Subdiag>
. - Store
diag.args
before setting arguments, so that you can restore the contents of the main diagnostic after deleting the arguments after subdiagnostic is rendered, to avoid deleting the main diagnostic's arg when they have the same name args.
I set up a
debug_assert
check for the arg method to make sure thatargs
inDiag
aren't easily overwritten, and I added theremove_arg()
method, so that if you do need to overwrite an arg, then you can explicitly callremove_arg()
to remove it first, then callarg()
to overwrite it.For the code before the #142015 change, it won't compile because it will report an error
This PR also modifies all diagnostics that fail the check to pass the check. There are two cases of check failure:
Vec
, the rendering may iteratively write the same arg over and over again. In this case, I changed the auto-generation withderive(SubDiagnostic)
to manually implementingSubDiagnostic
and manually rendered it witheagerly_translate()
, similar to fluent diagnostics can easily overwrite each others' fields/args #142031 (comment), and after rendering it I manually deleted useless arg with the newly addedremove_arg
method.(Some formatting needs to be adjusted though)
r? @oli-obk