Repository navigation
Future-incompatible warnings should always print a warning, even if lints are allowed #34596
Description
Currently, if a future-incompatible lint is set to allow (either manually or via cap-lints), no warning is printed. This is bad because it means that if you author a library that is widely used, but which you are not actively using at the moment, you might not notice that it will break in the future -- moreover, your users won't either, since cargo will cap lints when it builds your library as a dependency.
So basically the behavior for future-incompatible lints would be that they can either be "warn" or "deny" but can never be completely silenced. If cap-lints is set to allow, then while other lints will be set to allow, the future-incompatible lints will be set to warn (but not deny).
cc @rust-lang/compiler @brson
Reacted by Brian Anderson, Jonas Schievink, Scott Olson, Sander Maijers, jD91mZM2, Andy Hayden, Kornel, Felix S Klock II, runiq, Aaron Hill and 2 more
Activity
nikomatsakis commentedon Jul 1, 2016
I'm marking this as mentor because I'd be happy to mentor it (though until July 15 I'm on vacation). I marked it as easy because think this should be relatively straightforward, though I may be wrong about that. I'll try to look at the code in a bit and print out a summary of what would need to be done.
hgallagher1993 commentedon Jul 1, 2016
brson commentedon Jul 1, 2016
hgallagher1993 commentedon Jul 11, 2016
pnkfelix commentedon Jul 11, 2016
hgallagher1993 commentedon Jul 11, 2016
nikomatsakis commentedon Jul 21, 2016
nikomatsakis commentedon Aug 2, 2016
@hgallagher1993 ok, sorry for the delay. I did a bit of digging. I think the changes you need to make are roughly this:
In
src/librustc/lint.rs, there is a function calledset_level-- it changes the level of reporting for a lint. Currently, it first checks something called thelint_cap, which records a maximum level (this is used by cargo so that you don't see "unused variable" warnings for your dependencies).What we want to do I think is change that function to also check whether
self.future_incompatible(lint).is_some(), in which case this is a future-incompatibility warning. In that case, after applying the lint-cap, we should set the level tomax(level, warn)-- in other words, ensure the level is at least warn, regardless of what the lint-cap says.We can also add some test cases for this using the testing framework. The simplest test would be something that takes some future-incompatible lint and sets it to
#[allow]-- we should still get a warning. We can also have a test that passes-A cap-lints=allow, which should yield the same result. I can help you more w/ writing said tests if you need.(One twist: we may want to add a "dummy" future-incompatible lint just for testing purposes...)
nikomatsakis commentedon Aug 3, 2016
That said, @wycats has been arguing to me that this would be a bad thing to do. It may be worth having a wider discussion if there are other approaches we might consider.
nikomatsakis commentedon Aug 3, 2016
I think, if I may take a stab at summarizing @wycats's point-of-view, that his position is:
cargo yanksoon-to-be-broken versions and give a reason, for example citing the upcoming changejacwah commentedon Jan 16, 2017
@hgallagher1993 Do you still want to fix this issue? Otherwise I'd be willing to handle it.
@nikomatsakis Considering @wycats' comments, do you think the implementation you outlined above still should be followed?
Some thoughts:
Maybe the message could link to the crate's homepage/repository and invite the user to contact the maintainer?
This is very true. Say there's a very stable application working well and doing it's thing. There's no reason to change it or upgrade Rust version. This is a case where silencing future incompatible warnings may be desirable.
nikomatsakis commentedon Jan 17, 2017
@jacwah I'm honestly not sure what to do here. What we're doing now doesn't seem right, but just enabling the warnings also doesn't seem ideal. I think perhaps what is needed is better integration with cargo and maybe even crates.io -- i.e., if we (the compiler team) could leave annotations on abandoned crates, and those could be summarized to people who depend on them, that might be ideal.
cc @wycats @alexcrichton
22 remaining items
pnkfelix commentedon Mar 28, 2019
(removing nomination label since @Centril self-assigned. I'm just going to assert "yes, this is important" until someone convinces me otherwise.)
pnkfelix commentedon May 2, 2019
triage: re-prioritizing to P-medium; this is being addressed, and while the task is high priority, it does not need to be given the same level of urgency that I would like to associate with P-high.
Centril commentedon Sep 29, 2019
Reassigning to @pnkfelix since they are doing the work on this right now.
pnkfelix commentedon Jan 9, 2020
The aforementioned work was invested in drafting rust-lang/rfcs#2834
pnkfelix commentedon Sep 2, 2022
I think the problem raised here is now addressed by rust-lang/rfcs#2834 and #71249, right?
est31 commentedon Sep 2, 2022
@pnkfelix I agree this should be closed in favour of the RFC and its tracking issue.
RalfJung commentedon Sep 2, 2022
Is there something tracking the process of turning all existing future-incompatibility warnings into warnings that show up in the FCW report?
est31 commentedon Sep 2, 2022
@RalfJung outside of the C-future-compatibility label I don't know of any better tracking. As for whether each forward compatibility warning should be added to cargo, I'm not sure. I think that if the number of affected users is small it can also be skipped.
pnkfelix commentedon Nov 17, 2022
Closing in favor of rust-lang/rfcs#2834 and #71249,