From 19b7192c7266ddd6d3b6fb3b436458fb84682f9c Mon Sep 17 00:00:00 2001
From: Nicholas Nethercote <n.nethercote@gmail.com>
Date: Thu, 20 Jun 2024 10:08:06 +1000
Subject: [PATCH] Fix assertion failure for some `Expect` diagnostics.

In #120699 I moved some code dealing with `has_future_breakage` earlier
in `emit_diagnostic`. Issue #126521 identified a case where that
reordering was invalid (leading to an assertion failure) for some `Expect`
diagnostics.

This commit partially undoes the change, by moving the handling of
unstable `Expect` diagnostics earlier again. This makes
`emit_diagnostic` a bit uglier, but is necessary to fix the problem.

Fixes #126521.
---
 compiler/rustc_errors/src/lib.rs              | 28 +++++++++++++------
 ...ect-future_breakage-crash-issue-126521.rs} |  2 +-
 ...-future_breakage-crash-issue-126521.stderr | 13 +++++++++
 3 files changed, 33 insertions(+), 10 deletions(-)
 rename tests/{crashes/126521.rs => ui/lint/expect-future_breakage-crash-issue-126521.rs} (69%)
 create mode 100644 tests/ui/lint/expect-future_breakage-crash-issue-126521.stderr

diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs
index 83d5bbff0b0a4..620f56c01e899 100644
--- a/compiler/rustc_errors/src/lib.rs
+++ b/compiler/rustc_errors/src/lib.rs
@@ -1437,6 +1437,24 @@ impl DiagCtxtInner {
 
     // Return value is only `Some` if the level is `Error` or `DelayedBug`.
     fn emit_diagnostic(&mut self, mut diagnostic: DiagInner) -> Option<ErrorGuaranteed> {
+        match diagnostic.level {
+            Expect(expect_id) | ForceWarning(Some(expect_id)) => {
+                // The `LintExpectationId` can be stable or unstable depending on when it was
+                // created. Diagnostics created before the definition of `HirId`s are unstable and
+                // can not yet be stored. Instead, they are buffered until the `LintExpectationId`
+                // is replaced by a stable one by the `LintLevelsBuilder`.
+                if let LintExpectationId::Unstable { .. } = expect_id {
+                    // We don't call TRACK_DIAGNOSTIC because we wait for the
+                    // unstable ID to be updated, whereupon the diagnostic will be
+                    // passed into this method again.
+                    self.unstable_expect_diagnostics.push(diagnostic);
+                    return None;
+                }
+                // Continue through to the `Expect`/`ForceWarning` case below.
+            }
+            _ => {}
+        }
+
         if diagnostic.has_future_breakage() {
             // Future breakages aren't emitted if they're `Level::Allow`,
             // but they still need to be constructed and stashed below,
@@ -1512,16 +1530,8 @@ impl DiagCtxtInner {
                 return None;
             }
             Expect(expect_id) | ForceWarning(Some(expect_id)) => {
-                // Diagnostics created before the definition of `HirId`s are
-                // unstable and can not yet be stored. Instead, they are
-                // buffered until the `LintExpectationId` is replaced by a
-                // stable one by the `LintLevelsBuilder`.
                 if let LintExpectationId::Unstable { .. } = expect_id {
-                    // We don't call TRACK_DIAGNOSTIC because we wait for the
-                    // unstable ID to be updated, whereupon the diagnostic will
-                    // be passed into this method again.
-                    self.unstable_expect_diagnostics.push(diagnostic);
-                    return None;
+                    unreachable!(); // this case was handled at the top of this function
                 }
                 self.fulfilled_expectations.insert(expect_id.normalize());
                 if let Expect(_) = diagnostic.level {
diff --git a/tests/crashes/126521.rs b/tests/ui/lint/expect-future_breakage-crash-issue-126521.rs
similarity index 69%
rename from tests/crashes/126521.rs
rename to tests/ui/lint/expect-future_breakage-crash-issue-126521.rs
index 0a025aabcf2ee..a3c8544613bf6 100644
--- a/tests/crashes/126521.rs
+++ b/tests/ui/lint/expect-future_breakage-crash-issue-126521.rs
@@ -1,4 +1,3 @@
-//@ known-bug: rust-lang/rust#126521
 macro_rules! foo {
     ($val:ident) => {
         true;
@@ -7,5 +6,6 @@ macro_rules! foo {
 
 fn main() {
     #[expect(semicolon_in_expressions_from_macros)]
+    //~^ ERROR the `#[expect]` attribute is an experimental feature
     let _ = foo!(x);
 }
diff --git a/tests/ui/lint/expect-future_breakage-crash-issue-126521.stderr b/tests/ui/lint/expect-future_breakage-crash-issue-126521.stderr
new file mode 100644
index 0000000000000..b24831b1ae43a
--- /dev/null
+++ b/tests/ui/lint/expect-future_breakage-crash-issue-126521.stderr
@@ -0,0 +1,13 @@
+error[E0658]: the `#[expect]` attribute is an experimental feature
+  --> $DIR/expect-future_breakage-crash-issue-126521.rs:8:5
+   |
+LL |     #[expect(semicolon_in_expressions_from_macros)]
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: see issue #54503 <https://github.com/rust-lang/rust/issues/54503> for more information
+   = help: add `#![feature(lint_reasons)]` to the crate attributes to enable
+   = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0658`.