Fix the bugs of manual_memcpy, simplify the suggestion and refactor it

[rail-rain]

While I’m working on the long procrastinated work to expand manual_memcpy(#1670), I found a few minor bugs and probably unidiomatic or old coding style. There is a brief explanation of changes to the behaviour this PR will make below. And, I have a questoin: do I need to add tests for the first and second fixed bugs? I thought it might be too rare cases to include the tests for those. I added for the last one though.

• Bug fix

  • It negates resulted offsets (src/dst_offset) when offset is subtraction by 0. This PR will remove any subtraction by 0 as a part of minification.

    for i in 0..5 {
        dst[i - 0] = src[i];
    }

     warning: it looks like you're manually copying between slices
       --> src/main.rs:2:14
        |
    LL  |     for i in 0..5 {
    -   |              ^^^^ help: try replacing the loop by: `dst[..-5].clone_from_slice(&src[..5])`
    +   |              ^^^^ help: try replacing the loop by: `dst[..5].clone_from_slice(&src[..5])`
        |

  • It prints RangeTo or RangeFull when both of end and offset are 0, which have different meaning. This PR will print 0. I could reject the cases end is 0, but I thought I won’t catch other cases reverse_range_loop will trigger, and it’s over to catch every such cases.

    for i in 0..0 {
        dst[i] = src[i];
    }

     warning: it looks like you're manually copying between slices
       --> src/main.rs:2:14
        |
     LL |     for i in 0..0 {
    -   |              ^^^^ help: try replacing the loop by: `dst.clone_from_slice(&src[..])`
    +   |              ^^^^ help: try replacing the loop by: `dst[..0].clone_from_slice(&src[..0])`
        |

  • it prints four dots when end is None. This PR will ignore any for loops without end because a for loop that takes RangeFrom as its argument and contains indexing without the statements or the expressions that end loops such as break will definitely panic, and manual_memcpy should ignore the loops with such control flow.

    fn manual_copy(src: &[u32], dst: &mut [u32]) {
        for i in 0.. {
            dst[i] = src[i];
        }
    }

    -warning: it looks like you're manually copying between slices
    -  --> src/main.rs:2:14
    -   |
    -LL |     for i in 0.. {
    -   |              ^^^ help: try replacing the loop by: `dst[....].clone_from_slice(&src[....])`
    -   |

• Simplification of the suggestion

  • It prints 0 when start or end and offset are same (from Simplify manual_memcpy suggestion in some cases #3323). This PR will use RangeTo

changelog: fixed the bugs of manual_memcpy and also simplify the suggestion.

[phansch]

Thanks for splitting up the commits as they are, that makes reviewing significantly more easy 👍 I'll try and review this by the end of the day.

[flip1995]

For your question about the tests: You already have them in the PR description, why not just throw them in the test file, with the short comments you wrote, what they are supposed to test? :)

[phansch]

changes LGTM but the 2 tests would indeed be nice to have.

[rail-rain]

Thanks you for your reviews. I've added the 2 tests.

[phansch]

@bors r+

[bors]

📌 Commit 461f4a3 has been approved by phansch

[bors]

⌛ Testing commit 461f4a3 with merge e3b8c41...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: phansch
Pushing e3b8c41 to master...