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

missing-copy-implementations warning only triggers on externally-reachable types. #19712

Closed
ben0x539 opened this issue Dec 10, 2014 · 10 comments
Closed
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added.

Comments

@ben0x539
Copy link
Contributor

example, foo.rs:

#![crate_type = "lib"]
mod inner {
    pub struct Foo {
        pub field: i32
    }
}

pub fn foo() -> inner::Foo {
    inner::Foo { field: 42 }
}

bar.rs:

extern crate foo;

fn main() {
    let f = foo::foo();
    drop(f);
    println!("{}", f.field);
}

foo compiles cleanly, bar doesn't compile because oh no! I've forgotten to make foo::inner::Foo Copyable.

@sfackler
Copy link
Member

I believe this is intentional.

@tbu-
Copy link
Contributor

tbu- commented Dec 10, 2014

This might be a problem though, if we had a private uint wrapper:

 struct MyPrivateUint(uint);

And use that inside of a public struct:

 pub struct Index { idx: MyPrivateUint };

@kmcallister kmcallister added the A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. label Jan 17, 2015
@steveklabnik
Copy link
Member

Triage: this compiles today. Closing.

@tbu-
Copy link
Contributor

tbu- commented Feb 2, 2016

What compiles today? The examples give me

bar.rs:6:20: 6:27 error: use of moved value: `f.field` [E0382]
bar.rs:6     println!("{}", f.field);
                            ^~~~~~~
<std macros>:2:25: 2:56 note: in this expansion of format_args!
<std macros>:3:1: 3:54 note: in this expansion of print! (defined in <std macros>)
bar.rs:6:5: 6:29 note: in this expansion of println! (defined in <std macros>)
bar.rs:6:20: 6:27 help: run `rustc --explain E0382` to see a detailed explanation
bar.rs:5:10: 5:11 note: `f` moved here because it has type `foo::inner::Foo`, which is non-copyable
bar.rs:5     drop(f);
                  ^
error: aborting due to previous error

without a missing-copy-implementation in the foo.rs compilation.

@steveklabnik
Copy link
Member

Whoops! I don't know what I did wrong, but when I had typed that, the example verbatim compiled. Trying again, I get the same error as you.

I must have screwed up a filename or something.

@steveklabnik steveklabnik reopened this Feb 2, 2016
@petrochenkov
Copy link
Contributor

This is a legitimate bug, rustc_privacy::EmbargoVisitor has a large piece of functionality missing - function return types and other types available through everything except for reexports and type aliases are not marked as reachable. This leads to many kinds of problems for such types.

@petrochenkov
Copy link
Contributor

Fixed by #31641, needs a test.

@petrochenkov petrochenkov added E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. and removed A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. labels Feb 19, 2017
@petrochenkov petrochenkov added the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Mar 1, 2017
@topecongiro
Copy link
Contributor

The topmost example still fails. Tested onrustc 1.17.0-nightly (691eba135 2017-03-01) and rustc 1.15.1 (021bd294c 2017-02-08).

@petrochenkov
Copy link
Contributor

@topecongiro
missing_copy_implementations is allow-by-default, you need to explicitly enable it to test this:

#![deny(missing_copy_implementations)]

mod inner {
    pub struct Foo {
        pub field: i32
    }
}

pub fn foo() -> inner::Foo {
    inner::Foo { field: 42 }
}

fn main() {}

Result:

rustc 1.17.0-nightly (b1e31766d 2017-03-03)
error: type could implement `Copy`; consider adding `impl Copy`
 --> <anon>:4:5
  |
4 |       pub struct Foo {
  |  _____^ starting here...
5 | |         pub field: i32
6 | |     }
  | |_____^ ...ending here
  |
note: lint level defined here
 --> <anon>:1:9
  |
1 | #![deny(missing_copy_implementations)]
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

@topecongiro
Copy link
Contributor

I see. I should have searched other issues before commenting. Thank you for you reply!

bors added a commit that referenced this issue Mar 6, 2017
Add missing tests for 'E-needstest' labeled issues

This PR adds missing tests for issue #35988, #19712, ~~#18627~~, #24947, #28600 and #34751.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added.
Projects
None yet
Development

No branches or pull requests

7 participants