Support overriding warnings level for a specific lint via command line

[jieyouxu]

Supports the use case to

silence all but one particular rustc lint.
rustc -Awarnings "allows" all the warnings, easy so we only need to re-warn a single lint:
rustc -Awarnings -Wdead-code.

This PR allows the user to refine the more general warnings level specified on the command line with a more specific level for a particular lint.

Closes #105104.

[rustbot]

r? @oli-obk

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

[oli-obk]

@rfcbot merge

This seems like the right interpretation of what the user was asking for if they wrote -Awarnings -Wsome_lint. This does not affect any #[warn(some_other_lint)] attributes in the crate itself.

The only concern I see is that it's different from #[allow(warnings)], as that keeps silencing lints enabled by nested #[warn(lint)] attributes.

[rfcbot]

Team member @oli-obk has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Aaron1011
• @cjgillot
• @compiler-errors
• @davidtwco
• @eddyb
• @estebank
• @jackh726
• @lcnr
• @matthewjasper
• @michaelwoerister
• @nagisa
• @oli-obk
• @petrochenkov
• @pnkfelix
• @wesleywiser

Concerns:

• nesting (Support overriding warnings level for a specific lint via command line #113307 (comment))

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[petrochenkov]

I don't understand how this work.
In this case in-source annotations take priority over command line option, but above (#[warn(non_snake_case)]) command line takes priority over in-source annotation.

Also, what -A warnings means exactly

• allow all lints (not true according to this test)
• allow lints with default level warn
• allow lints with effective level warn (explicit level with fallback to default)

?

[jieyouxu]

In this case in-source annotations take priority over command line option, but above (#[warn(non_snake_case)]) command line takes priority over in-source annotation.

The warnings lint and -A warnings is... special. To the best of my knowedge, -A warnings suppresses warn level lints even when there is a source code annotation for the specific non-warnings lint (of warn level). However, when there is a #[level(warnings)] source code annotation, it overrides -A warnings for the lexical scope affected by the annotation.

[jieyouxu]

Also, lints with levels higher than warn are not affected by -A warnings or #[allow(warnings)] at all, such as deny-level lints.

[cjgillot]

I have 2 reservations.

1. If we are accepting this PR, I'd recommend taking the opportunity to have something fully consistent with how lints and lint groups are handled right now:

• command line arguments are order sensitive, so -Awarning -Wlint is different from -Wlint -Awarnings
• attributes nest: #[allow(warnings)] should mark all lints that have level Warn at that point and mark them as Allow.

@rfcbot concern nesting

2. I'm not really fond of the implementation. Would it be possible to move this special case in rustc_lint::levels? For instance, when the warnings pseudo-group is encountered, walk all the currently-warning lints to change their level?

[cjgillot]

Those are level and src arguments to the current function. We shouldn't recompute them.

[cjgillot]

Do we care about the order in which the command line arguments are passed?
What about -Wdead-code -Awarnings?

[jieyouxu]

2. I'm not really fond of the implementation. Would it be possible to move this special case in rustc_lint::levels? For instance, when the warnings pseudo-group is encountered, walk all the currently-warning lints to change their level?

I'm not really fond of the implementation myself either. I will investigate how to make this less hacky. I don't really like the "filter based on warnings at last minute" existing approach either. I think I want to construct the correct lint levels to begin with and respect command line / source code #[L(warnings)] attributes.

[jieyouxu]

I am actually very confused myself, if we're at the node corresponding to #[warn(some_other_lint)], would you expect specific_lint to fire? How about some_other_lint? Do we have any clear rules on how to handle cases like this?

@rustbot label: +I-lang-nominated

[cjgillot]

The first question is: what does "warnings" denote?

1. Is it the group of all lints that are warn by default? In that case, it behaves like any other lint group, and LintStore::check_lint_name will return the (large) list of such lints.
2. It it the group of all lints that are warn at the point of the attribute? In that case, implementation is more difficult.

In your example, supposing both lints are warn by default, the lint level stack looks like this:

• -A warnings: both are allow;
• -W specific_lint: specific lint is warn, the other is allow;
• allow(warnings): specific lint passes from warn to allow, so both are allow again;
• warn(specific_lint): specific lint is warn, the other is allow;
• warn(some_other_lint): specific lint is warn, the other is warn too.

If the lints are not warn-by-default, the behaviour of both interpretations diverge.

[jieyouxu]

Right, although the current behavior for something like

fn call() { println!("Hello World"); }

#[allow(warnings)]
#[warn(non_snake_case)]
fn main() {
    let _OwO = 0u8;

    unreachable!();
    #[warn(unreachable_code)]
    call();
}

is that all the nested specific lint #[warn]s are suppressed by the outer #[allow(warnings)]. This goes for -A warnings as well, where all non-#[warn(warnings)] specific lint warn attributes are suppressed.

[nagisa]

@rfcbot fcp reviewed

(I did not review the code, but the high-level interpretation of the flag combination makes a ton of sense to me)

[petrochenkov]

@rfcbot concern nesting
(Wasn't properly registered in #113307 (review))

[nikomatsakis]

We discussed this in the @rust-lang/lang meeting. @jieyouxu unfortunately we do not seem to have a very clear place where we have documented our intentions, although I recall us having many discussions on the topic over the years.

My recollection of related decisions is as follows:

• "Warnings" means "the set of things that are presently warn" -- this is very surprising to me! I expected it to be the "Set of lints that are warn-by-default". But I recall that this ruled out some use cases (I think @cramertj in particular had some opinions here). Moreover, this is what the Rust reference says, although I think that text could be more clear that the value of this group changes depending on where it appears in the tree (cc @rust-lang/lang-docs).
• We have decided that, within a nesting level, the things ordered later take priority (I think @Mark-Simulacrum drove this?)
• I don't 100% recall how command-line parameters fit into things that appear in the source text; my intuition is that they would be a root level, similar to how you're diagram depicted it, but I seem to recall some people feeling that they should override what appears in the source-text, or at least what appears at the root level in the source text.

I would really like to see this written into the reference, the current section doesn't seem very clear.

Two conclusions from the above:

1. This area is underspecified. An RFC that collects and documents for all the things, probably including Lint Reasons RFC rfcs#2383 and the "force-warn" command line flag (which to me was always a distinct thing, but I know it got rationalized into a lint level somehow), would be really valuable. Ideally it would justify the decisions with clear use cases, similar to the work that @xFrednet did for #[expect] recently.
2. I'm confused as to the actual question we are being asked! In particular, the question here asks "if we're at the node corresponding to #[warn(some_other_lint)], would you expect specific_lint to fire". But if I go up the tree, the innermost thing I see there is #[warn(specific_lint)], so it seems that the definition of warnings and the behavior of the command line is not really relevant here, this will always be a warn. Maybe you can rephrase the question to highlight why you would think it might be warning and why it might be something else? I guess maybe this has to do with whether the command line overrides the things that appear in the source file? I realize I'm again assuming that the command line forms the root of the tree, which may not be the case.

[nikomatsakis]

@jieyouxu

In case it got buried in the text, can you clarify this point...

2. I'm confused as to the actual question we are being asked! In particular, the question here asks "if we're at the node corresponding to #[warn(some_other_lint)], would you expect specific_lint to fire". But if I go up the tree, the innermost thing I see there is #[warn(specific_lint)], so it seems that the definition of warnings and the behavior of the command line is not really relevant here, this will always be a warn. Maybe you can rephrase the question to highlight why you would think it might be warning and why it might be something else? I guess maybe this has to do with whether the command line overrides the things that appear in the source file? I realize I'm again assuming that the command line forms the root of the tree, which may not be the case.

[jieyouxu]

@jieyouxu

In case it got buried in the text, can you clarify this point...

2. I'm confused as to the actual question we are being asked! In particular, the question here asks "if we're at the node corresponding to #[warn(some_other_lint)], would you expect specific_lint to fire". But if I go up the tree, the innermost thing I see there is #[warn(specific_lint)], so it seems that the definition of warnings and the behavior of the command line is not really relevant here, this will always be a warn. Maybe you can rephrase the question to highlight why you would think it might be warning and why it might be something else? I guess maybe this has to do with whether the command line overrides the things that appear in the source file? I realize I'm again assuming that the command line forms the root of the tree, which may not be the case.

Sorry for the late reply. I believe I wanted to ask that if the command line indeed forms the root of the tree, or if it actually overrides the source annotations.

[tmandry]

Nesting

I think the command line (specifically -A, -W, -D flags) should form the root of the tree. We have --cap-lints, --force-warn, and -F (forbid) for overriding the source. (Actually the mental model documented in the rustc book is that force-warn and forbid still form the root of the tree, but cannot be overridden; I think the distinction is mostly academic.)

That's almost all the expressive power one could want along this axis. One wrinkle is that --forbid is overridden by --cap-lints, while --force-warn is not. If we wanted full fine-grained control we could always add --force-allow and --force-deny.

warnings

Regarding the meaning of warnings, it is a simpler mental model for this to mean "the set of things that are warn-by-default". But this ignores what I perceive to be a common (and valid) use case, which is to disallow all warnings in a codebase: In other words, prevent code from being checked in that causes warnings to be printed to a user's screen. Of course, for this to be practical one must control the version of rustc being used to build a codebase, but that is common in monorepo setups.

Conclusion

Given that there is an existing use case that relies on documented behavior, I think we should continue to treat warnings as a "redirect" for all warnings that come out of a particular level of the tree. Interpreting -Awarnings -Wfoo in the way proposed by this PR would muddy the (already complicated) mental model and add inconsistency between CLI and the command line, as noted by @oli-obk.

A different group, like default-warnings, could be used to mean "the set of things that are warn-by-default". The compiler could further warn users that specify -Awarnings -Wfoo on the command line to use -Adefault-warnings -Wfoo instead.

[jieyouxu]

I strongly agree with the suggestion to keep the current behavior because users are relying on it. I will close this PR now.