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

Explain why &T is cloned when T is not Clone #95585

Merged
merged 1 commit into from
Apr 6, 2022

Conversation

compiler-errors
Copy link
Member

Fixes #95535

@rust-highfive
Copy link
Collaborator

r? @estebank

(rust-highfive has picked a reviewer for you, use r? to override)

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 2, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 2, 2022
@notriddle
Copy link
Contributor

This seems like it’s redundant with the https://github.com/rust-lang/rust/blob/master/compiler/rustc_lint/src/noop_method_call.rs lint, which is disabled right now, but would accomplish the same thing.

Is it desirable to merge this, and then remove it later when the lint gets stabilized?

@compiler-errors
Copy link
Member Author

compiler-errors commented Apr 2, 2022

@notriddle, given this code

#![warn(noop_method_call)]

struct PlainType<T>(T);

pub fn foo(non_clone_type_ref: &PlainType<u32>) -> PlainType<u32> {
    non_clone_type_ref.clone()
}

The suggestion I added fires, since it's a suggestion on the type error, but the noop method call lint does not... I don't know very much about how late lints work, do you know if it only fires if typeck succeeds? If so, then it doesn't seem like we can deduplicate the logic here.

@notriddle
Copy link
Contributor

It looks like it doesn’t run when there’s a type error, but this is a known issue, if I’m understanding #91536 correctly.

@compiler-errors
Copy link
Member Author

Interesting! Thanks for pointing that out. I may take a look at that issue..

@bors
Copy link
Contributor

bors commented Apr 5, 2022

☔ The latest upstream changes (presumably #95667) made this pull request unmergeable. Please resolve the merge conflicts.

@estebank
Copy link
Contributor

estebank commented Apr 5, 2022

@compiler-errors it's a long standing issue for lints that would help not triggering in the face of errors. It is also better (if more work) to consolidate useful information in the earliest possible error and avoid the later ones (or, as we do in some cases, keep information around from the first error, not emit it, and extend the later one).

@estebank
Copy link
Contributor

estebank commented Apr 5, 2022

r=me after rebase <3

@compiler-errors
Copy link
Member Author

@bors r=estebank

@bors
Copy link
Contributor

bors commented Apr 5, 2022

📌 Commit d12689c has been approved by estebank

@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 Apr 5, 2022
--> $DIR/explain_clone_autoref.rs:9:5
|
LL | nc.clone()
| ^^
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(potential follow up work?/random thought) adding a label to this span to say "this is of type NotClone" might be useful (particularly for longer method chains).

bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 5, 2022
Rollup of 8 pull requests

Successful merges:

 - rust-lang#88025 (ScmCredentials netbsd implementation.)
 - rust-lang#95473 (track individual proc-macro expansions in the self-profiler)
 - rust-lang#95547 (caution against ptr-to-int transmutes)
 - rust-lang#95585 (Explain why `&T` is cloned when `T` is not `Clone`)
 - rust-lang#95591 (Use revisions to track NLL test output (part 1))
 - rust-lang#95663 (diagnostics: give a special note for unsafe fn / Fn/FnOnce/FnMut)
 - rust-lang#95673 (:arrow_up: rust-analyzer)
 - rust-lang#95681 (resolve: Fix resolution of empty paths passed from rustdoc)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 42ab448 into rust-lang:master Apr 6, 2022
@rustbot rustbot added this to the 1.62.0 milestone Apr 6, 2022
@compiler-errors compiler-errors deleted the ref-clone branch April 7, 2022 04:29
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. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No hint about implementing clone on type mismatch when cloning reference
6 participants