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

Resolve: stop requiring use and extern crate declarations to precede statements in blocks #31144

Merged
merged 1 commit into from
Jan 29, 2016

Conversation

jseyfried
Copy link
Contributor

We no longer require use and extern crate items to precede other items in modules thanks to RFC #385, but we still require use and extern crate items to precede statements in blocks (other items can appear anywhere in a block).

I think that this is a needless distinction between imports and other items that contradicts the intent of the RFC.

@rust-highfive
Copy link
Collaborator

r? @alexcrichton

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

@nagisa
Copy link
Member

nagisa commented Jan 23, 2016

I personally think that lifting the restriction in question could only result in worse code. Perhaps we should downgrade it to a warning-by-default lint?

@jseyfried
Copy link
Contributor Author

@nagisa
Do you think it is ever a good idea to have a module, struct, or enum follow a statement?

If so, I think this example (for modules, also applies to enums) and this example (for structs) from the RFC motivate allowing use items as well.

If not, I think we should not permit any item to follow a statement (or at least make it a warning/lint). I would be OK with this, but I don't see any reason to only permit a certain class of item like we are doing now.

@alexcrichton
Copy link
Member

r? @nrc

cc @rust-lang/lang

@rust-highfive rust-highfive assigned nrc and unassigned alexcrichton Jan 24, 2016
@huonw
Copy link
Member

huonw commented Jan 24, 2016

I feel like this would allow something like

fn foo() {}

fn bar() {
    foo();
    use qux::foo;
    foo();
}

which would presumably be calling qux::foo twice, but doesn't look like it. This isn't such a problem with normal items because there's no layered scoping like this (i.e. you don't inherit items in scope from a parent inside a mod, everything has to be imported/declared explicitly), and we disallow repeated definitions of a name inside a single scope.

@jseyfried
Copy link
Contributor Author

@huonw I agree that your example is problematic, but we currently allow

fn foo() {}
fn bar() {
    foo(); // Prints "foo!"
    fn foo() { println!("foo!"); }
    foo(); // Prints "foo!"
}

which I think is problematic for exactly the same reason.
I don't see why we would allow my example but not yours.

@huonw
Copy link
Member

huonw commented Jan 24, 2016

I don't think something confusing existing already is a good justification for adding more confusing things. :) Don't get me wrong though: uniformity is a good reason.

@petrochenkov
Copy link
Contributor

+1 to this PR, item scoping in blocks may already be surprising and needs to be explained well, and random exceptions don't make things easier to explain.

@oli-obk
Copy link
Contributor

oli-obk commented Jan 24, 2016

clippy now has an items_after_statements lint that does these checks, except if a macro expands to an item.

@nrc nrc added I-nominated T-lang Relevant to the language team, which will review and decide on the PR/issue. and removed I-nominated labels Jan 25, 2016
@nrc
Copy link
Member

nrc commented Jan 28, 2016

@bors: r+

@bors
Copy link
Contributor

bors commented Jan 28, 2016

📌 Commit f05bc16 has been approved by nrc

@bors
Copy link
Contributor

bors commented Jan 29, 2016

⌛ Testing commit f05bc16 with merge f030d1f...

bors added a commit that referenced this pull request Jan 29, 2016
…r=nrc

We no longer require `use` and `extern crate` items to precede other items in modules thanks to [RFC #385](rust-lang/rfcs#385), but we still require `use` and `extern crate` items to precede statements in blocks (other items can appear anywhere in a block).

I think that this is a needless distinction between imports and other items that contradicts the intent of the RFC.
@bors bors merged commit f05bc16 into rust-lang:master Jan 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants