rustc_lint: Enforce rustc::potential_query_instability lint
#119251
Conversation
|
r? @wesleywiser (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
S-waiting-on-review
compiler/rustc_lint/src/levels.rs
Outdated
| @@ -1028,6 +1028,8 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> { | |||
| } | |||
|
|
|||
| if !is_crate_node { | |||
| // We break on the first lint, and it does not matter which one. | |||
There was a problem hiding this comment.
It may not matter, but emitting a different diagnostic depending on the architecture is a bug.
There was a problem hiding this comment.
I changed current_specs() to return a &FxIndexMap, which only cascaded to 13 other places, which seems fine. So now the iteration order here should be deterministic.
(Due to conflicts with master I also had to rebase.)
… r=cjgillot rustc_lint: Prevent triplication of various lints Prevent triplication of various lints. The triplication happens because we run the same lint three times (or less in some cases): * In `BuiltinCombinedPreExpansionLintPass` * In `BuiltinCombinedEarlyLintPass` * In `shallow_lint_levels_on()` Only run the lints one time by checking the `lint_added_lints` bool. Set your GitHub diff setting to ignore whitespaces changes when reviewing this PR, since I had to enclose a block inside an if. Closes rust-lang#73301 (I found this while exploring the code related to [this](rust-lang#119251 (comment)) comment.)
Rollup merge of rust-lang#119388 - Enselic:prevent-lint-triplication, r=cjgillot rustc_lint: Prevent triplication of various lints Prevent triplication of various lints. The triplication happens because we run the same lint three times (or less in some cases): * In `BuiltinCombinedPreExpansionLintPass` * In `BuiltinCombinedEarlyLintPass` * In `shallow_lint_levels_on()` Only run the lints one time by checking the `lint_added_lints` bool. Set your GitHub diff setting to ignore whitespaces changes when reviewing this PR, since I had to enclose a block inside an if. Closes rust-lang#73301 (I found this while exploring the code related to [this](rust-lang#119251 (comment)) comment.)
|
☔ The latest upstream changes (presumably #119421) made this pull request unmergeable. Please resolve the merge conflicts. |
…exMap` So that lint iteration order becomes predicitable. Discovered with `rustc::potential_query_instability`.
Stop allowing `rustc::potential_query_instability` on all of `rustc_lint` and instead allow it on a case-by-case basis if it is safe to do so. In this particular crate, all lints were safe to allow.
Enselic
force-pushed
the
rustc_lint-query-stability
branch
from
bdc6fc4
to
295d600
Compare
|
r? @cjgillot |
|
@bors r+ |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (e51e98d): comparison URL. Overall result: ❌ regressions - ACTION NEEDEDNext Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 665.69s -> 666.95s (0.19%) |
|
The performance regression should be fixed by #119488. |
Mark-Simulacrum
added
the
perf-regression-triaged
Stop allowing
rustc::potential_query_instabilityon all ofrustc_lintand instead allow it on a case-by-case basis if it is safe to do so. In this particular crate, all lints were safe to allow.Part of #84447 which is E-help-wanted.