-
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
Add new lint manual_memmove #8337
Conversation
Still found some bugs, ignore. |
LL | | // left shift by 4 | ||
LL | | arr[i] = arr[i + 4]; | ||
LL | | } | ||
| |_____^ help: try replacing the loop by: `arr.copy_within(4..((arr.len() - 4) + 4), 0);` |
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.
The range bound could be simplified. My suspicion is that MinifyingSugg doesn't work properly here, but I would be "fine" with this output.
8477b7f
to
901d95d
Compare
Is there a reason you didn't just add a small extension to I would consider using the same lint as well, possibly renaming to something a little more general. |
if let Some(big_sugg) = big_sugg { | ||
span_lint_and_sugg( | ||
cx, | ||
MANUAL_MEMCPY, |
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.
need to change this
@Jarcho i'll try that next time I work on this, indeed i was reusing a lot of stuff from manual_memcpy, but that comes from cargo-cult, and the potential future challenges with manual_memmove (see testcases) are quite different |
r? @flip1995 (Is highfive awake?) |
Wat? |
☔ The latest upstream changes (presumably #8174) made this pull request unmergeable. Please resolve the merge conflicts. |
@untitaker are you interested in continuing with this lint? Or should it be closed ヘ(^・ω・^=)~ |
I think this lint was kind of in an OK state from what I can remember but could use further refactor to deduplicate code. No real desire to work on it right now |
Don't worry, no hard feelings (and you left some public work already done for future contributors). Have fun on other projects! (●ↀωↀ●) ❤️ |
Add a very basic lint that checks against unnecessary reimplementations of
copy_within
. The lint reuses some now-public parts of the memcpy lint, but supports almost none of the fanciness that the memcpy lint has. The main reason being that overlapping indices/ranges change behavior in memmove, but do not in memcpy (because src is immutable). The secondary reason being that this is my first lint.Fixes #8335
changelog: New lint [
manual_memmove
]