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

Forbid glob-importing from a type alias #32134

Merged
merged 3 commits into from
Mar 11, 2016

Conversation

jseyfried
Copy link
Contributor

This PR forbids glob-importing from a type alias or trait (fixes #30560):

type Alias = ();
use Alias::*; // This is currently allowed but shouldn't be

This is a [breaking-change]. Since the disallowed glob imports don't actually import anything, any breakage can be fixed by removing the offending glob import.

r? @alexcrichton

@jseyfried
Copy link
Contributor Author

I could also make this a warning for a warning cycle.
cc @petrochenkov

@petrochenkov
Copy link
Contributor

trait Tr {}
use Tr::*;

also compiles.
(I've checked other items, they all seem to behave appropriately.)

@jseyfried
Copy link
Contributor Author

Good point, I'll forbid that too.

@alexcrichton
Copy link
Member

Thanks @jseyfried! I'll run crater to see what kind of fallout we're looking at.

@alexcrichton alexcrichton self-assigned this Mar 8, 2016
@alexcrichton
Copy link
Member

Looks like there are six root regressions, but it looks like most of those are actually just rooted in my own mistake. I'm personally always a fan of aggressive bug fixing, so I would be ok with pushing forward without a warning cycle, but others might feel differently!

So in terms of code, r=me, but in terms of scheduling/warning, I'll defer to @rust-lang/lang

cc @nikomatsakis

@alexcrichton
Copy link
Member

er, r? @nikomatsakis

@alexcrichton
Copy link
Member

To put the numbers into context, 12/13 regressions were fixed by the new versions of libgit2-sys I just published, so it seems like a relatively low-impact change to me. It's definitely "weird" code that would exercise this.

@jseyfried
Copy link
Contributor Author

The remaining regression is in angle-0.1.1 and is caused by glob importing from a trait.

@jseyfried jseyfried force-pushed the forbid_type_alias_as_module branch from 76781d8 to 1a6092e Compare March 9, 2016 05:19
TimNN added a commit to TimNN/angle-rs that referenced this pull request Mar 9, 2016
The import should have had no effect and will become an error once rust-lang/rust#32134 lands.
@nikomatsakis
Copy link
Contributor

I think most everyone would agree this is a straight-up bug fix and it's ok to Just Fix It.

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Mar 11, 2016

📌 Commit 1a6092e has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Mar 11, 2016

⌛ Testing commit 1a6092e with merge 5807fbb...

bors added a commit that referenced this pull request Mar 11, 2016
…atsakis

Forbid glob-importing from a type alias

This PR forbids glob-importing from a type alias or trait (fixes #30560):
```rust
type Alias = ();
use Alias::*; // This is currently allowed but shouldn't be
```

This is a [breaking-change]. Since the disallowed glob imports don't actually import anything, any breakage can be fixed by removing the offending glob import.

r? @alexcrichton
@bors bors merged commit 1a6092e into rust-lang:master Mar 11, 2016
@jseyfried jseyfried deleted the forbid_type_alias_as_module branch March 25, 2016 22:56
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.

rustdoc: panic caused by a "reexport" of a type alias
5 participants