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.

Sign up for GitHub

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

Jump to bottom

Properly handle feature-gated lints #72970

Merged
merged 1 commit into from
Jun 7, 2020
Merged

Properly handle feature-gated lints #72970

merged 1 commit into from
Jun 7, 2020

Conversation

OddCoincidence
Copy link
Contributor

Closes #72694

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @petrochenkov (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 4, 2020
LeSeulArtichaut
LeSeulArtichaut reviewed Jun 4, 2020
View reviewed changes
Copy link
Contributor

@LeSeulArtichaut LeSeulArtichaut left a comment

Choose a reason for hiding this comment

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

Welcome to the Rust contributors 😄

src/librustc_lint/context.rs Outdated Show resolved Hide resolved
src/librustc_lint/context.rs Outdated Show resolved Hide resolved
src/librustc_lint/context.rs Outdated Show resolved Hide resolved
LeSeulArtichaut
LeSeulArtichaut approved these changes Jun 5, 2020
View reviewed changes
Copy link
Contributor

@LeSeulArtichaut LeSeulArtichaut left a comment

Choose a reason for hiding this comment

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

Looks good to me now.

@LeSeulArtichaut
Copy link
Contributor

The only thing I'm not sure about is whether we should test this behavior, and if we should, how to do it. The only test that exists for now is the feature gate test for unsafe_block_in_unsafe_fn, which will eventually be stabilized. Maybe cc @nikomatsakis?

@petrochenkov
Copy link
Contributor

Looks like a purely internal refactoring, no special testing should be necessary.

r=me after squashing the commits

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 5, 2020
@nikomatsakis
Copy link
Contributor

Looks good to me too. I don't think further tests are needed. One thing is that it'd be nice to document this but I guess we don't have coverage of creating lints in the rustc-dev-guide that I can see so I guess it's fine for now.

@OddCoincidence
properly handle feature gated lints
e7e6bc1 
- lint: add `feature_gate` field, allow specifying it in `declare_lint!`

- generalize the lint feature gate check

- librustdoc: generalize whitelisting of feature-gated lints
@OddCoincidence
Copy link
Contributor Author

@petrochenkov I squashed the commits as requested

One thing is that it'd be nice to document this but I guess we don't have coverage of creating lints in the rustc-dev-guide that I can see so I guess it's fine for now.

Perhaps my next contribution could be adding a new lint, and in the process I could add that coverage to the dev guide.

@petrochenkov
Copy link
Contributor

Thanks!
@bors r+

@bors
Copy link
Contributor

bors commented Jun 5, 2020

📌 Commit e7e6bc1 has been approved by petrochenkov

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 5, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jun 6, 2020
@Dylan-DPC
Rollup merge of rust-lang#72970 - OddCoincidence:feature-gated-lints,…
43202b4 
… r=petrochenkov

Properly handle feature-gated lints

Closes rust-lang#72694
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Jun 6, 2020
Closed
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jun 6, 2020
@Dylan-DPC
Rollup merge of rust-lang#72970 - OddCoincidence:feature-gated-lints,…
d3cf890 
… r=petrochenkov

Properly handle feature-gated lints

Closes rust-lang#72694
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Jun 6, 2020
Closed
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Jun 7, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 7, 2020
@bors
Auto merge of rust-lang#73093 - Dylan-DPC:rollup-9gh5tyu, r=Dylan-DPC
f9fdf64 
Rollup of 5 pull requests

Successful merges:

 - rust-lang#72764 (Be more careful around ty::Error in generators)
 - rust-lang#72908 (rename FalseEdges -> FalseEdge)
 - rust-lang#72970 (Properly handle feature-gated lints)
 - rust-lang#72998 (Mention that some atomic operations may not be available on some platforms)
 - rust-lang#73063 (Elide type on liballoc vec)

Failed merges:

r? @ghost
@bors bors merged commit 1ff0ba0 into rust-lang:master Jun 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LeSeulArtichaut LeSeulArtichaut LeSeulArtichaut approved these changes

Assignees

@petrochenkov petrochenkov

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Properly handle feature-gated lints
6 participants
@OddCoincidence @rust-highfive @LeSeulArtichaut @petrochenkov @nikomatsakis @bors