Add a FCW for special cased blocks

compiler/rustc_ast_lowering/src/block.rs

@@ -30,6 +30,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
     ) -> (&'hir [hir::Stmt<'hir>], Option<&'hir hir::Expr<'hir>>) {
         let mut stmts = SmallVec::<[hir::Stmt<'hir>; 8]>::new();
         let mut expr = None;
+        tracing::debug!(?ast_stmts);
         while let [s, tail @ ..] = ast_stmts {
             match &s.kind {
                 StmtKind::Let(local) => {

compiler/rustc_lint/messages.ftl

@@ -386,6 +386,13 @@ lint_mixed_script_confusables =
 
 lint_multiple_supertrait_upcastable = `{$ident}` is object-safe and has multiple supertraits
 
+lint_never_block_without_tail_expr = diverging block without tail expression which has non-unit type
+    .note = if the last statement is `return`, `break`, or `continue`, it will still compile in Rust 2024
+
+lint_never_block_without_tail_expr_help_generic = the tail expression in this block must have an appropriate type, this can be achieved by moving the diverging expression to the end
+
+lint_never_block_without_tail_expr_help_remove_semi = remove semicolon here
+
 lint_node_source = `forbid` level set here
     .note = {$reason}
 

compiler/rustc_lint/src/lib.rs

@@ -65,6 +65,7 @@ mod lints;
 mod map_unit_fn;
 mod methods;
 mod multiple_supertrait_upcastable;
+mod never_block_without_tail_expr;
 mod non_ascii_idents;
 mod non_fmt_panic;
 mod non_local_def;
@@ -102,6 +103,7 @@ use let_underscore::*;
 use map_unit_fn::*;
 use methods::*;
 use multiple_supertrait_upcastable::*;
+use never_block_without_tail_expr::NeverBlockWithoutTailExpr;
 use non_ascii_idents::*;
 use non_fmt_panic::NonPanicFmt;
 use non_local_def::*;
@@ -180,6 +182,7 @@ late_lint_methods!(
     [
         BuiltinCombinedModuleLateLintPass,
         [
+            NeverBlockWithoutTailExpr: NeverBlockWithoutTailExpr,
             ForLoopsOverFallibles: ForLoopsOverFallibles,
             DerefIntoDynSupertrait: DerefIntoDynSupertrait,
             DropForgetUseless: DropForgetUseless,

compiler/rustc_lint/src/never_block_without_tail_expr.rs

@@ -0,0 +1,133 @@
+use crate::{LateContext, LateLintPass, LintContext};
+use rustc_session::lint::FutureIncompatibilityReason;
+
+use rustc_hir::{Block, Expr, ExprKind, StmtKind};
+use rustc_macros::{LintDiagnostic, Subdiagnostic};
+use rustc_session::{declare_lint, declare_lint_pass};
+use rustc_span::{edition::Edition, sym, Span, SyntaxContext};
+use tracing::debug;
+
+declare_lint! {
+    /// The `never_block_without_tail_expr` lint checks for blocks which have type `!`, but do not
+    /// end in an expression or a `return`, `break`, or `continue` statement
+    ///
+    /// ### Example
+    ///
+    /// ```rust
+    /// let _: u8 = {
+    ///     return 1;
+    /// };
+    /// ```
+    ///
+    /// {{produces}}
+    ///
+    /// ### Explanation
+    ///
+    /// Previously, blocks which diverged in all branches (e.g. by having a `return`) had type `!`
+    /// (which can be coerced to any type). This is being changed in the 2024 edition.
+    pub NEVER_BLOCK_WITHOUT_TAIL_EXPR,
+    Warn,
+    "`Iterator::map` call that discard the iterator's values",
+    @future_incompatible = FutureIncompatibleInfo {
+        reason: FutureIncompatibilityReason::EditionError(Edition::Edition2024),
+        reference: "https://github.com/rust-lang/rust/issues/123747",
+    };
+}
+
+declare_lint_pass!(NeverBlockWithoutTailExpr => [NEVER_BLOCK_WITHOUT_TAIL_EXPR]);
+
+impl<'tcx> LateLintPass<'tcx> for NeverBlockWithoutTailExpr {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'_>) {
+        // A block without tail expression
+        let ExprKind::Block(Block { stmts, expr: None, .. }, ..) = expr.kind else { return };
+
+        // Last statement of which is not `return`, `break`, `continue`, or `become`
+        if let [.., last] = stmts
+            && let StmtKind::Semi(e) = last.kind
+            && matches!(e.kind, ExprKind::Ret(_) | ExprKind::Break(_, _) | ExprKind::Continue(_))
+        {
+            return;
+        }
+
+        // Which has type `!`, which is coerced to non-unit
+        let typeck = cx.typeck_results();
+        debug!(block_ty = ?typeck.expr_ty(expr), block_ty_adjusted = ?typeck.expr_ty_adjusted(expr));
+        if !typeck.expr_ty(expr).is_never() || typeck.expr_ty_adjusted(expr).is_unit() {
+            return;
+        }
+
+        // If last statement has type `!` suggest turning it into a tail expression,
+        // otherwise provide a generic help message suggesting possible solutions.
+        let help = if let [.., last] = stmts
+            && let StmtKind::Semi(e) = last.kind
+            // Due to how never type fallback works, the panic macro does not have type `!`,
+            // fallback happens before this lint (which on edition < 2024 sets the type to `()`),
+            // and the adjustment is on some inner level inside macro.
+            //
+            // Special casing std/core panic-like macros is not nice, but I do not see a better solution.
+            && (typeck.expr_ty(e).is_never() || is_panic_macro_expansion(cx, e))
+            // Climb out of macros, so that we get span of the call,
+            // instead of the span of the macro internals
+            && let e_span =
+                e.span.find_oldest_ancestor_in_same_ctxt().with_ctxt(SyntaxContext::root())
+            // Get the span between the expression and the end of the statement,
+            // i.e. the span of the semicolon
+            && let span = e_span.between(last.span.shrink_to_hi())
+            // TODO: remove this workaround for macros misbehaving
+            //       (`;` is not included in the span of the statement after macro expansion,
+            //        so the span ends up being empty, so debug assertions in diagnostic code fail)
+            && !span.is_empty()
+        {
+            Help::RemoveSemi { span }
+        } else {
+            Help::Generic
+        };
+
+        cx.emit_span_lint(
+            NEVER_BLOCK_WITHOUT_TAIL_EXPR,
+            expr.span,
+            NeverBlockWithoutTailExprLint { help },
+        );
+    }
+}
+
+fn is_panic_macro_expansion(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
+    let Some(id) = expr.span.ctxt().outer_expn_data().macro_def_id else { return false };
+    let Some(name) = cx.tcx.get_diagnostic_name(id) else { return false };
+
+    matches!(
+        name,
+        sym::core_panic_macro
+            | sym::core_panic_2015_macro
+            | sym::core_panic_2021_macro
+            | sym::std_panic_macro
+            | sym::std_panic_2015_macro
+            | sym::unreachable_macro
+            | sym::todo_macro
+    )
+}
+
+#[derive(LintDiagnostic)]
+#[diag(lint_never_block_without_tail_expr)]
+#[note]
+struct NeverBlockWithoutTailExprLint {
+    #[subdiagnostic]
+    help: Help,
+}
+
+#[derive(Subdiagnostic)]
+enum Help {
+    #[suggestion(
+        lint_never_block_without_tail_expr_help_remove_semi,
+        style = "short",
+        code = "",
+        applicability = "machine-applicable"
+    )]
+    RemoveSemi {
+        #[primary_span]
+        span: Span,
+    },
+
+    #[help(lint_never_block_without_tail_expr_help_generic)]
+    Generic,
+}

compiler/rustc_parse/src/parser/stmt.rs

@@ -820,6 +820,8 @@ impl<'a> Parser<'a> {
         }
 
         stmt.span = stmt.span.to(self.prev_token.span);
+        trace!(?stmt.span, ?self.prev_token.span, ?add_semi_to_stmt, ?eat_semi, ?stmt);
+        debug_assert!(stmt.span.contains(self.prev_token.span));
         Ok(Some(stmt))
     }
 

tests/ui/lint/never_block_without_tail_expr.auto_fix.fixed

@@ -0,0 +1,80 @@
+//@ revisions: auto_fix manual_fix
+//@[auto_fix] run-rustfix
+#![feature(never_type)]
+#![allow(unreachable_code, unused)]
+#![deny(never_block_without_tail_expr)]
+
+fn main() {}
+
+#[cfg(manual_fix)]
+fn a() {
+    let _: ! = {
+        //[manual_fix]~^ error: diverging block without tail expression which has non-unit type
+        //[manual_fix]~| warn: this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2024!
+        return;
+        ();
+    };
+
+    let _: ! = {
+        //[manual_fix]~^ error: diverging block without tail expression which has non-unit type
+        //[manual_fix]~| warn: this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2024!
+        return;
+
+        fn helper() {}
+    };
+}
+
+fn b() {
+    let _: u8 = {
+        //~^ error: diverging block without tail expression which has non-unit type
+        //~| warn: this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2024!
+        abort()
+    };
+}
+
+// TODO: this suggestion does not work, because macro expansion looses the span of `;`
+//       see <https://rust-lang.zulipchat.com/#narrow/stream/182449-t-compiler.2Fhelp/topic/How.20to.20get.20span.20of.20the.20macro.20call.2C.20not.20internals.3F/near/441575274>
+// fn c() {
+//     let _: u8 = {
+//         // ~^ error: diverging block without tail expression which has non-unit type
+//         // ~| warn: this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2024!
+//         panic!();
+//     };
+// }
+
+fn d() {
+    #[rustfmt::skip]
+    let _: u8 = {
+        //~^ error: diverging block without tail expression which has non-unit type
+        //~| warn: this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2024!
+        loop {}
+    };
+}
+
+#[rustfmt::skip]
+fn e() -> ! {
+    //~^ error: diverging block without tail expression which has non-unit type
+    //~| warn: this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2024!
+    loop {}
+}
+
+// `return;` & company are currently allowed
+fn okay() {
+    let _: u8 = {
+        return;
+    };
+
+    loop {
+        let _: u8 = {
+            break;
+        };
+
+        let _: u8 = {
+            continue;
+        };
+    }
+}
+
+fn abort() -> ! {
+    loop {}
+}

tests/ui/lint/never_block_without_tail_expr.auto_fix.stderr

@@ -0,0 +1,55 @@
+error: diverging block without tail expression which has non-unit type
+  --> $DIR/never_block_without_tail_expr.rs:28:17
+   |
+LL |       let _: u8 = {
+   |  _________________^
+LL | |
+LL | |
+LL | |         abort();
+   | |                - help: remove semicolon here
+LL | |     };
+   | |_____^
+   |
+   = warning: this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2024!
+   = note: for more information, see https://github.com/rust-lang/rust/issues/123747
+   = note: if the last statement is `return`, `break`, or `continue`, it will still compile in Rust 2024
+note: the lint level is defined here
+  --> $DIR/never_block_without_tail_expr.rs:5:9
+   |
+LL | #![deny(never_block_without_tail_expr)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: diverging block without tail expression which has non-unit type
+  --> $DIR/never_block_without_tail_expr.rs:47:17
+   |
+LL |       let _: u8 = {
+   |  _________________^
+LL | |
+LL | |
+LL | |         loop {};
+   | |                - help: remove semicolon here
+LL | |     };
+   | |_____^
+   |
+   = warning: this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2024!
+   = note: for more information, see https://github.com/rust-lang/rust/issues/123747
+   = note: if the last statement is `return`, `break`, or `continue`, it will still compile in Rust 2024
+
+error: diverging block without tail expression which has non-unit type
+  --> $DIR/never_block_without_tail_expr.rs:55:13
+   |
+LL |   fn e() -> ! {
+   |  _____________^
+LL | |
+LL | |
+LL | |     loop {};
+   | |            - help: remove semicolon here
+LL | | }
+   | |_^
+   |
+   = warning: this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2024!
+   = note: for more information, see https://github.com/rust-lang/rust/issues/123747
+   = note: if the last statement is `return`, `break`, or `continue`, it will still compile in Rust 2024
+
+error: aborting due to 3 previous errors
+