-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Don't lint various match lints when expanded by a proc-macro #8667
Conversation
r? @flip1995 (rust-highfive has picked a reviewer for you, use r? to override) |
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.
LGTM, small comment on the utils function.
fn helper(sm: &SourceMap, span: Span, text: &str) -> bool { | ||
let pos = sm.lookup_byte_offset(span.lo()); |
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 this helper function would only change this line inside the whole helper function
fn helper(sm: &SourceMap, span: Span, text: &str) -> bool { | |
let pos = sm.lookup_byte_offset(span.lo()); | |
cx.sess().source_map().lookup_byte_offset(span.lo()); |
but we save a level of indentation and an inner function
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.
It's to deal with duplication due to monomorphization on the lint context.
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.
Shouldn't that just get optimized out by the outliner or other opt passes?
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.
Just checked: it doesn't. This saves around 1 kB in code size.
@bors r+ Thanks! |
📌 Commit 63f6a79 has been approved by |
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
fixes #4952
As always for proc-macro output this is a hack-job of a fix. It would be really nice if more proc-macro authors would set spans correctly.
changelog: Don't lint various lints on proc-macro output.