From 5620c82e5344b42e99c317a550bdd1be878c5267 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 10 Aug 2024 19:46:48 +0000 Subject: [PATCH] Report uninhabited call return types on MIR. --- compiler/rustc_mir_build/messages.ftl | 5 ++ .../rustc_mir_build/src/builder/expr/into.rs | 13 +--- compiler/rustc_mir_build/src/builder/mod.rs | 76 +++++++++++++++++++ compiler/rustc_mir_build/src/errors.rs | 12 +++ compiler/rustc_passes/messages.ftl | 5 -- compiler/rustc_passes/src/errors.rs | 12 --- compiler/rustc_passes/src/liveness.rs | 49 +----------- tests/ui/enum-discriminant/issue-46519.rs | 1 + tests/ui/enum-discriminant/issue-46519.stderr | 18 +++++ .../intrinsics/panic-uninitialized-zeroed.rs | 2 +- tests/ui/lint/dead-code/issue-85071-2.rs | 2 +- tests/ui/lint/dead-code/issue-85071-2.stderr | 39 +++++----- tests/ui/lint/dead-code/issue-85071.rs | 2 +- tests/ui/lint/dead-code/issue-85071.stderr | 39 +++++----- 14 files changed, 156 insertions(+), 119 deletions(-) create mode 100644 tests/ui/enum-discriminant/issue-46519.stderr diff --git a/compiler/rustc_mir_build/messages.ftl b/compiler/rustc_mir_build/messages.ftl index e84e42b8a44b..4a741169443d 100644 --- a/compiler/rustc_mir_build/messages.ftl +++ b/compiler/rustc_mir_build/messages.ftl @@ -372,6 +372,11 @@ mir_build_union_field_requires_unsafe_unsafe_op_in_unsafe_fn_allowed = mir_build_union_pattern = cannot use unions in constant patterns .label = can't use a `union` here +mir_build_unreachable_due_to_uninhabited = unreachable {$descr} + .label = unreachable {$descr} + .label_orig = any code following this expression is unreachable + .note = this expression has type `{$ty}`, which is uninhabited + mir_build_unreachable_making_this_unreachable = collectively making this unreachable mir_build_unreachable_making_this_unreachable_n_more = ...and {$covered_by_many_n_more_count} other patterns collectively make this unreachable diff --git a/compiler/rustc_mir_build/src/builder/expr/into.rs b/compiler/rustc_mir_build/src/builder/expr/into.rs index 7676b720e357..bb65ef28bdc8 100644 --- a/compiler/rustc_mir_build/src/builder/expr/into.rs +++ b/compiler/rustc_mir_build/src/builder/expr/into.rs @@ -390,18 +390,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { args, unwind: UnwindAction::Continue, destination, - // The presence or absence of a return edge affects control-flow sensitive - // MIR checks and ultimately whether code is accepted or not. We can only - // omit the return edge if a return type is visibly uninhabited to a module - // that makes the call. - target: expr - .ty - .is_inhabited_from( - this.tcx, - this.parent_module, - this.infcx.typing_env(this.param_env), - ) - .then_some(success), + target: Some(success), call_source: if from_hir_call { CallSource::Normal } else { diff --git a/compiler/rustc_mir_build/src/builder/mod.rs b/compiler/rustc_mir_build/src/builder/mod.rs index 7ca94e655fb2..c0b5d3bb8f8b 100644 --- a/compiler/rustc_mir_build/src/builder/mod.rs +++ b/compiler/rustc_mir_build/src/builder/mod.rs @@ -24,10 +24,12 @@ use rustc_middle::mir::*; use rustc_middle::thir::{self, ExprId, LintLevel, LocalVarId, Param, ParamId, PatKind, Thir}; use rustc_middle::ty::{self, ScalarInt, Ty, TyCtxt, TypeVisitableExt, TypingMode}; use rustc_middle::{bug, span_bug}; +use rustc_session::lint; use rustc_span::{Span, Symbol, sym}; use crate::builder::expr::as_place::PlaceBuilder; use crate::builder::scope::DropKind; +use crate::errors; pub(crate) fn closure_saved_names_of_captured_variables<'tcx>( tcx: TyCtxt<'tcx>, @@ -531,6 +533,7 @@ fn construct_fn<'tcx>( return_block.unit() }); + builder.lint_and_remove_uninhabited(); let mut body = builder.finish(); body.spread_arg = if abi == ExternAbi::RustCall { @@ -588,6 +591,7 @@ fn construct_const<'a, 'tcx>( builder.build_drop_trees(); + builder.lint_and_remove_uninhabited(); builder.finish() } @@ -806,6 +810,78 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { writer.write_mir_fn(&body, &mut std::io::stdout()).unwrap(); } + fn lint_and_remove_uninhabited(&mut self) { + let mut lints = vec![]; + + for bbdata in self.cfg.basic_blocks.iter_mut() { + let term = bbdata.terminator_mut(); + let TerminatorKind::Call { ref mut target, destination, .. } = term.kind else { + continue; + }; + let Some(target_bb) = *target else { continue }; + + let ty = destination.ty(&self.local_decls, self.tcx).ty; + let ty_is_inhabited = ty.is_inhabited_from( + self.tcx, + self.parent_module, + self.infcx.typing_env(self.param_env), + ); + + if !ty_is_inhabited { + // Unreachable code warnings are already emitted during type checking. + // However, during type checking, full type information is being + // calculated but not yet available, so the check for diverging + // expressions due to uninhabited result types is pretty crude and + // only checks whether ty.is_never(). Here, we have full type + // information available and can issue warnings for less obviously + // uninhabited types (e.g. empty enums). The check above is used so + // that we do not emit the same warning twice if the uninhabited type + // is indeed `!`. + if !ty.is_never() { + lints.push((target_bb, ty, term.source_info.span)); + } + + // The presence or absence of a return edge affects control-flow sensitive + // MIR checks and ultimately whether code is accepted or not. We can only + // omit the return edge if a return type is visibly uninhabited to a module + // that makes the call. + *target = None; + } + } + + for (target_bb, orig_ty, orig_span) in lints { + if orig_span.in_external_macro(self.tcx.sess.source_map()) { + continue; + } + let target_bb = &self.cfg.basic_blocks[target_bb]; + let (target_loc, descr) = target_bb + .statements + .iter() + .find_map(|stmt| match stmt.kind { + StatementKind::StorageLive(_) | StatementKind::StorageDead(_) => None, + StatementKind::FakeRead(..) => Some((stmt.source_info, "definition")), + _ => Some((stmt.source_info, "expression")), + }) + .unwrap_or_else(|| (target_bb.terminator().source_info, "expression")); + let lint_root = self.source_scopes[target_loc.scope] + .local_data + .as_ref() + .unwrap_crate_local() + .lint_root; + self.tcx.emit_node_span_lint( + lint::builtin::UNREACHABLE_CODE, + lint_root, + target_loc.span, + errors::UnreachableDueToUninhabited { + expr: target_loc.span, + orig: orig_span, + descr, + ty: orig_ty, + }, + ); + } + } + fn finish(self) -> Body<'tcx> { let mut body = Body::new( MirSource::item(self.def_id.to_def_id()), diff --git a/compiler/rustc_mir_build/src/errors.rs b/compiler/rustc_mir_build/src/errors.rs index e2aae5b6283f..3d9753d72da5 100644 --- a/compiler/rustc_mir_build/src/errors.rs +++ b/compiler/rustc_mir_build/src/errors.rs @@ -720,6 +720,18 @@ pub(crate) struct WantedConstant { pub(crate) const_path: String, } +#[derive(LintDiagnostic)] +#[diag(mir_build_unreachable_due_to_uninhabited)] +pub(crate) struct UnreachableDueToUninhabited<'desc, 'tcx> { + pub descr: &'desc str, + #[label] + pub expr: Span, + #[label(mir_build_label_orig)] + #[note] + pub orig: Span, + pub ty: Ty<'tcx>, +} + #[derive(Diagnostic)] #[diag(mir_build_const_pattern_depends_on_generic_parameter, code = E0158)] pub(crate) struct ConstPatternDependsOnGenericParameter { diff --git a/compiler/rustc_passes/messages.ftl b/compiler/rustc_passes/messages.ftl index 9971f0c3fa32..be9843988669 100644 --- a/compiler/rustc_passes/messages.ftl +++ b/compiler/rustc_passes/messages.ftl @@ -619,11 +619,6 @@ passes_unnecessary_partial_stable_feature = the feature `{$feature}` has been pa passes_unnecessary_stable_feature = the feature `{$feature}` has been stable since {$since} and no longer requires an attribute to enable -passes_unreachable_due_to_uninhabited = unreachable {$descr} - .label = unreachable {$descr} - .label_orig = any code following this expression is unreachable - .note = this expression has type `{$ty}`, which is uninhabited - passes_unrecognized_argument = unrecognized argument diff --git a/compiler/rustc_passes/src/errors.rs b/compiler/rustc_passes/src/errors.rs index e01e9e384c03..8ca0641c4dd2 100644 --- a/compiler/rustc_passes/src/errors.rs +++ b/compiler/rustc_passes/src/errors.rs @@ -1308,18 +1308,6 @@ pub(crate) struct ProcMacroBadSig { pub kind: ProcMacroKind, } -#[derive(LintDiagnostic)] -#[diag(passes_unreachable_due_to_uninhabited)] -pub(crate) struct UnreachableDueToUninhabited<'desc, 'tcx> { - pub descr: &'desc str, - #[label] - pub expr: Span, - #[label(passes_label_orig)] - #[note] - pub orig: Span, - pub ty: Ty<'tcx>, -} - #[derive(LintDiagnostic)] #[diag(passes_unused_var_maybe_capture_ref)] #[help] diff --git a/compiler/rustc_passes/src/liveness.rs b/compiler/rustc_passes/src/liveness.rs index b628ff8a12df..e175ac1c035e 100644 --- a/compiler/rustc_passes/src/liveness.rs +++ b/compiler/rustc_passes/src/liveness.rs @@ -96,7 +96,7 @@ use rustc_index::IndexVec; use rustc_middle::query::Providers; use rustc_middle::span_bug; use rustc_middle::ty::print::with_no_trimmed_paths; -use rustc_middle::ty::{self, RootVariableMinCaptureList, Ty, TyCtxt}; +use rustc_middle::ty::{self, RootVariableMinCaptureList, TyCtxt}; use rustc_session::lint; use rustc_span::edit_distance::find_best_match_for_name; use rustc_span::{BytePos, Span, Symbol}; @@ -1317,52 +1317,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { fn check_is_ty_uninhabited(&mut self, expr: &Expr<'_>, succ: LiveNode) -> LiveNode { let ty = self.typeck_results.expr_ty(expr); let m = self.ir.tcx.parent_module(expr.hir_id).to_def_id(); - if ty.is_inhabited_from(self.ir.tcx, m, self.typing_env) { - return succ; - } - match self.ir.lnks[succ] { - LiveNodeKind::ExprNode(succ_span, succ_id) => { - self.warn_about_unreachable(expr.span, ty, succ_span, succ_id, "expression"); - } - LiveNodeKind::VarDefNode(succ_span, succ_id) => { - self.warn_about_unreachable(expr.span, ty, succ_span, succ_id, "definition"); - } - _ => {} - }; - self.exit_ln - } - - fn warn_about_unreachable<'desc>( - &mut self, - orig_span: Span, - orig_ty: Ty<'tcx>, - expr_span: Span, - expr_id: HirId, - descr: &'desc str, - ) { - if !orig_ty.is_never() { - // Unreachable code warnings are already emitted during type checking. - // However, during type checking, full type information is being - // calculated but not yet available, so the check for diverging - // expressions due to uninhabited result types is pretty crude and - // only checks whether ty.is_never(). Here, we have full type - // information available and can issue warnings for less obviously - // uninhabited types (e.g. empty enums). The check above is used so - // that we do not emit the same warning twice if the uninhabited type - // is indeed `!`. - - self.ir.tcx.emit_node_span_lint( - lint::builtin::UNREACHABLE_CODE, - expr_id, - expr_span, - errors::UnreachableDueToUninhabited { - expr: expr_span, - orig: orig_span, - descr, - ty: orig_ty, - }, - ); - } + if ty.is_inhabited_from(self.ir.tcx, m, self.typing_env) { succ } else { self.exit_ln } } } diff --git a/tests/ui/enum-discriminant/issue-46519.rs b/tests/ui/enum-discriminant/issue-46519.rs index e5f0138c95ce..bad568ca6c5a 100644 --- a/tests/ui/enum-discriminant/issue-46519.rs +++ b/tests/ui/enum-discriminant/issue-46519.rs @@ -7,6 +7,7 @@ #[should_panic(expected = "creating inhabited type")] fn test() { FontLanguageOverride::system_font(SystemFont::new()); + //~^ WARNING unreachable expression } pub enum FontLanguageOverride { diff --git a/tests/ui/enum-discriminant/issue-46519.stderr b/tests/ui/enum-discriminant/issue-46519.stderr new file mode 100644 index 000000000000..5e4f536a59a7 --- /dev/null +++ b/tests/ui/enum-discriminant/issue-46519.stderr @@ -0,0 +1,18 @@ +warning: unreachable expression + --> $DIR/issue-46519.rs:9:5 + | +LL | FontLanguageOverride::system_font(SystemFont::new()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-----------------^ + | | | + | | any code following this expression is unreachable + | unreachable expression + | +note: this expression has type `SystemFont`, which is uninhabited + --> $DIR/issue-46519.rs:9:39 + | +LL | FontLanguageOverride::system_font(SystemFont::new()); + | ^^^^^^^^^^^^^^^^^ + = note: `#[warn(unreachable_code)]` (part of `#[warn(unused)]`) on by default + +warning: 1 warning emitted + diff --git a/tests/ui/intrinsics/panic-uninitialized-zeroed.rs b/tests/ui/intrinsics/panic-uninitialized-zeroed.rs index db2b4a6ad20a..00fb2b029f9a 100644 --- a/tests/ui/intrinsics/panic-uninitialized-zeroed.rs +++ b/tests/ui/intrinsics/panic-uninitialized-zeroed.rs @@ -7,7 +7,7 @@ //@ ignore-backends: gcc //@ edition:2024 -#![allow(deprecated, invalid_value)] +#![allow(deprecated, invalid_value, unreachable_code)] #![feature(never_type, rustc_private)] use std::{ diff --git a/tests/ui/lint/dead-code/issue-85071-2.rs b/tests/ui/lint/dead-code/issue-85071-2.rs index 5db873589941..06bbcac73739 100644 --- a/tests/ui/lint/dead-code/issue-85071-2.rs +++ b/tests/ui/lint/dead-code/issue-85071-2.rs @@ -17,6 +17,6 @@ fn main() { let s = S; let x = s.f(); //~^ WARNING: unused variable: `x` + //~| WARNING: unreachable definition let _y = x; - //~^ WARNING: unreachable definition } diff --git a/tests/ui/lint/dead-code/issue-85071-2.stderr b/tests/ui/lint/dead-code/issue-85071-2.stderr index 5e963183d094..ab88d7235969 100644 --- a/tests/ui/lint/dead-code/issue-85071-2.stderr +++ b/tests/ui/lint/dead-code/issue-85071-2.stderr @@ -1,23 +1,3 @@ -warning: unreachable definition - --> $DIR/issue-85071-2.rs:20:9 - | -LL | let x = s.f(); - | ----- any code following this expression is unreachable -LL | -LL | let _y = x; - | ^^ unreachable definition - | -note: this expression has type `Foo`, which is uninhabited - --> $DIR/issue-85071-2.rs:18:13 - | -LL | let x = s.f(); - | ^^^^^ -note: the lint level is defined here - --> $DIR/issue-85071-2.rs:7:26 - | -LL | #![warn(unused_variables,unreachable_code)] - | ^^^^^^^^^^^^^^^^ - warning: unused variable: `x` --> $DIR/issue-85071-2.rs:18:9 | @@ -30,5 +10,24 @@ note: the lint level is defined here LL | #![warn(unused_variables,unreachable_code)] | ^^^^^^^^^^^^^^^^ +warning: unreachable definition + --> $DIR/issue-85071-2.rs:18:9 + | +LL | let x = s.f(); + | ^ ----- any code following this expression is unreachable + | | + | unreachable definition + | +note: this expression has type `Foo`, which is uninhabited + --> $DIR/issue-85071-2.rs:18:13 + | +LL | let x = s.f(); + | ^^^^^ +note: the lint level is defined here + --> $DIR/issue-85071-2.rs:7:26 + | +LL | #![warn(unused_variables,unreachable_code)] + | ^^^^^^^^^^^^^^^^ + warning: 2 warnings emitted diff --git a/tests/ui/lint/dead-code/issue-85071.rs b/tests/ui/lint/dead-code/issue-85071.rs index 84f2c9fc74ee..6f177905b596 100644 --- a/tests/ui/lint/dead-code/issue-85071.rs +++ b/tests/ui/lint/dead-code/issue-85071.rs @@ -14,6 +14,6 @@ fn f() -> Foo {todo!()} fn main() { let x = f(); //~^ WARNING: unused variable: `x` + //~| WARNING: unreachable definition let _ = x; - //~^ WARNING: unreachable expression } diff --git a/tests/ui/lint/dead-code/issue-85071.stderr b/tests/ui/lint/dead-code/issue-85071.stderr index 721fb8148d96..c94923063903 100644 --- a/tests/ui/lint/dead-code/issue-85071.stderr +++ b/tests/ui/lint/dead-code/issue-85071.stderr @@ -1,23 +1,3 @@ -warning: unreachable expression - --> $DIR/issue-85071.rs:17:13 - | -LL | let x = f(); - | --- any code following this expression is unreachable -LL | -LL | let _ = x; - | ^ unreachable expression - | -note: this expression has type `Foo`, which is uninhabited - --> $DIR/issue-85071.rs:15:13 - | -LL | let x = f(); - | ^^^ -note: the lint level is defined here - --> $DIR/issue-85071.rs:9:26 - | -LL | #![warn(unused_variables,unreachable_code)] - | ^^^^^^^^^^^^^^^^ - warning: unused variable: `x` --> $DIR/issue-85071.rs:15:9 | @@ -30,5 +10,24 @@ note: the lint level is defined here LL | #![warn(unused_variables,unreachable_code)] | ^^^^^^^^^^^^^^^^ +warning: unreachable definition + --> $DIR/issue-85071.rs:15:9 + | +LL | let x = f(); + | ^ --- any code following this expression is unreachable + | | + | unreachable definition + | +note: this expression has type `Foo`, which is uninhabited + --> $DIR/issue-85071.rs:15:13 + | +LL | let x = f(); + | ^^^ +note: the lint level is defined here + --> $DIR/issue-85071.rs:9:26 + | +LL | #![warn(unused_variables,unreachable_code)] + | ^^^^^^^^^^^^^^^^ + warning: 2 warnings emitted