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

RFC: Prevent lint changes being a breaking change #1193

Merged
merged 3 commits into from Jul 24, 2015

Conversation

Projects
None yet
8 participants
@alexcrichton
Copy link
Member

commented Jul 8, 2015

Add a new flag to the compiler, --cap-lints, which set the maximum possible
lint level for the entire crate (and cannot be overridden). Cargo will then pass
--cap-lints allow to all upstream dependencies when compiling code.

Fixes #1029.

Rendered

RFC: Prevent lint changes being a breaking change
Add a new flag to the compiler, `--cap-lints`, which set the maximum possible
lint level for the entire crate (and cannot be overridden). Cargo will then pass
`--cap-lints allow` to all upstream dependencies when compiling code.
@alexcrichton

This comment has been minimized.

Copy link
Member Author

commented Jul 8, 2015

# Drawbacks
This RFC adds surface area to the command line of the compiler with a relatively
obscure option `--cap-lints`. The option will almost never be passed by anything

This comment has been minimized.

Copy link
@sfackler

sfackler Jul 8, 2015

Member

We could hide it away in e.g. -Z to keep it from polluting --help.

This comment has been minimized.

Copy link
@alexcrichton

alexcrichton Jul 8, 2015

Author Member

We do currently kinda have this already, the --help --verbose page is a little longer than --help. I'd also prefer to continue to consider -Z as "unstable flags", aka flags that Cargo should avoid.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

commented Jul 8, 2015

Big 👍 from me.

@nrc

This comment has been minimized.

Copy link
Member

commented Jul 8, 2015

+1 from me.

```

For example when `--cap-lints allow` is passed, all instances of `#[warn]`,
`#[deny]`, and `#[forbid] are ignored. If, however `--cap-lints warn` is passed

This comment has been minimized.

Copy link
@huonw

huonw Jul 13, 2015

Member

(`#[forbid] needs a closing backtick)

obscure option `--cap-lints`. The option will almost never be passed by anything
other than Cargo, so having it show up here is a little unfortunate.
Some crates may inadvertently rely on memory safety through lints, or otherwise

This comment has been minimized.

Copy link
@huonw

huonw Jul 13, 2015

Member

I think it's not entirely unreasonable for crates to rely on their own custom lints to guarantee memory safety (I think servo does this). The key point being custom. One way to tackle this is for the capping to only apply to built-in lints?

This comment has been minimized.

Copy link
@alexcrichton

alexcrichton Jul 13, 2015

Author Member

That makes sense to me, although it's possible that modifications to the compiler could also cause custom lints to trigger more warnings. For example if the compiler collected a set of "mutable variables never used mutably" and then a custom lint (e.g. pretend we didn't have a built-in one) iterated this set and issued warnings, when we fix the compiler to add more items to this set (if they were forgotten earlier), the change would bubble down to custom lints.

I do agree though that we could probably tackle this later on. Custom lints could perhaps register themselves as "not having a cap" which could opt-in to possibly breaking changes.

@pcwalton

This comment has been minimized.

Copy link
Contributor

commented Jul 16, 2015

After being bit hard by deny(missing_docs) Armageddon, I think this is a must-have.

@jroesch

This comment has been minimized.

Copy link
Member

commented Jul 16, 2015

👍 Alex!

@alexcrichton

This comment has been minimized.

Copy link
Member Author

commented Jul 16, 2015

The tools team has decided to place this RFC in its final comment period.

brson added a commit to brson/rust that referenced this pull request Jul 20, 2015

Revert "Fix `missing_docs` lint for const and static."
This reverts commit 00130cf.

As mentioned in a regression report[1], this caused a notable amount
of breakage.  Because there's a plan to mitigate[2] this type of
breakage, I'm reverting this until then.

[1]: https://internals.rust-lang.org/t/new-crater-reports-1-1-stable-vs-beta-2015-07-10-and-nightly-2015-07-10/2358
[2]: rust-lang/rfcs#1193

bors added a commit to rust-lang/rust that referenced this pull request Jul 21, 2015

Auto merge of #27160 - brson:revdoc, r=huonw
This reverts commit 00130cf.

As mentioned in a regression report[1], this caused a notable amount
of breakage. Because there's a plan to mitigate[2] this type of
breakage, I'm reverting this until then.

[1]: https://internals.rust-lang.org/t/new-crater-reports-1-1-stable-vs-beta-2015-07-10-and-nightly-2015-07-10/2358
[2]: rust-lang/rfcs#1193

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Jul 24, 2015

rustc: Add a --cap-lints flag to the compiler
This commit is an implementation of [RFC 1193][rfc] which adds the ability to
the compiler to cap the lint level for the entire compilation session. This flag
will ensure that no lints will go above this level, and Cargo will primarily use
this flag passing `--cap-lints allow` to all upstream dependencies.

[rfc]: rust-lang/rfcs#1193
@alexcrichton

This comment has been minimized.

Copy link
Member Author

commented Jul 24, 2015

The consensus of the tools team is to merge this RFC, so I'm going to do so!

Tracking issue

@alexcrichton alexcrichton merged commit ba2f1fe into rust-lang:master Jul 24, 2015

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Jul 24, 2015

rustc: Add a --cap-lints flag to the compiler
This commit is an implementation of [RFC 1193][rfc] which adds the ability to
the compiler to cap the lint level for the entire compilation session. This flag
will ensure that no lints will go above this level, and Cargo will primarily use
this flag passing `--cap-lints allow` to all upstream dependencies.

[rfc]: rust-lang/rfcs#1193

Closes rust-lang#27259

@alexcrichton alexcrichton deleted the alexcrichton:cap-lints branch Jul 24, 2015

@alexcrichton alexcrichton restored the alexcrichton:cap-lints branch Jul 24, 2015

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Jul 29, 2015

rustc: Add a --cap-lints flag to the compiler
This commit is an implementation of [RFC 1193][rfc] which adds the ability to
the compiler to cap the lint level for the entire compilation session. This flag
will ensure that no lints will go above this level, and Cargo will primarily use
this flag passing `--cap-lints allow` to all upstream dependencies.

[rfc]: rust-lang/rfcs#1193

Closes rust-lang#27259

bors added a commit to rust-lang/rust that referenced this pull request Jul 29, 2015

Auto merge of #27260 - alexcrichton:cap-lints, r=nrc
This commit is an implementation of [RFC 1193][rfc] which adds the ability to
the compiler to cap the lint level for the entire compilation session. This flag
will ensure that no lints will go above this level, and Cargo will primarily use
this flag passing `--cap-lints allow` to all upstream dependencies.

[rfc]: rust-lang/rfcs#1193

Closes #27259
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.