-
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 apply string_lit_as_bytes
if in macro expansion
#10665
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @giraffate (or someone else) soon. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
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 creating the pull request! I made some comments. If you have any questions, feel free to ask us.
- It would be better to ignore it only in an external macro, so this would be helpful for it: fix
almost_swapped
: Ignore external macros #10502 - It looks like that this fix affects the
string_from_utf8_as_bytes
lint. At first, it would be better to fix onlystring_lit_as_bytes
.
Thanks, I wasn't sure how to do that. Done |
☔ The latest upstream changes (presumably #10669) made this pull request unmergeable. Please resolve the merge conflicts. |
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.
Overall looks good, thanks!
We follow the no merge-commit policy, so can you do the rebase the master
branch, not merge? Once this is resolved, I'll merge this.
https://github.com/rust-lang/rust-clippy/blob/master/book/src/development/basics.md#pr
97a49c0
to
14a6fa4
Compare
@bors r+ Thanks! |
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
The following code will emit a warning on both w! and h!, despite there being nothing the user (or library author) could do about it:
This is because windows-rs will create a binding
const INPUT: &[u8] = $s.as_bytes()
, and changing this to b"$s" is, well, suboptimal. I don't know enough about Rust to know if this is something that can be detected though if it can be I'm happy with closing this in favor of implementing that.I'm not sure whether this is how it should be done though, as this simply tells clippy to not invoke this even if it's applicable (this also affects the other string lints, but didn't cause any tests to fail).
changelog: [
string_lit_as_bytes
]: Don't lint if in external macro