-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Doc: clarify priority of lint level sources #142021
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
base: master
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
src/doc/rustc/src/lints/levels.md
Outdated
|
||
## Priority of Lint Level Sources | ||
|
||
Rust allows setting lint levels (`allow`, `warn`, `deny`, etc.) through various sources: directly in the code, via command-line flags, or through global compiler configuration. When these sources conflict, the compiler follows a well-defined priority order to determine which setting takes effect. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some general remarks (I don't have bandwidth to take over review for this ATM):
- I wonder if we should explicitly delineate between lint level control mechanisms which are compiler-only, versus those part of the language (such as in-source lint level attributes).
- I'm not so sure the priority order is well-defined. See Support overriding
warnings
level for a specific lint via command line #113307 (comment). - Note that the ordering of compiler flags can matter! Some flags are "commutative" (in terms of observable final behavior) with respect to each other IIRC, while some flags aren't.
- (Unstable, not suitable for stable rustc book) there's also the
--crate-attr
flag, which can somewhat blur the line between compiler vs language.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback! I took a closer look and I think you're right. If that's the case, it might be best to just document the ordering for in-source lint level attributes, where the behavior is straightforward: the closer attribute takes precedence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For what it's worth, the reason I made that distinction is because I feel like:
- Compiler flag mechanisms belong in the rustc book, but
- Language (in-source) attributes and the concept IMO belongs in the Reference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Looks like there is a CI error that needs to be resolved.
src/doc/rustc/src/lints/levels.md
Outdated
|
||
## Priority of Lint Level Sources | ||
|
||
Rust allows setting lint levels (`allow`, `warn`, `deny`, etc.) through various sources: directly in the code, via command-line flags, or through global compiler configuration. When these sources conflict, the compiler follows a well-defined priority order to determine which setting takes effect. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is a "global compiler configuration"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Poor wording on my side, I meant options in the .cargo file, which is the same as CLI options, so I removed this part.
src/doc/rustc/src/lints/levels.md
Outdated
|
||
Rust allows setting lint levels (`allow`, `warn`, `deny`, etc.) through various sources: directly in the code, via command-line flags, or through global compiler configuration. When these sources conflict, the compiler follows a well-defined priority order to determine which setting takes effect. | ||
|
||
This section explains how Rust resolves conflicts between **different sources of lint configuration**, rather than between the lint levels themselves. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wording nit:
This section explains how Rust resolves conflicts between **different sources of lint configuration**, rather than between the lint levels themselves. | |
This section explains how conflicts are resolved between **different sources of lint configuration**, rather than between the lint levels themselves. |
src/doc/rustc/src/lints/levels.md
Outdated
|
||
This section explains how Rust resolves conflicts between **different sources of lint configuration**, rather than between the lint levels themselves. | ||
|
||
### Priority of Sources (Highest to Lowest) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually use sentence case for section titles.
### Priority of Sources (Highest to Lowest) | |
### Priority of sources (highest to lowest) |
src/doc/rustc/src/lints/levels.md
Outdated
in-line attributes (#[...]) > | ||
crate-level attributes (#![...]) > |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the way this is presented is possibly confusing. The language doesn't differentiate between inner and outer attributes for lint levels. It just so happens that crate-level can only be specified by an inner attribute. But inner attributes can show up in many places. I would probably just say "attributes" here, since attributes just create an AST-hierarchy with the most-specific taking precedence.
src/doc/rustc/src/lints/levels.md
Outdated
command-line flags (-A, -W, -D) | ||
``` | ||
|
||
### Special Cases |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
### Special Cases | |
### Special case priorities |
src/doc/rustc/src/lints/levels.md
Outdated
|
||
### Special Cases | ||
|
||
The special levels `forbid(lint)` and `force_warn(lint)` take precedence over normal configurations **regardless of where they are set**. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of parentheses looks a little strange here to me, since force_warn(lint)
isn't a valid syntax anywhere. How about just something like this?
The special levels `forbid(lint)` and `force_warn(lint)` take precedence over normal configurations **regardless of where they are set**. | |
The special levels forbid and force-warn take precedence over normal configurations **regardless of where they are set**. |
I don't know what "normal configurations" are.
Also, this doesn't seem quite accurate to me. cap-lints
takes precedence over forbid.
And in a sense it does matter where forbid
is set, since it applies only to the AST-hierarchy below where it is set.
force-warn can only be set in one place (the CLI), though I'm not sure if this is trying to convey something about the order of CLI arguments?
This doesn't say how forbid and force-warn interact with one another.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a bit confusing for me. As far as I understood, for allow
, deny
, and warn
whoever has the most-specific scope takes precedence. However, it is the opposite case for forbid
and force-warn
.
Even the ordering in the cli options, as mentioned in the book, for -A
, -D
, and -W
which ever option comes last, takes place, but for --force-warn
and -F
which ever option comes first, takes place.
bb04e82
to
6e8d426
Compare
This comment has been minimized.
This comment has been minimized.
6e8d426
to
3cf81db
Compare
This comment has been minimized.
This comment has been minimized.
3cf81db
to
d6603df
Compare
This comment has been minimized.
This comment has been minimized.
d6603df
to
e097fd1
Compare
src/doc/rustc/src/lints/levels.md
Outdated
} | ||
``` | ||
|
||
### Priority of sources (`force-warn`and `forbid`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
### Priority of sources (`force-warn`and `forbid`) | |
### Priority of sources (`force-warn` and `forbid`) |
src/doc/rustc/src/lints/levels.md
Outdated
|
||
### Priority of sources (`force-warn`and `forbid`) | ||
|
||
Special lint levels take precedence over standard lint levels, and with eachother, **the least specific scope wins** — the further the attribute is to the code, the higher its precedence: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Special lint levels take precedence over standard lint levels, and with eachother, **the least specific scope wins** — the further the attribute is to the code, the higher its precedence: | |
Special lint levels take precedence over standard lint levels, and with each other, **the least specific scope wins** — the further the attribute is to the code, the higher its precedence: |
This section in general is feeling a little confusing to me.
Some specific comments:
- This introduces novel terms like "special lint level" and "standard lint level", but doesn't define what those are.
- force-warn doesn't really have a "scope" in the sense of the AST.
I'd like to dig in a little to where the confusion with the current documentation is coming from. And maybe @jieyouxu can chime in with their thoughts?
The existing documentation for force-warn
I think pretty clearly specifies that it forces a lint to be at warning level, and it cannot be overridden by anything, including cap-lints.
The existing documentation for forbid
specifies that it forces something to be an error, and cannot be overridden except by --cap-lints
.
(I think it would be great to move these examples up to those sections, though.)
As a mental model for "forbid", I wouldn't think about it as "least specific scope" wins. The actual behavior is that attempts to override forbid in more specific scopes is an error. Thus, there cannot be any different levels in more specific scopes; they are not allowed. Other than that restriction, forbid
is no different from deny
.
So, I'm curious where it feels like things aren't being specified?
I notice in #124088 that it mentions confusion over how CLI -A
-W
-D
relate to attributes. And it seems like it would be great to be more explicit about that in the via-compiler-flag section.
The issue also mentions CLI groups, but it seems like that is already covered?
The "warnings" group I would also agree can be confusing. However, it has strange behaviors that might actually be buggy, or at least not what people expect (see things like #118140, #75668, #91262). We do have a little documentation here, but I'm not sure how much more we can say before venturing into "this might be a bug" territory.
Can you clarify more about where the confusions with the existing docs might be, and what the goals are?
If I were to add a section summarizing priorities, I might write it something like this:
## Summary of priorities
In summary, the different lint controls and how the interact are:
1. [`--force-warn`](#force-warn) forces a lint to warning level, and takes precedence over attributes and all other CLI flag.
2. [`--cap-lints`](#capping-lints) sets the maximum level of a lint, and takes precedence over attributes and the `-A`, `-D`, `-W`, `-F` CLI flags.
3. [CLI level flags](#via-compiler-flag) take precedence over attributes.
- The order of the flags matter; flags on the right take precedence over earlier flags.
4. Within the source, [attributes](#via-an-attribute) at a lower-level in the syntax tree take precedence over attributes at a higher level, or from a previous attribute on the same entity as listed in left-to-right source order.
- The exception is once a lint is set to "forbid", it is an error to try to change its level except for `deny`, which is allowed inside a forbid context, but is ignored.
In terms of priority, [lint groups](groups.md) are treated as-if they are expanded to a list of all of the lints they contain. The exception is the `warnings` group which ignores attribute and CLI order and applies to all lints that would otherwise warn within the entity.
And I wouldn't add any other new sections, since I'd be reluctant to belabor the point. I do like adding the examples to the sections above, and the section on attributes should probably say something about the fourth point about ast-hierarchy precedence.
(Sorry, I have to stop right now due to time, but I think the above is accurate. Does that make sense to you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with the feedback here. Primarily, I think for this PR, we should restrict its scope to the more "well-specified" parts.
There are some interactions (like flag precedence and warnings
) which are underspecified, which I wanted to eventually make the interactions well-defined or clarified through test coverage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #142610 for what I had in mind on how to properly specify this, backed with test coverage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review. Sorry for the late response. I agree, and I have updated accordingly.
The only change I made was in the --cap-lints section, as it does not make sense for -A to be there:
2. [`--cap-lints`](#capping-lints) sets the maximum level of a lint, and takes precedence over attributes as well as the `-D`, `-W`, and `-F` CLI flags.
src/doc/rustc/src/lints/levels.md
Outdated
|
||
Special lint levels take precedence over standard lint levels, and with eachother, **the least specific scope wins** — the further the attribute is to the code, the higher its precedence: | ||
|
||
Special lint levels take precedence over standard levels. When multiple special levels are in effect, **the least specific (outermost) scope** takes precedence. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This paragraph seems to be partially repeating the previous one? I'm uncertain what's intended here.
src/doc/rustc/src/lints/levels.md
Outdated
|
||
General rule of thumb for allow deny and warn in the AST-hierarchy whoever has the most-specific scope takes precedence. | ||
|
||
General rule of thumb for force-warn forbid in the AST-hierarchy whoever has the least-specific scope takes precedence. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to the comment above, force-warn isn't really in the AST-hierarchy. Its an external mechanism that imposes its will upon all others. It is similar to --cap-lints
which is also an external mechnaism, but --force-warn
wins over --cap-lints
.
src/doc/rustc/src/lints/levels.md
Outdated
|
||
force-warn / forbid take precedence over deny, allow, and warn regardless. | ||
|
||
### Special case priorities |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This section seems like a duplicate of the previous one? I'm not quite clear why this is being repeated.
src/doc/rustc/src/lints/levels.md
Outdated
|
||
### Special case priorities | ||
|
||
- `forbid` takes precendece over other lint configurations in case of multiple, except for --cap-lint |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned above, it's not that it takes precedence. It's that it prevents overriding the lint level (with the exception that --cap-lints
and --force-warn
are able to override it).
src/doc/rustc/src/lints/levels.md
Outdated
pollute the output of your build. | ||
pollute the output of your build. However, note that `--cap-lints allow` does **not** override lints marked as `force-warn`. | ||
|
||
## Priority of Lint Level Sources |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Be sure to use sentence case.
## Priority of Lint Level Sources | |
## Priority of lint level sources |
(FYI: I'll only be able to get to this next Monday at the earliest.) |
src/doc/rustc/src/lints/levels.md
Outdated
Rust allows setting lint levels (`allow`, `warn`, `deny`, etc.) through various sources: | ||
|
||
- Attributes (`#[allow(...)]`, `#![deny(...)]`, etc.) | ||
- Command-line options (e.g., `--cap-lints`, `-A unused_variables`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: there is also --force-warn
src/doc/rustc/src/lints/levels.md
Outdated
- Attributes (`#[allow(...)]`, `#![deny(...)]`, etc.) | ||
- Command-line options (e.g., `--cap-lints`, `-A unused_variables`) | ||
|
||
When multiple lint levels apply to the same lint, the compiler resolves conflicts based on the **source’s scope and strength**. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: this wording is IMO a bit confusing, this is moreso about lint level precedence, there are no "conflicts", and I don't think "strength" is a thing.
src/doc/rustc/src/lints/levels.md
Outdated
|
||
### Priority of sources (`force-warn`and `forbid`) | ||
|
||
Special lint levels take precedence over standard lint levels, and with eachother, **the least specific scope wins** — the further the attribute is to the code, the higher its precedence: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with the feedback here. Primarily, I think for this PR, we should restrict its scope to the more "well-specified" parts.
There are some interactions (like flag precedence and warnings
) which are underspecified, which I wanted to eventually make the interactions well-defined or clarified through test coverage.
e097fd1
to
fc2c6ce
Compare
This comment has been minimized.
This comment has been minimized.
fe26632
to
cb6214f
Compare
This updates the rustc book to clearly document how conflicting lint configurations are resolved across different sources, including command-line flags, crate-level attributes, in-line attributes, and `--cap-lints`. It also explains the special behavior of `forbid` and `force_warn`.
cb6214f
to
2760b24
Compare
This updates the rustc book to clearly document how conflicting lint configurations are resolved across different sources, including command-line flags, crate-level attributes, in-line attributes, and
--cap-lints
.It also explains the special behavior of
forbid
andforce_warn
.Fixes #124088