Improve unused `extern crate` and unused `#[macro_use]` warnings #39060

jseyfried · 2017-01-14T09:59:24Z

This PR

adds unused_imports warnings for unused #[macro_use] extern crate macro imports,
improves unused_extern_crates warnings (avoids false negatives), and
removes unused #[macro_use] imports and unused extern crates.

petrochenkov · 2017-01-14T11:40:28Z

src/librustc_errors/lib.rs

And all these unused crates will still live undetected in Cargo.tomls and introduce build dependencies :(
rustc needs to somehow communicate this information to cargo so it could warn too.
cc @alexcrichton

It's true yeah that it needs to be propagated outwards, but this may not need rustc -> cargo support. The compiler could simply warn about unused --extern flags, and we could perhaps tailor it so the warning makes sense if you're just using Cargo.

In any case that seems like future work to me.

alexcrichton · 2017-01-14T17:50:58Z

makes unused_extern_crates warn by default instead of allow by default.

I've been wary of doing this in the past because linking a crate can have side effects other than wanting to pull in items/macros. For example you might link an allocator or just link a crate which contains a native library. I'd personally still prefer that the lint were allow by default.

jseyfried · 2017-01-15T01:54:16Z

@alexcrichton
Do you think it would be feasible to track crates that link an allocator, contain a native library, or have other "side effects" from linking to avoid emitting unused warnings for these crates?

alexcrichton · 2017-01-15T17:29:00Z

@jseyfried I haven't though too closely about how we might solve that problem, although it would likely involve taking a look at the crates that just that one crate pulls in (e.g. not shared dependencies from other crates) and then seeing what items it pulls in (e.g. any native libs, allocators, etc.)

jseyfried · 2017-01-16T04:02:55Z

@alexcrichton Ok, seems feasible but not easy -- I might try later in another PR.
@nrc I removed the commit makes unused_extern_crates warn-by-default instead of allow-by-default.

KalitaAlexey · 2017-01-16T08:27:30Z

@alexcrichton, @jseyfried
Why can't we make unused_extern_crates to be warn-by-default and make a developer write #[allow(unused_extern_crates)] where it is applicable?

petrochenkov · 2017-01-16T09:56:59Z

@alexcrichton
There's some existing practice regarding unused lints, for example

let m = MyMutex;

is reported by the unused_variables lint despite m being used for its destructor.
The user has to actively suppress it, e.g. by prefixing the variable name with _.
Using crates for linking only is relatively exotic, detecting unused usual crates seems by default seems more useful.
@jseyfried what was the ratio "true positive" / "false positive" (for rustc bootstrap) for unused_extern_crates before it was made allow-by-default?

alexcrichton · 2017-01-16T18:13:36Z

@KalitaAlexey @petrochenkov

I'm merely stating my opinion that I would like to not have this lint turned on by default. I personally feel very strongly that if a lint has a false positive it should be allow-by-default, but that's mostly just me.

KalitaAlexey · 2017-01-16T18:18:24Z

@petrochenkov,
But what if a false positive is very rare?

jseyfried · 2017-01-16T22:20:35Z

@petrochenkov There were 31 "true positives" and 1 "false positive" in rustc bootstrap.

nrc · 2017-01-18T01:13:42Z

@bors: r+

bors · 2017-01-18T01:13:43Z

📌 Commit 2b1d677 has been approved by nrc

Rollup of 28 pull requests - Successful merges: #38603, #38761, #38842, #38847, #38955, #38966, #39062, #39068, #39077, #39111, #39112, #39114, #39118, #39120, #39132, #39135, #39138, #39142, #39143, #39146, #39157, #39166, #39167, #39168, #39179, #39184, #39195, #39197 - Failed merges: #39060, #39145

bors · 2017-01-21T03:28:08Z

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

jseyfried · 2017-01-21T08:46:15Z

@alexcrichton Thanks!
@bors r=nrc

bors · 2017-01-21T08:46:15Z

📌 Commit 7753f19 has been approved by nrc

bors · 2017-01-21T10:58:17Z

⌛ Testing commit 7753f19 with merge f117e15...

bors · 2017-01-21T11:47:25Z

💔 Test failed - status-travis

alexcrichton

Btw if you want to annotate crates in tree to deny this lint by default I'd be down for that

alexcrichton · 2017-01-21T12:14:06Z

src/libtest/lib.rs

This, while unused, is intended to be linked for the side effects.

Fixed -- thanks again :)

bors · 2017-01-21T23:33:31Z

📌 Commit 9e9262b has been approved by nrc

bors · 2017-01-22T00:25:26Z

⌛ Testing commit 9e9262b with merge 7a8c8af...

bors · 2017-01-22T01:18:33Z

💔 Test failed - status-travis

jseyfried · 2017-01-22T01:31:34Z

@bors r=nrc

bors · 2017-01-22T01:31:34Z

📌 Commit 191abc4 has been approved by nrc

bors · 2017-01-22T03:41:47Z

