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

check constants even if they are not used in the current crate #32644

Merged
merged 1 commit into from
Apr 4, 2016

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Mar 31, 2016

For now this is just a warn-by-default lint. I suggest to make it a deny-by-default lint in the next release cycle (so no dependencies break), and then in another release cycle move to an error.

cc #19265
cc #3170

@rust-highfive
Copy link
Collaborator

r? @nrc

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

@@ -17,7 +17,8 @@ impl std::ops::Neg for S {
}

const _MAX: usize = -1;
//~^ ERROR unary negation of unsigned integer
//~^ WARN unary negation of unsigned integer
//~| ERROR unary negation of unsigned integer
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is odd - we shouldn't get a warning and an error

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well... we can turn the lint into a deny-by-default lint and then we'd never reach the error

@nrc
Copy link
Member

nrc commented Apr 1, 2016

r+ with the make tidy issues fixed.

@oli-obk oli-obk force-pushed the check_all_constants_early branch from d22bf20 to aa573a8 Compare April 1, 2016 07:19
@oli-obk
Copy link
Contributor Author

oli-obk commented Apr 1, 2016

fixed

@nrc
Copy link
Member

nrc commented Apr 1, 2016

@bors: r+

@bors
Copy link
Contributor

bors commented Apr 1, 2016

📌 Commit aa573a8 has been approved by nrc

@bors
Copy link
Contributor

bors commented Apr 2, 2016

☔ The latest upstream changes (presumably #32562) made this pull request unmergeable. Please resolve the merge conflicts.

@oli-obk oli-obk force-pushed the check_all_constants_early branch from aa573a8 to 913a2b4 Compare April 3, 2016 13:18
@oli-obk
Copy link
Contributor Author

oli-obk commented Apr 3, 2016

rebased

@nrc
Copy link
Member

nrc commented Apr 4, 2016

@bors: r+

@bors
Copy link
Contributor

bors commented Apr 4, 2016

📌 Commit 913a2b4 has been approved by nrc

@bors
Copy link
Contributor

bors commented Apr 4, 2016

⌛ Testing commit 913a2b4 with merge 6a3b558...

bors added a commit that referenced this pull request Apr 4, 2016
check constants even if they are not used in the current crate

For now this is just a `warn`-by-default lint. I suggest to make it a `deny`-by-default lint in the next release cycle (so no dependencies break), and then in another release cycle move to an error.

cc #19265
cc #3170
@bors bors merged commit 913a2b4 into rust-lang:master Apr 4, 2016
@oli-obk oli-obk deleted the check_all_constants_early branch April 5, 2016 07:07
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.

4 participants