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

Do not warn when deriving Copy for raw pointers #21362

Merged
merged 2 commits into from
Feb 3, 2015
Merged

Do not warn when deriving Copy for raw pointers #21362

merged 2 commits into from
Feb 3, 2015

Conversation

aochagavia
Copy link
Contributor

Fixes #21272 and #21296

r? @cmr

@aochagavia
Copy link
Contributor Author

@cmr Is there a way to test the lack of a warning?

@emberian
Copy link
Member

@aochagavia you can use #[forbid(raw_pointer_derive)] in a run-pass test, and if the lint triggers the test will fail.

@aochagavia
Copy link
Contributor Author

@cmr Thanks! I have added a test.

@emberian
Copy link
Member

r? @huonw (You have the strongest opinions about this lint, I believe)

@rust-highfive rust-highfive assigned huonw and unassigned emberian Jan 18, 2015
@aochagavia
Copy link
Contributor Author

Any news on this?

@huonw
Copy link
Member

huonw commented Feb 1, 2015

Sorry, @aochagavia, lost track of this. I think it's good, just a small implementation nit.

ast::ItemImpl(..) => {
ast::ItemImpl(_, _, _, ref t_ref_opt, _, _) => {
// Deriving the Copy trait does not cause a warning
if let &Some(ref t_ref) = t_ref_opt {
Copy link
Member

Choose a reason for hiding this comment

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

Currently this code will mean that any trait called Copy isn't warned about, while the compiler actually has enough info to do it "perfectly", i.e. only avoid warning for the special Copy trait.

let trait_ref = ty::node_id_to_trait_ref(cx.tcx, t_ref.ref_id);
let def_id = ty::trait_ref_to_def_id(cx.tcx, trait_ref.def_id);
if Some(def_id) == cx.tcx.lang_items.copy_trait() {
    return
}

(I'm not 100% certain that will work, and I haven't compiled it.)

@huonw
Copy link
Member

huonw commented Feb 1, 2015

If I'm so slow again be feel to pester me.

@aochagavia
Copy link
Contributor Author

@huonw Addressed your nit. Thanks for reviewing!

@pnkfelix
Copy link
Member

pnkfelix commented Feb 2, 2015

@bors r=huonw 8b84f09

@pnkfelix
Copy link
Member

pnkfelix commented Feb 2, 2015

@bors r=huonw 8b84f09 rollup

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Feb 2, 2015
@bors bors merged commit 8b84f09 into rust-lang:master Feb 3, 2015
@sorccu
Copy link

sorccu commented Feb 4, 2015

Sorry to comment after closing, but should deriving Debug produce the raw_pointer_derive warning like it currently does?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Structs with a raw pointer warn whether or not you derive Copy
6 participants