⌛ Testing commit 191abc4 with merge e5b0829...

@nrc

Improve unused `extern crate` and unused `#[macro_use]` warnings This PR - adds `unused_imports` warnings for unused `#[macro_use] extern crate` macro imports, - improves `unused_extern_crates` warnings (avoids false negatives), and - removes unused `#[macro_use]` imports and unused `extern crate`s. r? @nrc

bors · 2017-01-22T06:40:47Z

☀️ Test successful - status-appveyor, status-travis
Approved by: nrc
Pushing e5b0829 to master...

rust-highfive assigned nrc Jan 14, 2017

jseyfried force-pushed the improve_unused branch 2 times, most recently from 47c8ba5 to 4a90ee4 Compare January 14, 2017 10:51

petrochenkov reviewed Jan 14, 2017

View reviewed changes

jseyfried force-pushed the improve_unused branch 2 times, most recently from 2ae2155 to 3186708 Compare January 15, 2017 01:50

jseyfried force-pushed the improve_unused branch 3 times, most recently from 4785cf4 to ec51ba3 Compare January 15, 2017 06:19

jseyfried force-pushed the improve_unused branch 3 times, most recently from 67dc901 to 2b1d677 Compare January 16, 2017 03:57

jseyfried mentioned this pull request Jan 16, 2017

Implement #[proc_macro_attribute] #38842

Merged

alexcrichton mentioned this pull request Jan 19, 2017

Rollup of 28 pull requests #39199

Merged

Improve unused_extern_crate warnings.

2efec3c

jseyfried force-pushed the improve_unused branch from 2b1d677 to 7753f19 Compare January 21, 2017 07:42

alexcrichton reviewed Jan 21, 2017

View reviewed changes

jseyfried force-pushed the improve_unused branch 2 times, most recently from 60240aa to 9e9262b Compare January 21, 2017 23:33

jseyfried force-pushed the improve_unused branch from 9e9262b to 191abc4 Compare January 22, 2017 01:30

jseyfried added 2 commits January 22, 2017 01:31

Warn on unused #[macro_use] imports.

356fa2c

Remove unused extern crates.

191abc4

bors merged commit 191abc4 into rust-lang:master Jan 22, 2017

This was referenced Jan 22, 2017

Privatize constructors of tuple structs with private fields #38932

Merged

Refactor TokenStream #39173

Merged

jseyfried deleted the improve_unused branch January 22, 2017 06:41

Ms2ger mentioned this pull request Jan 23, 2017

unused-extern-crates causes false positive with macro_use #30849

Closed

jseyfried mentioned this pull request Mar 14, 2017

Tracking issue for RFC 1566: Procedural macros #38356

Closed

31 tasks

Improve unused extern crate and unused #[macro_use] warnings #39060

Improve unused extern crate and unused #[macro_use] warnings #39060

Uh oh!

Conversation

jseyfried commented Jan 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petrochenkov Jan 14, 2017

Choose a reason for hiding this comment

Uh oh!

alexcrichton Jan 14, 2017

Choose a reason for hiding this comment

Uh oh!

alexcrichton commented Jan 14, 2017

Uh oh!

jseyfried commented Jan 15, 2017

Uh oh!

alexcrichton commented Jan 15, 2017

Uh oh!

jseyfried commented Jan 16, 2017

Uh oh!

KalitaAlexey commented Jan 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petrochenkov commented Jan 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexcrichton commented Jan 16, 2017

Uh oh!

KalitaAlexey commented Jan 16, 2017

Uh oh!

jseyfried commented Jan 16, 2017

Uh oh!

nrc commented Jan 18, 2017

Uh oh!

bors commented Jan 18, 2017

Uh oh!

bors commented Jan 21, 2017

Uh oh!

jseyfried commented Jan 21, 2017

Uh oh!

bors commented Jan 21, 2017

Uh oh!

bors commented Jan 21, 2017

Uh oh!

bors commented Jan 21, 2017

Uh oh!

alexcrichton left a comment

Choose a reason for hiding this comment

Uh oh!

alexcrichton Jan 21, 2017

Choose a reason for hiding this comment

Uh oh!

jseyfried Jan 21, 2017

Choose a reason for hiding this comment

Uh oh!

bors commented Jan 21, 2017

Uh oh!

bors commented Jan 22, 2017

Uh oh!

bors commented Jan 22, 2017

Uh oh!

jseyfried commented Jan 22, 2017

Uh oh!

bors commented Jan 22, 2017

Uh oh!

bors commented Jan 22, 2017

Uh oh!

bors commented Jan 22, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Improve unused `extern crate` and unused `#[macro_use]` warnings #39060

Improve unused `extern crate` and unused `#[macro_use]` warnings #39060

jseyfried commented Jan 14, 2017 •

edited

Loading

KalitaAlexey commented Jan 16, 2017 •

edited

Loading

petrochenkov commented Jan 16, 2017 •

edited

Loading