Skip to content
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

Make AssocOp Copy #73629

Merged
merged 1 commit into from
Jun 26, 2020
Merged

Make AssocOp Copy #73629

merged 1 commit into from
Jun 26, 2020

Conversation

flip1995
Copy link
Member

Found that this enum is not Copy while reviewing this Clippy PR: rust-lang/rust-clippy#5727 (comment)

There shouldn't be a reason why this should not be Copy.

@rust-highfive
Copy link
Collaborator

r? @ecstatic-morse

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 22, 2020
@Manishearth
Copy link
Member

@bors r+ rollup=always

@bors
Copy link
Contributor

bors commented Jun 26, 2020

📌 Commit 2792014 has been approved by Manishearth

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 26, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 26, 2020
…arth

Rollup of 13 pull requests

Successful merges:

 - rust-lang#72620 (Omit DW_AT_linkage_name when it is the same as DW_AT_name)
 - rust-lang#72967 (Don't move cursor in search box when using arrows to navigate results)
 - rust-lang#73102 (proc_macro: Stop flattening groups with dummy spans)
 - rust-lang#73297 (Support configurable deny-warnings for all in-tree crates.)
 - rust-lang#73507 (Cleanup MinGW LLVM linkage workaround)
 - rust-lang#73588 (Fix handling of reserved registers for ARM inline asm)
 - rust-lang#73597 (Record span of `const` kw in GenericParamKind)
 - rust-lang#73629 (Make AssocOp Copy)
 - rust-lang#73681 (Update Chalk to 0.14)
 - rust-lang#73707 (Fix links in `SliceIndex` documentation)
 - rust-lang#73719 (emitter: column width defaults to 140)
 - rust-lang#73729 (disable collectionbenches for android)
 - rust-lang#73748 (Add code block to code in documentation of `List::rebase_onto`)

Failed merges:

r? @ghost
@bors bors merged commit 3d7521d into rust-lang:master Jun 26, 2020
@flip1995 flip1995 deleted the assoc_op_copy_clone branch June 26, 2020 15:12
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jul 14, 2020
…arth

Rollup of 13 pull requests

Successful merges:

 - rust-lang#72620 (Omit DW_AT_linkage_name when it is the same as DW_AT_name)
 - rust-lang#72967 (Don't move cursor in search box when using arrows to navigate results)
 - rust-lang#73102 (proc_macro: Stop flattening groups with dummy spans)
 - rust-lang#73297 (Support configurable deny-warnings for all in-tree crates.)
 - rust-lang#73507 (Cleanup MinGW LLVM linkage workaround)
 - rust-lang#73588 (Fix handling of reserved registers for ARM inline asm)
 - rust-lang#73597 (Record span of `const` kw in GenericParamKind)
 - rust-lang#73629 (Make AssocOp Copy)
 - rust-lang#73681 (Update Chalk to 0.14)
 - rust-lang#73707 (Fix links in `SliceIndex` documentation)
 - rust-lang#73719 (emitter: column width defaults to 140)
 - rust-lang#73729 (disable collectionbenches for android)
 - rust-lang#73748 (Add code block to code in documentation of `List::rebase_onto`)

Failed merges:

r? @ghost
bors added a commit to rust-lang/rust-clippy that referenced this pull request Oct 10, 2020
Expands `manual_memcpy` to lint ones with loop counters

Closes #1670

This PR expands `manual_memcpy` to lint ones with loop counters as described in #1670 (comment)

Although the current code is working, I have a couple of questions and concerns.

~~Firstly, I manually implemented `Clone` for `Sugg` because `AssocOp` lacks `Clone`. As `AssocOp` only holds an enum, which is `Copy`, as a value, it seems `AssocOp` can be `Clone`; but, I was not sure where to ask it. Should I make a PR to `rustc`?~~ The [PR]( rust-lang/rust#73629) was made.

Secondly, manual copying with loop counters are likely to trigger `needless_range_loop` and `explicit_counter_loop` along with `manual_memcpy`; in fact, I explicitly allowed them in the tests. Is there any way to disable these two lints when a code triggers `manual_memcpy`?

And, another thing I'd like to note is that `Sugg` adds unnecessary parentheses when expressions with parentheses passed to its `hir` function, as seen here:

```
error: it looks like you're manually copying between slices
  --> $DIR/manual_memcpy.rs:145:14
   |
LL |     for i in 3..(3 + src.len()) {
   |              ^^^^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[3..((3 + src.len()))].clone_from_slice(&src[..((3 + src.len()) - 3)])
```

However, using the `hir` function is needed to prevent the suggestion causing  errors when users use bitwise operations; and also this have already existed, for example: `verbose_bit_mask`. Thus, I think this is fine.

changelog: Expands `manual_memcpy` to lint ones with loop counters
@cuviper cuviper added this to the 1.46 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants