Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

never_patterns: lower a never arm to an unreachable terminator #120758

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions compiler/rustc_ast/src/util/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ pub enum ExprPrecedence {
Struct,
Gen,
Await,
Unreachable,
Copy link
Member

Choose a reason for hiding this comment

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

Can you just give ExprKind::Unreachable a dummy precedence? Like, just give it the same precedence as Lit or something.

Err,
}

Expand Down Expand Up @@ -353,6 +354,7 @@ impl ExprPrecedence {
| ExprPrecedence::TryBlock
| ExprPrecedence::Gen
| ExprPrecedence::Struct
| ExprPrecedence::Unreachable
| ExprPrecedence::Err => PREC_PAREN,
}
}
Expand Down
12 changes: 1 addition & 11 deletions compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -570,19 +570,9 @@ impl<'hir> LoweringContext<'_, 'hir> {
self.dcx().emit_err(NeverPatternWithGuard { span: g.span });
}

// We add a fake `loop {}` arm body so that it typecks to `!`.
// FIXME(never_patterns): Desugar into a call to `unreachable_unchecked`.
Copy link
Member

Choose a reason for hiding this comment

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

Your PR description doesn't explain the challenges of just lowering a call to, for example, the unreachable intrinsic. I feel like that's far less invasive than adding a new HIR expr kind.

Copy link
Member Author

@Nadrieril Nadrieril Feb 8, 2024

Choose a reason for hiding this comment

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

Ah yes, we discussed this on Zulip. Intrinsics don't exist at the hir level, they have to be constructed during mir lowering/codegen, so that's essentially what I'm doing.

Alternatives would be to make unreachable_unchecked a lang item, or keep the loop and rely on the UninhabitedEnumBranching mir opt to elide it. I went with Unreachable because that's what everyone seemed to prefer; I don't have a strong opinion.

let block = self.arena.alloc(hir::Block {
stmts: &[],
expr: None,
hir_id: self.next_id(),
rules: hir::BlockCheckMode::DefaultBlock,
span,
targeted_by_break: false,
});
self.arena.alloc(hir::Expr {
hir_id: self.next_id(),
kind: hir::ExprKind::Loop(block, None, hir::LoopSource::Loop, span),
kind: hir::ExprKind::Unreachable,
span,
})
};
Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_ast_pretty/src/pprust/state/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -919,14 +919,14 @@ impl<'a> State<'a> {
self.maybe_print_comment(arm.pat.span.lo());
self.print_outer_attributes(&arm.attrs);
self.print_pat(&arm.pat);
self.space();
if let Some(e) = &arm.guard {
self.space();
self.word_space("if");
self.print_expr(e, FixupContext::default());
self.space();
}

if let Some(body) = &arm.body {
self.space();
self.word_space("=>");

match &body.kind {
Expand All @@ -951,7 +951,9 @@ impl<'a> State<'a> {
}
}
} else {
// Empty body following a never pattern.
self.word(",");
self.end(); // Close the ibox for the pattern.
}
self.end(); // Close enclosing cbox.
}
Expand Down
11 changes: 10 additions & 1 deletion compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1604,6 +1604,7 @@ impl Expr<'_> {
ExprKind::Struct(..) => ExprPrecedence::Struct,
ExprKind::Repeat(..) => ExprPrecedence::Repeat,
ExprKind::Yield(..) => ExprPrecedence::Yield,
ExprKind::Unreachable => ExprPrecedence::Unreachable,
ExprKind::Err(_) => ExprPrecedence::Err,
}
}
Expand Down Expand Up @@ -1671,6 +1672,7 @@ impl Expr<'_> {
| ExprKind::Yield(..)
| ExprKind::Cast(..)
| ExprKind::DropTemps(..)
| ExprKind::Unreachable
| ExprKind::Err(_) => false,
}
}
Expand Down Expand Up @@ -1706,7 +1708,10 @@ impl Expr<'_> {

