-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Note the version and PR of removed features when using it #141642
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
Signed-off-by: xizheyin <[email protected]>
This comment has been minimized.
This comment has been minimized.
r? compiler (bandwidth) |
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.
thank you! this looks very good, i just have one comment about the new macro :)
I tried
So, I use helper macros to parse |
This comment has been minimized.
This comment has been minimized.
gotcha - can you write out the match explicitly instead of using separately i will open an issue to make |
compiler/rustc_expand/src/config.rs
Outdated
sess.dcx().emit_err(FeatureRemoved { | ||
span: mi.span(), | ||
reason: f.reason.map(|reason| FeatureRemovedReason { reason }), | ||
span, |
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.
nit: seems the shorthand style here has more lines of code.
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.
Thanks for the reminder, indeed.
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.
can you write out the match explicitly instead of using or?
My current approach should have accomplished the goal?
macro_rules! opt_nonzero_u32 { | ||
() => { | ||
None | ||
}; | ||
($val:expr) => { | ||
Some(NonZeroU32::new($val).unwrap()) | ||
}; |
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 used an auxiliary macro to get around this to make sure the git blame doesn't get broken.
Signed-off-by: xizheyin <[email protected]>
Partially Fixes #141619
I added the diagnostic information. Since all the current version information is present, it prints the version information anyway, as shown in tests/ui. And PR will not print if it is None, we can gradually add the PR links.
Split into two commits for easier review.
r? compiler
cc @jyn514 Since you're on vocation in the review list, I can't r? you.