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

Turn deprecation lint legacy_imports into a hard error #50760

Merged
merged 1 commit into from
May 19, 2018

Conversation

petrochenkov
Copy link
Contributor

Closes #38260

The lint was introduced in Dec 2016, then made deny-by-default in Jun 2017 when crater run found 0 regressions caused by it.

This lint requires some not entirely trivial amount of import resolution logic that (surprisingly or not) interacts with feature(use_extern_macros) (#35896), so it would be desirable to remove it before stabilizing use_extern_macros.
In particular, this PR fixes the failing example in #50725 (but not the whole issue, use std::panic::{self} still can cause other undesirable errors when use_extern_macros is enabled).

@rust-highfive
Copy link
Collaborator

r? @eddyb

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 14, 2018
@petrochenkov
Copy link
Contributor Author

r? @nikomatsakis

@nikomatsakis
Copy link
Contributor

Seems reasonable. =) Do we feel the need to do a final "cargo check" crater run? (So we can cc the relevant people)

@nikomatsakis
Copy link
Contributor

OTOH if previous runs found no regressions, and we are deny by default, seems like overkill.

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented May 15, 2018

📌 Commit d1b0274 has been approved by nikomatsakis

@bors bors 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 May 15, 2018
@bors
Copy link
Contributor

bors commented May 19, 2018

⌛ Testing commit d1b0274 with merge 413bc952821db174ec08d4338f7fbbc2ec468124...

@bors
Copy link
Contributor

bors commented May 19, 2018

💔 Test failed - status-appveyor

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 19, 2018
@kennytm
Copy link
Member

kennytm commented May 19, 2018

@bors retry

---- net::tcp::tests::connect_timeout_unbound stdout ----
thread 'net::tcp::tests::connect_timeout_unbound' panicked at 'called `Result::unwrap_err()` on an `Ok` value: TcpStream { addr: V4(127.0.0.1:3329), peer: V4(127.0.0.1:3329), socket: 408 }', libcore\result.rs:945:5

1 second is not enough for the OS to close the port?

@bors bors 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 May 19, 2018
@bors
Copy link
Contributor

bors commented May 19, 2018

⌛ Testing commit d1b0274 with merge ef8ee64...

bors added a commit that referenced this pull request May 19, 2018
Turn deprecation lint `legacy_imports` into a hard error

Closes #38260

The lint was introduced in Dec 2016, then made deny-by-default in Jun 2017 when crater run found 0 regressions caused by it.

This lint requires some not entirely trivial amount of import resolution logic that (surprisingly or not) interacts with `feature(use_extern_macros)` (#35896), so it would be desirable to remove it before stabilizing `use_extern_macros`.
In particular, this PR fixes the failing example in #50725 (but not the whole issue, `use std::panic::{self}` still can cause other undesirable errors when `use_extern_macros` is enabled).
@bors
Copy link
Contributor

bors commented May 19, 2018

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

@petrochenkov petrochenkov deleted the legimp branch June 5, 2019 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

legacy_imports future-compatibility warnings
6 participants