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

`use super::*` importing private members from parent module #38176

Closed
sgrif opened this Issue Dec 5, 2016 · 6 comments

Comments

Projects
None yet
5 participants
@sgrif
Copy link
Contributor

sgrif commented Dec 5, 2016

Reproduction script: https://is.gd/qvbF0b

This code compiles on stable and beta but fails to compile on nightly. use super::* previously did not include private members. On nightly it is reported that "Foo is ambiguous"

@bluss

This comment has been minimized.

Copy link
Contributor

bluss commented Dec 5, 2016

It's an intentional change, but everything that makes stable code stop compile is classed as a regression initially.

@sgrif

This comment has been minimized.

Copy link
Contributor Author

sgrif commented Dec 5, 2016

Figured as much. I believe technically this is a "minor breaking change" since code can be forwards compatibly changed to handle this by not using glob imports, right? Is it still a regression in that case? Either way a release where this warns instead of erroring would definitely be nice.

@jseyfried

This comment has been minimized.

Copy link
Contributor

jseyfried commented Dec 6, 2016

@sgrif right, there can only be a regression when these conditions are met.

We landed this change #37127 without a warning cycle because Crater found no breakage. It would be feasible to implement a warning cycle as detailed in #35120 (comment).

cc @nrc

@sgrif

This comment has been minimized.

Copy link
Contributor Author

sgrif commented Dec 6, 2016

As an aside, I don't think that Crater finding no breakage should be used as justification for skipping warning cycles. It does not examine all available Rust code, or even all code on crates.io

@jseyfried jseyfried self-assigned this Dec 8, 2016

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Dec 8, 2016

Discussed in @rust-lang/compiler meeting. Indeed some fallout from these changes was expected but a warning cycle is still appropriate. The only reason to skip a warning cycle is supposed to be if a warning cycle is technically infeasible -- in which case we are to use crater to make a best effort to have all affected crates get updated beforehand.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Dec 8, 2016

triage: P-high

Specifically, high priority to add a warning cycle.

@rust-highfive rust-highfive added the P-high label Dec 8, 2016

ustulation added a commit to ustulation/cargo-prune that referenced this issue Dec 15, 2016

test doc.rs
This will pass build on stable but fail current nightly due to rust-lang/rust#38176 . So we are just checking if doc.rs generates stuff if nightly fails but stable passes

ustulation added a commit to ustulation/cargo-prune that referenced this issue Dec 15, 2016

test doc.rs
This will pass build on stable but fail current nightly due to rust-lang/rust#38176 . So we are just checking if doc.rs generates stuff if nightly fails but stable passes

bors added a commit that referenced this issue Dec 20, 2016

Auto merge of #38271 - jseyfried:rfc_1560_warning_cycle, r=nrc
resolve: change most backwards incompatible ambiguity errors to `legacy_imports` warnings

Fixes #38176.
r? @nrc or @nikomatsakis

@bors bors closed this in #38271 Dec 20, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.