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

rustc: Trim down the `rust_2018_idioms` lint group #52926

Merged
merged 1 commit into from Aug 1, 2018

Conversation

Projects
None yet
6 participants
@alexcrichton
Copy link
Member

commented Jul 31, 2018

These migration lints aren't all up to par in terms of a good migration
experience. Some, like unreachable_pub, hit bugs like #52665 and unprepared
macros to be handled enough of the time. Others like linting against
#[macro_use] are swimming upstream in an ecosystem that's not quite ready (and
slightly buggy pending a few current PRs).

The general idea is that we will continue to recommend the rust_2018_idioms
lint group as part of the transition guide (as an optional step) but we'll be
much more selective about which lints make it into this group. Only those with a
strong track record of not causing too much churn will make the cut.

cc #52679

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

commented Jul 31, 2018

r? @pnkfelix

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

ELLIPSIS_INCLUSIVE_RANGE_PATTERNS,
ELIDED_LIFETIMES_IN_PATHS

// FIXME(#52665) not always applicable and not all macros

This comment has been minimized.

Copy link
@zackmdavis

zackmdavis Aug 1, 2018

Member

#47816 also needs to be fixed. Between #52665 (comment) and #47816 (comment), it should at least be conceptually straightforward to eliminate the false-positives at the cost of accepting a lot of false negatives (although I expect #47816 to be solvable in general with more thought/attention).

This comment has been minimized.

Copy link
@alexcrichton

alexcrichton Aug 1, 2018

Author Member

Aha yes indeed!

rustc: Trim down the `rust_2018_idioms` lint group
These migration lints aren't all up to par in terms of a good migration
experience. Some, like `unreachable_pub`, hit bugs like #52665 and unprepared
macros to be handled enough of the time. Others like linting against
`#[macro_use]` are swimming upstream in an ecosystem that's not quite ready (and
slightly buggy pending a few current PRs).

The general idea is that we will continue to recommend the `rust_2018_idioms`
lint group as part of the transition guide (as an optional step) but we'll be
much more selective about which lints make it into this group. Only those with a
strong track record of not causing too much churn will make the cut.

cc #52679

@alexcrichton alexcrichton force-pushed the alexcrichton:trim-idioms-lints branch from 2256cad to 27b3cb5 Aug 1, 2018

@alexcrichton

This comment has been minimized.

Copy link
Member Author

commented Aug 1, 2018

@rust-highfive rust-highfive assigned oli-obk and unassigned pnkfelix Aug 1, 2018

@oli-obk

This comment has been minimized.

Copy link
Contributor

commented Aug 1, 2018

Makes sense to me.

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

commented Aug 1, 2018

📌 Commit 27b3cb5 has been approved by oli-obk

pietroalbini added a commit to pietroalbini/rust that referenced this pull request Aug 1, 2018

Rollup merge of rust-lang#52926 - alexcrichton:trim-idioms-lints, r=o…
…li-obk

rustc: Trim down the `rust_2018_idioms` lint group

These migration lints aren't all up to par in terms of a good migration
experience. Some, like `unreachable_pub`, hit bugs like rust-lang#52665 and unprepared
macros to be handled enough of the time. Others like linting against
`#[macro_use]` are swimming upstream in an ecosystem that's not quite ready (and
slightly buggy pending a few current PRs).

The general idea is that we will continue to recommend the `rust_2018_idioms`
lint group as part of the transition guide (as an optional step) but we'll be
much more selective about which lints make it into this group. Only those with a
strong track record of not causing too much churn will make the cut.

cc rust-lang#52679

bors added a commit that referenced this pull request Aug 1, 2018

Auto merge of #52958 - pietroalbini:rollup, r=pietroalbini
Rollup of 15 pull requests

Successful merges:

 - #52793 (Add test for NLL: unexpected "free region `` does not outlive" error )
 - #52799 (Use BitVector for global sets of AttrId)
 - #52809 (Add test for unexpected region for local data ReStatic)
 - #52834 ([NLL] Allow conflicting borrows of promoted length zero arrays)
 - #52835 (Fix Alias intra doc ICE)
 - #52854 (fix memrchr in miri)
 - #52899 (tests/ui: Add missing mips{64} ignores)
 - #52908 (Use SetLenOnDrop in Vec::truncate())
 - #52915 (Don't count MIR locals as borrowed after StorageDead when finding locals live across a yield terminator)
 - #52926 (rustc: Trim down the `rust_2018_idioms` lint group)
 - #52930 (rustc_resolve: record single-segment extern crate import resolutions.)
 - #52939 (Make io::Read::read_to_end consider io::Take::limit)
 - #52942 (Another SmallVec.extend optimization)
 - #52947 (1.27 actually added the `armv5te-unknown-linux-musleabi` target)
 - #52954 (async can begin expressions)

Failed merges:

r? @ghost

@bors bors merged commit 27b3cb5 into rust-lang:master Aug 1, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@alexcrichton alexcrichton deleted the alexcrichton:trim-idioms-lints branch Aug 3, 2018

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.