pub fn can_have_side_effects(&self) -> bool {
match self.peel_drop_temps().kind {
ExprKind::Path(_) | ExprKind::Lit(_) | ExprKind::OffsetOf(..) => false,
ExprKind::Path(_)
| ExprKind::Lit(_)
| ExprKind::OffsetOf(..)
| ExprKind::Unreachable => false,
ExprKind::Type(base, _)
| ExprKind::Unary(_, base)
| ExprKind::Field(base, _)
Expand Down Expand Up @@ -1931,6 +1936,10 @@ pub enum ExprKind<'hir> {
/// A suspension point for coroutines (i.e., `yield <expr>`).
Yield(&'hir Expr<'hir>, YieldSource),

/// The absent body of a match arm without a body, encountered in the presence of never
/// patterns. Statically known to be unreachable.
Unreachable,

/// A placeholder for an expression that wasn't syntactically well formed in some way.
Err(rustc_span::ErrorGuaranteed),
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir/src/intravisit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -804,7 +804,7 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr<'v>)
ExprKind::Yield(ref subexpression, _) => {
visitor.visit_expr(subexpression);
}
ExprKind::Lit(_) | ExprKind::Err(_) => {}
ExprKind::Lit(_) | ExprKind::Unreachable | ExprKind::Err(_) => {}
}
}

