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

Compiler doesn't recognize existing #[derive(Clone)] and suggests to add it #122750

Closed
bluenote10 opened this issue Mar 19, 2024 · 4 comments
Closed
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` C-bug Category: This is a bug. D-confusing Diagnostics: Confusing error or lint that should be reworked. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. S-has-mcve Status: A Minimal Complete and Verifiable Example has been found for this issue T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@bluenote10
Copy link
Contributor

I tried this code:

use std::cell::RefCell;
use std::rc::Rc;

// The compiler says: `Dynamic<T>` does not implement `Clone`
// and suggests that I should add a #[derive(Clone)] here even
// though it already is there.
#[derive(Clone)]
pub struct Dynamic<T>(Rc<RefCell<T>>);

/*
// Replacing the #[derive(Clone)] by this explicit implementation
// makes it compile.
impl<T> Clone for Dynamic<T>
{
    fn clone(&self) -> Self {
        Self(self.0.clone())
    }
}
*/

impl<T> Dynamic<T>
{
    pub fn into_consumer(&self) -> Consumer<T> {
        Consumer::new(self.clone())
    }    
}

pub struct Consumer<T>(Dynamic<T>);

impl<T> Consumer<T>
{
    pub fn new(dynamic: Dynamic<T>) -> Self {
        Self(dynamic)
    }
}

Example on Rust playground

I expected to see this happen: Since Dynamic<T> has a #[derive(Clone)] I'd expect it to implement Clone, and the code to compile.

Instead, this happened: The code doesn't compile. The compiler claims Dynamic<T> does not implement Clone and suggests that I should add a #[derive(Clone)] exactly at where it already is. The exact error is:

error[E0308]: mismatched types
  --> src/lib.rs:24:23
   |
24 |         Consumer::new(self.clone())
   |         ------------- ^^^^^^^^^^^^ expected `Dynamic<T>`, found `&Dynamic<T>`
   |         |
   |         arguments to this function are incorrect
   |
   = note: expected struct `Dynamic<_>`
           found reference `&Dynamic<_>`
note: `Dynamic<T>` does not implement `Clone`, so `&Dynamic<T>` was cloned instead
  --> src/lib.rs:24:23
   |
24 |         Consumer::new(self.clone())
   |                       ^^^^
   = help: `Clone` is not implemented because the trait bound `T: Clone` is not satisfied
note: associated function defined here
  --> src/lib.rs:32:12
   |
32 |     pub fn new(dynamic: Dynamic<T>) -> Self {
   |            ^^^ -------------------
help: consider annotating `Dynamic<T>` with `#[derive(Clone)]`
   |
8  + #[derive(Clone)]
9  | pub struct Dynamic<T>(Rc<RefCell<T>>);
   |

Meta

rustc --version --verbose:

rustc 1.76.0 (07dca489a 2024-02-04)
binary: rustc
commit-hash: 07dca489ac2d933c78d3c5158e3f43beefeb02ce
commit-date: 2024-02-04
host: x86_64-unknown-linux-gnu
release: 1.76.0
LLVM version: 17.0.6

Related issues

I spend a while searching for existing issues, expecting that there probably is already an open issue for that. I found these issue which seem the have a connection, but still didn't manifest exactly the same:

@bluenote10 bluenote10 added the C-bug Category: This is a bug. label Mar 19, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Mar 19, 2024
@bluenote10
Copy link
Contributor Author

Additional info: The expanded macro is:

#[automatically_derived]
impl<T: ::core::clone::Clone> ::core::clone::Clone for Dynamic<T> {
    #[inline]
    fn clone(&self) -> Dynamic<T> {
        Dynamic(::core::clone::Clone::clone(&self.0))
    }
}

So I guess the issue here is also that Clone is used as a trait bound on T, which kind of invalidates the impl, as also explained in #108894 (comment)

Interestingly in that other issue, the compiler error does give the confusing recommendation to add a #derive[Clone], which is why I'm not sure if it is an exact duplicate.

@jieyouxu jieyouxu added A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` D-confusing Diagnostics: Confusing error or lint that should be reworked. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. S-has-mcve Status: A Minimal Complete and Verifiable Example has been found for this issue T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Mar 19, 2024
@estebank
Copy link
Contributor

This is partly a consequence of a lack of "perfect derives", were the derive macro is over constraining the impl on T: Clone unnecessarily. That's why your manual impl works. The suggestion needs to be silenced, and ideally what I just said explained in a note, likely with a suggestion to write a manual impl.

@estebank
Copy link
Contributor

I believe this might be a dupe of #110446

@bluenote10
Copy link
Contributor Author

I believe this might be a dupe of #110446

You're right, it most likely is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` C-bug Category: This is a bug. D-confusing Diagnostics: Confusing error or lint that should be reworked. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. S-has-mcve Status: A Minimal Complete and Verifiable Example has been found for this issue T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants