Skip to content

Merge ForceWarning/Allow/Expect into Warning - #163290

Merged
rust-bors[bot] merged 3 commits into
rust-lang:mainfrom
nnethercote:merge-warning-levels
Sep 26, 2026
Merged

Merge ForceWarning/Allow/Expect into Warning#163290
rust-bors[bot] merged 3 commits into
rust-lang:mainfrom
nnethercote:merge-warning-levels

Conversation

@nnethercote

@nnethercote nnethercote commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

Because they're all just special cases of warnings. Details in individual commits.

r? @oli-obk

@nnethercote
Some emit_diagnostic improvements
e01dc94 
There is one match arm handling some `ForceWarning` cases and another
arm handling the remaining `ForceWarning` case and also `Expect`. They
can be rearranged into one arm handling `ForceWarning` and one arm
handling `Expect`.

Also fix some comments:
- `ForceWarning` no longer has a field.
- Clarify the `LintExpectationId` location.
- The one about deduplication was inverted.
@nnethercote
Remove an out-of-date FIXME comment
4fc112e 
I think the ship has sailed on the "every error should have an error
code" idea and it's not worth pretending otherwise.
@rustbot

rustbot commented Sep 24, 2026

Copy link
Copy Markdown
Collaborator

miri is developed in its own repository. If the Miri part of this change can be broken out, consider making this change to rust-lang/miri instead. However, if Miri needs adjusting for rustc changes, just ignore this message.

cc @rust-lang/miri

rustc_errors::annotate_snippet_emitter_writer was changed

cc @Muscraft

rustfmt is developed in its own repository. If possible, consider making this change to rust-lang/rustfmt instead.

cc @rust-lang/rustfmt

@rustbot rustbot added the A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. label Sep 24, 2026
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustfmt Relevant to the rustfmt team, which will review and decide on the PR/issue. labels Sep 24, 2026
@nnethercote

nnethercote commented Sep 24, 2026

Copy link
Copy Markdown
Contributor Author

@oli-obk: this undoes the first commit from #138610, which removed fields from Level::ForceWarning and Level::Expect. The commit message didn't explain why those fields were removed, though presumably it was related to the PR's overall goal of impl !PartialOrd for HirId.

I've always found the lint/expectation stuff confusing and I feel like this new structure finally lets things be clear in my head. See what you think.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
oli-obk
oli-obk requested changes Sep 25, 2026
View reviewed changes

@oli-obk oli-obk left a comment •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

though presumably it was related to the PR's overall goal of impl !PartialOrd for HirId.

yes, initially lint level was orderable, but it isn't anymore, so you can add back such information.

this PR makes things indeed nicer, r=me with a nit

View changes since this review

Comment thread compiler/rustc_codegen_ssa/src/back/write.rs Outdated
}
Ok(SharedEmitterMessage::InlineAsmError(inner)) => {
assert_matches!(inner.level, Level::Error | Level::Warning | Level::Note);
assert_matches!(inner.level, Level::Error | Level::Warning(_) | Level::Note);

@oli-obk oli-obk Sep 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to preserve the previous assertion this should be Level::Warning(None), right?

@nnethercote nnethercote Sep 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I checked all the other occurrences of Warning(_), I think all the others are ok.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 25, 2026
@rustbot

rustbot commented Sep 25, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@nnethercote
Merge ForceWarning/Allow/Expect into Warning
fcf74ea 
Because they're all just special cases of warnings. This requires
introducing a new `Option<EmissionOverride>` field to `Warning` that
describes the special case behaviour; the `DiagInner::lint_id` field
also gets merged in.

Specific nice things about this:

- Removes some unreachable match arms for `Allow`/`Expect`.

- Removes the hacky upgrading of `Allow`/`Expect` to `Warning` in
  `emit_future_breakage_report`.

- The types now have structure that used to be maintained by comments
  and assertions. E.g. it's now impossible to not have a `lint_id` for
  an `expect` lint. (I always found the `DiagInner::lint_id` field
  confusing; it's clearer now.)

- There's a nice comment on `EmissionOverride` summarizing all the
  different cases.

- A little less code overall.
@nnethercote

nnethercote commented Sep 25, 2026

Copy link
Copy Markdown
Contributor Author

I fixed the nit.

@bors r=oli-obk

@rust-bors

rust-bors Bot commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

📌 Commit fcf74ea has been tentatively approved by oli-obk

It will be put into the queue for this repository once PR CI succeeds.

@rust-bors rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 25, 2026
@jhpratt jhpratt mentioned this pull request Sep 26, 2026
Merged
rust-bors Bot pushed a commit that referenced this pull request Sep 26, 2026
@bors
Auto merge of #163363 - jhpratt:rollup-zVCvTBh, r=jhpratt
fd98695 
Rollup of 13 pull requests

Successful merges:

 - #160695 (Detect bad number of generics caused by bad derive)
 - #162883 (No longer auto-inline `f16` and `f128` on well-supported platforms)
 - #163133 ([rustdoc] Fix invalid jump to def link when `#[rustc_allow_incoherent_impl]` is involved)
 - #163215 (Fix suggestion for Option to bool with proper precedence handling)
 - #163266 (More deferred liveness cleanups)
 - #163274 (Support -Z merge-functions with gcc and add stack-protector asm tests)
 - #163290 (Merge `ForceWarning`/`Allow`/`Expect` into `Warning`)
 - #163298 (Don't emit sugg for pub-reexport extern std)
 - #163312 (Add rustdoc regression test for glob import of a crate that re-exports)
 - #163357 (Borrowck cleanups part trois)
 - #161710 (Stabilize `mem::conjure_zst`)
 - #163332 (Add some docs to `Global`)
 - #163362 (simplify ndk compiler test)
@rust-bors
rust-bors Bot merged commit 7756b3e into rust-lang:main Sep 26, 2026
13 checks passed
@rustbot rustbot added this to the 1.100.0 milestone Sep 26, 2026
rust-bors Bot pushed a commit that referenced this pull request Sep 26, 2026
@bors
Unrolled build for #163290 in rollup 163363
da89441 
Rollup merge of #163290 - nnethercote:merge-warning-levels, r=oli-obk

Merge `ForceWarning`/`Allow`/`Expect` into `Warning`

Because they're all just special cases of warnings. Details in individual commits.

r? @oli-obk
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oli-obk oli-obk oli-obk requested changes

Assignees

@oli-obk oli-obk

Labels

A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustfmt Relevant to the rustfmt team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.100.0

Development

Successfully merging this pull request may close these issues.

4 participants

@nnethercote @rustbot @rust-log-analyzer @oli-obk