Expand Down
16 changes: 13 additions & 3 deletions compiler/rustc_hir_pretty/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1519,6 +1519,9 @@ impl<'a> State<'a> {
self.word_space("yield");
self.print_expr_maybe_paren(expr, parser::PREC_JUMP);
}
hir::ExprKind::Unreachable => {
self.word("unreachable!()");
}
hir::ExprKind::Err(_) => {
self.popen();
self.word("/*ERROR*/");
Expand Down Expand Up @@ -1877,16 +1880,16 @@ impl<'a> State<'a> {
self.ibox(0);
self.print_outer_attributes(self.attrs(arm.hir_id));
self.print_pat(arm.pat);
self.space();
if let Some(ref g) = arm.guard {
self.space();
self.word_space("if");
self.print_expr(g);
self.space();
}
self.word_space("=>");

match arm.body.kind {
hir::ExprKind::Block(blk, opt_label) => {
self.space();
self.word_space("=>");
if let Some(label) = opt_label {
self.print_ident(label.ident);
self.word_space(":");
Expand All @@ -1900,7 +1903,14 @@ impl<'a> State<'a> {
self.word(",");
}
}
hir::ExprKind::Unreachable => {
// Empty body following a never pattern.
self.word(",");
self.end(); // close the ibox for the pattern
}
_ => {
self.space();
self.word_space("=>");
self.end(); // close the ibox for the pattern
self.print_expr(arm.body);
self.word(",");
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
ExprKind::Match(discrim, arms, match_src) => {
self.check_match(expr, discrim, arms, expected, match_src)
}
ExprKind::Unreachable => {
self.diverges.set(self.diverges.get() | Diverges::always(expr.span));
self.tcx.types.never
}
ExprKind::Closure(closure) => self.check_expr_closure(closure, expr.span, expected),
ExprKind::Block(body, _) => self.check_block_with_expected(body, expected),
ExprKind::Call(callee, args) => self.check_call(expr, callee, args, expected),
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir_typeck/src/expr_use_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
}

hir::ExprKind::Continue(..)
| hir::ExprKind::Unreachable
| hir::ExprKind::Lit(..)
| hir::ExprKind::ConstBlock(..)
| hir::ExprKind::OffsetOf(..)
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir_typeck/src/mem_categorization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,7 @@ impl<'a, 'tcx> MemCategorizationContext<'a, 'tcx> {
| hir::ExprKind::AssignOp(..)
| hir::ExprKind::Closure { .. }
| hir::ExprKind::Ret(..)
| hir::ExprKind::Unreachable
| hir::ExprKind::Become(..)
| hir::ExprKind::Unary(..)
| hir::ExprKind::Yield(..)
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_middle/src/thir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,9 @@ pub enum ExprKind<'tcx> {
Yield {
value: ExprId,
},
/// The absent body of a match arm without a body, encountered in the presence of never
/// patterns. Statically known to be unreachable.
Unreachable,
}

/// Represents the association of a field identifier and an expression.
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/thir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ pub fn walk_expr<'thir, 'tcx: 'thir, V: Visitor<'thir, 'tcx>>(
OffsetOf { container: _, fields: _ } => {}
ThreadLocalRef(_) => {}
Yield { value } => visitor.visit_expr(&visitor.thir()[value]),
Unreachable => {}
}
}

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_build/src/build/expr/as_place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
| ExprKind::Break { .. }
| ExprKind::Continue { .. }
| ExprKind::Return { .. }
| ExprKind::Unreachable { .. }
| ExprKind::Become { .. }
| ExprKind::Literal { .. }
| ExprKind::NamedConst { .. }
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_build/src/build/expr/as_rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
| ExprKind::Break { .. }
| ExprKind::Continue { .. }
| ExprKind::Return { .. }
| ExprKind::Unreachable { .. }
| ExprKind::Become { .. }
| ExprKind::InlineAsm { .. }
| ExprKind::PlaceTypeAscription { .. }
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_build/src/build/expr/category.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ impl Category {
| ExprKind::Break { .. }
| ExprKind::Continue { .. }
| ExprKind::Return { .. }
| ExprKind::Unreachable { .. }
| ExprKind::Become { .. } =>
// FIXME(#27840) these probably want their own
// category, like "nonterminating"
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_mir_build/src/build/expr/into.rs
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
resume.unit()
}

ExprKind::Unreachable => {
this.cfg.terminate(block, source_info, TerminatorKind::Unreachable);
this.cfg.start_new_block().unit()
}

// these are the cases that are more naturally handled by some other mode
ExprKind::Unary { .. }
| ExprKind::Binary { .. }
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_build/src/check_unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,7 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
| ExprKind::Return { .. }
| ExprKind::Become { .. }
| ExprKind::Yield { .. }
| ExprKind::Unreachable { .. }
| ExprKind::Loop { .. }
| ExprKind::Let { .. }
| ExprKind::Match { .. }
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_build/src/thir/cx/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -786,6 +786,7 @@ impl<'tcx> Cx<'tcx> {
hir::ExprKind::Tup(fields) => ExprKind::Tuple { fields: self.mirror_exprs(fields) },

hir::ExprKind::Yield(v, _) => ExprKind::Yield { value: self.mirror_expr(v) },
hir::ExprKind::Unreachable => ExprKind::Unreachable,
hir::ExprKind::Err(_) => unreachable!("cannot lower a `hir::ExprKind::Err` to THIR"),
};

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_build/src/thir/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
}

// These diverge.
Become { .. } | Break { .. } | Continue { .. } | Return { .. } => true,
Become { .. } | Break { .. } | Continue { .. } | Return { .. } | Unreachable => true,

// These are statements that evaluate to `()`.
Assign { .. } | AssignOp { .. } | InlineAsm { .. } | Let { .. } => true,
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_mir_build/src/thir/print.rs
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,9 @@ impl<'a, 'tcx> ThirPrinter<'a, 'tcx> {
self.print_expr(*value, depth_lvl + 2);
print_indented!(self, "}", depth_lvl);
}
Unreachable => {
print_indented!(self, "Unreachable", depth_lvl);
}
}
}

