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.

Sign up for GitHub

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

Jump to bottom

Allow unused extern crate again #44825

Merged
merged 1 commit into from
Sep 27, 2017
Merged

Allow unused extern crate again #44825

merged 1 commit into from
Sep 27, 2017

Conversation

dtolnay
Copy link
Member

@dtolnay dtolnay commented Sep 25, 2017

This is a partial revert of #42588. There is a usability concern reported in #44294 that was not considered in the discussion of the PR, so I would like to back this out of 1.21. As is, I think users would have a worse and more confusing experience with this lint enabled by default. We can re-enabled once there are better diagnostics or the case in #44294 does not trigger the lint.

@dtolnay
Allow unused extern crate again
247b58b 
This is a partial revert of rust-lang#42588. There is a usability concern
reported in rust-lang#44294 that was not considered in the discussion of the PR,
so I would like to back this out of 1.21. As is, I think users would
have a worse and more confusing experience with this lint enabled by
default. We can re-enabled once there are better diagnostics or the case
in rust-lang#44294 does not trigger the lint.
@rust-highfive
Copy link
Collaborator

r? @arielb1

(rust_highfive has picked a reviewer for you, use r? to override)

@arielb1
Copy link
Contributor

arielb1 commented Sep 25, 2017 •
edited
Loading

This area, both error messages and imports, is changing very rapidly. I think @aturon is taking the lead on imports - what do you think of disabling the lint?

@arielb1 arielb1 assigned aturon and unassigned arielb1 Sep 25, 2017
@arielb1
Copy link
Contributor

arielb1 commented Sep 25, 2017

r? @aturon

@est31
Copy link
Member

est31 commented Sep 25, 2017 •
edited
Loading

I had to disable the lint in multiple occasions in my code due to false positives of it... so definitely a 👍 from me. I'm not annoyed that I have to #![allow] the lint, I'm more annoyed that I have to allow it so many times. Optimally you could just do #![every_use_of_this_crate_is_legal] in your lib.rs and the lint won't fire for your crate. But until we have that disabling is probably a good path.

The issue (false positives connected to linking) has been discussed here before. EDIT: I've opened #44827 to discuss my suggestion.

I'd also say that there is probably another question to discuss which is how to handle the lint in the future when extern crate is treated as legacy. Will we still have the lint? Will rustc parse Cargo.toml? Or will it instead communicate that it didn't have use for a crate somehow to Cargo, which then prints the lint itself? That would mean more communication between rustc and cargo.

@bjorn3
Copy link
Member

bjorn3 commented Sep 25, 2017

Could we stop triggering the lint for crates with #[no_mangle] extern fn ... functions or #[link="..."] extern {/*...*/} in it.

@carols10cents carols10cents added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. beta-nominated Nominated for backporting to the compiler in the beta channel. labels Sep 25, 2017
@aturon
Copy link
Member

aturon commented Sep 25, 2017

@bors: r+

@bors
Copy link
Contributor

bors commented Sep 25, 2017

📌 Commit 247b58b has been approved by aturon

@arielb1 arielb1 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 Sep 26, 2017
@arielb1 arielb1 mentioned this pull request Sep 26, 2017
Merged
@alexcrichton
Copy link
Member

@bors: p=1

this is likely to be beta-accepted

bors added a commit that referenced this pull request Sep 27, 2017
@bors
Auto merge of #44825 - dtolnay:cratelint, r=aturon
412ac93 
Allow unused extern crate again

This is a partial revert of #42588. There is a usability concern reported in #44294 that was not considered in the discussion of the PR, so I would like to back this out of 1.21. As is, I think users would have a worse and more confusing experience with this lint enabled by default. We can re-enabled once there are better diagnostics or the case in #44294 does not trigger the lint.
@bors
Copy link
Contributor

bors commented Sep 27, 2017

⌛ Testing commit 247b58b with merge 412ac93...

@bors
Copy link
Contributor

bors commented Sep 27, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: aturon
Pushing 412ac93 to master...

@bors bors merged commit 247b58b into rust-lang:master Sep 27, 2017
@nikomatsakis nikomatsakis added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Sep 27, 2017
@nikomatsakis
Copy link
Contributor

Marking as beta-accepted. Small issue. cc @rust-lang/compiler

@nikomatsakis
Copy link
Contributor

Backporting now.

This was referenced Sep 28, 2017
Closed
Merged
@alexcrichton alexcrichton removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Sep 28, 2017
bors added a commit that referenced this pull request Sep 28, 2017
@bors
Auto merge of #44908 - nikomatsakis:beta-20170928, r=alexcrichton
d158897 
Beta 20170928

Backports of:

- Allow unused extern crate again #44825
- macros: fix bug in collecting trait and impl items with derives. #44757
-  `--cap-lints allow` switches off `can_emit_warnings` #44627
-  Update the libc submodule #44116
- limit and clear cache obligations opportunistically #44269
- clear out projection subobligations after they are processed #43999
@dtolnay dtolnay deleted the cratelint branch October 1, 2017 15:40
bors added a commit that referenced this pull request Oct 7, 2017
@bors
Auto merge of #44908 - nikomatsakis:beta-20170928, r=alexcrichton
aa30011 
Beta 20170928

Backports of:

- Allow unused extern crate again #44825
- macros: fix bug in collecting trait and impl items with derives. #44757
-  `--cap-lints allow` switches off `can_emit_warnings` #44627
-  Update the libc submodule #44116
- limit and clear cache obligations opportunistically #44269
- clear out projection subobligations after they are processed #43999
-  fix logic error in #44269's `prune_cache_value_obligations` #45065
- REVERTS #43543: Cleanup some remains of `hr_lifetime_in_assoc_type` compatibility lint:
@dtolnay dtolnay mentioned this pull request Nov 30, 2017
Closed
@dtolnay dtolnay mentioned this pull request Mar 28, 2018
Closed
@petrochenkov petrochenkov mentioned this pull request Aug 19, 2018
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@aturon aturon

Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@dtolnay @rust-highfive @arielb1 @est31 @bjorn3 @aturon @bors @alexcrichton @nikomatsakis @carols10cents