Expand Down
38 changes: 34 additions & 4 deletions compiler/rustc_passes/src/hir_stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,10 +319,40 @@ impl<'v> hir_visit::Visitor<'v> for StatCollector<'v> {
record_variants!(
(self, e, e.kind, Id::Node(e.hir_id), hir, Expr, ExprKind),
[
ConstBlock, Array, Call, MethodCall, Tup, Binary, Unary, Lit, Cast, Type,
DropTemps, Let, If, Loop, Match, Closure, Block, Assign, AssignOp, Field, Index,
Path, AddrOf, Break, Continue, Ret, Become, InlineAsm, OffsetOf, Struct, Repeat,
Yield, Err
ConstBlock,
Array,
Call,
MethodCall,
Tup,
Binary,
Unary,
Lit,
Cast,
Type,
DropTemps,
Let,
If,
Loop,
Match,
Closure,
Block,
Assign,
AssignOp,
Field,
Index,
Path,
AddrOf,
Break,
Continue,
Ret,
Become,
InlineAsm,
OffsetOf,
Struct,
Repeat,
Yield,
Unreachable,
Err
]
);
hir_visit::walk_expr(self, e)
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_passes/src/liveness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,7 @@ impl<'tcx> Visitor<'tcx> for IrMaps<'tcx> {
| hir::ExprKind::InlineAsm(..)
| hir::ExprKind::OffsetOf(..)
| hir::ExprKind::Type(..)
| hir::ExprKind::Unreachable
| hir::ExprKind::Err(_)
| hir::ExprKind::Path(hir::QPath::TypeRelative(..))
| hir::ExprKind::Path(hir::QPath::LangItem(..)) => {}
Expand Down Expand Up @@ -1103,6 +1104,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {

hir::ExprKind::Lit(..)
| hir::ExprKind::ConstBlock(..)
| hir::ExprKind::Unreachable
| hir::ExprKind::Err(_)
| hir::ExprKind::Path(hir::QPath::TypeRelative(..))
| hir::ExprKind::Path(hir::QPath::LangItem(..))
Expand Down Expand Up @@ -1396,6 +1398,7 @@ fn check_expr<'tcx>(this: &mut Liveness<'_, 'tcx>, expr: &'tcx Expr<'tcx>) {
| hir::ExprKind::Path(_)
| hir::ExprKind::Yield(..)
| hir::ExprKind::Type(..)
| hir::ExprKind::Unreachable
| hir::ExprKind::Err(_) => {}
}
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_passes/src/naked_functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ impl<'tcx> CheckInlineAssembly<'tcx> {
| ExprKind::Become(..)
| ExprKind::Struct(..)
| ExprKind::Repeat(..)
| ExprKind::Unreachable
| ExprKind::Yield(..) => {
self.items.push((ItemKind::NonAsm, span));
}
Expand Down
6 changes: 5 additions & 1 deletion compiler/rustc_ty_utils/src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,10 @@ fn recurse_build<'tcx>(
error(GenericConstantTooComplexSub::ClosureAndReturnNotSupported(node.span))?
}
// let expressions imply control flow
ExprKind::Match { .. } | ExprKind::If { .. } | ExprKind::Let { .. } => {
ExprKind::Match { .. }
| ExprKind::If { .. }
| ExprKind::Let { .. }
| ExprKind::Unreachable => {
error(GenericConstantTooComplexSub::ControlFlowNotSupported(node.span))?
}
ExprKind::InlineAsm { .. } => {
Expand Down Expand Up @@ -341,6 +344,7 @@ impl<'a, 'tcx> IsThirPolymorphic<'a, 'tcx> {
| thir::ExprKind::Break { .. }
| thir::ExprKind::Continue { .. }
| thir::ExprKind::Return { .. }
| thir::ExprKind::Unreachable { .. }
| thir::ExprKind::Become { .. }
| thir::ExprKind::Array { .. }
| thir::ExprKind::Tuple { .. }
Expand Down
1 change: 1 addition & 0 deletions src/tools/clippy/clippy_lints/src/loops/never_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@ fn never_loop_expr<'tcx>(
NeverLoopResult::Normal
},
})),
ExprKind::Unreachable => NeverLoopResult::Diverging,
ExprKind::OffsetOf(_, _)
| ExprKind::Yield(_, _)
| ExprKind::Closure { .. }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,8 @@ impl<'a, 'tcx> Visitor<'tcx> for SigDropHelper<'a, 'tcx> {
ExprKind::Loop(_, _, _, _) |
ExprKind::Path(_) |
ExprKind::Struct(_, _, _) |
ExprKind::Type(_, _) => {
ExprKind::Type(_, _) |
ExprKind::Unreachable => {
return;
}
}
Expand Down
1 change: 1 addition & 0 deletions src/tools/clippy/clippy_lints/src/utils/author.rs
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,7 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> {
},
}
},
ExprKind::Unreachable => kind!("Unreachable"),
ExprKind::Err(_) => kind!("Err(_)"),
ExprKind::DropTemps(expr) => {
bind!(self, expr);
Expand Down
Loading
Loading